Re: [boost] [Intrusive] Cleaning up trailing white space
Olaf van der Spek <ml@vdspek.org> On Thu, May 17, 2012 at 1:12 AM, Brian Wood <woodbrian77@gmail.com> wrote:
This is probably the easiest way to improve Boost. ?I hope authors will
Really? Why? It can be done with a simple sed script and applied either to individual libraries or all of Boost. It's a small detail, but I hope it gets some attention. I brought it to Ion's attention as I use the Intrusive library and take a number of steps to reduce the need for new hardware. I won't stop using the library if he doesn't do this, but the squeaky wheel gets the oil.
Cheers, Brian Ebenezer Enterprises http://webEbenezer.net <http://webebenezer.net/>
On Fri, May 18, 2012 at 8:58 AM, Brian Wood <woodbrian77@gmail.com> wrote:
Olaf van der Spek <ml@vdspek.org> On Thu, May 17, 2012 at 1:12 AM, Brian Wood <woodbrian77@gmail.com> wrote:
This is probably the easiest way to improve Boost. ?I hope authors will
Really? Why? It can be done with a simple sed script and applied either to individual libraries or all of Boost. It's a small detail, but I hope it gets some attention. I brought it to Ion's attention as I use the Intrusive library and take a number of steps to reduce the need for new hardware. I won't stop using the library if he doesn't do this, but the squeaky wheel gets the oil.
That's an explanation for the easy part, but why is this an improvement? Olaf -- Olaf
On Friday 18 May 2012 09:19:41 Olaf van der Spek wrote:
On Fri, May 18, 2012 at 8:58 AM, Brian Wood <woodbrian77@gmail.com> wrote:
Olaf van der Spek <ml@vdspek.org>
On Thu, May 17, 2012 at 1:12 AM, Brian Wood <woodbrian77@gmail.com> wrote:
This is probably the easiest way to improve Boost. ?I hope authors will
Really? Why?
It can be done with a simple sed script and applied either to individual libraries or all of Boost. It's a small detail, but I hope it gets some attention. I brought it to Ion's attention as I use the Intrusive library and take a number of steps to reduce the need for new hardware. I won't stop using the library if he doesn't do this, but the squeaky wheel gets the oil.
That's an explanation for the easy part, but why is this an improvement?
Are you implying that cleaning the code will make it worse? Personally, I doubt there's a measurable impact on performance of this change, but it improves code clarity. Those extra spaces irritate a little when I read or edit code, so I'm all for it.
On Fri, May 18, 2012 at 9:30 AM, Andrey Semashev <andrey.semashev@gmail.com> wrote:
On Friday 18 May 2012 09:19:41 Olaf van der Spek wrote:
On Fri, May 18, 2012 at 8:58 AM, Brian Wood <woodbrian77@gmail.com> wrote:
Olaf van der Spek <ml@vdspek.org>
On Thu, May 17, 2012 at 1:12 AM, Brian Wood <woodbrian77@gmail.com> wrote:
This is probably the easiest way to improve Boost. ?I hope authors will
Really? Why?
It can be done with a simple sed script and applied either to individual libraries or all of Boost. It's a small detail, but I hope it gets some attention. I brought it to Ion's attention as I use the Intrusive library and take a number of steps to reduce the need for new hardware. I won't stop using the library if he doesn't do this, but the squeaky wheel gets the oil.
That's an explanation for the easy part, but why is this an improvement?
Are you implying that cleaning the code will make it worse?
No
Personally, I doubt there's a measurable impact on performance of this change, but it improves code clarity. Those extra spaces irritate a little when I read or edit code, so I'm all for it.
How do they (trailing whitespace) affect reading? -- Olaf
On Friday 18 May 2012 09:33:35 Olaf van der Spek wrote:
On Fri, May 18, 2012 at 9:30 AM, Andrey Semashev
Personally, I doubt there's a measurable impact on performance of this change, but it improves code clarity. Those extra spaces irritate a little when I read or edit code, so I'm all for it.
How do they (trailing whitespace) affect reading?
They clutter the code when spaces are visible. And it's simply untidy.
on Fri May 18 2012, Andrey Semashev <andrey.semashev-AT-gmail.com> wrote:
On Friday 18 May 2012 09:33:35 Olaf van der Spek wrote:
On Fri, May 18, 2012 at 9:30 AM, Andrey Semashev
Personally, I doubt there's a measurable impact on performance of this change, but it improves code clarity. Those extra spaces irritate a little when I read or edit code, so I'm all for it.
How do they (trailing whitespace) affect reading?
They clutter the code when spaces are visible. And it's simply untidy.
They never bothered me a bit until emacs started higlighting them in red. Now I waste time trying to get rid of them instead of just turning off the highlight :-) -- Dave Abrahams BoostPro Computing http://www.boostpro.com
Dave Abrahams <dave@boostpro.com> writes:
They never bothered me a bit until emacs started higlighting them in red. Now I waste time trying to get rid of them instead of just ^^^^^^^^^^ turning off the highlight :-)
(add-hook 'write-file-functions (lambda () (whitespace-cleanup) nil)) One can easily write a script that cleans the whole source tree. Christopher
on Mon May 21 2012, Christopher Schmidt <christopher-AT-ch.ristopher.com> wrote:
Dave Abrahams <dave@boostpro.com> writes:
They never bothered me a bit until emacs started higlighting them in red. Now I waste time trying to get rid of them instead of just ^^^^^^^^^^ turning off the highlight :-)
(add-hook 'write-file-functions (lambda () (whitespace-cleanup) nil))
One can easily write a script that cleans the whole source tree.
Yup. But that's not so great either when you start working in someone else's file and it starts adding spurious changes. And yes, I've tried packages that try to be smarter about that. Didn't work for me, so far. -- Dave Abrahams BoostPro Computing http://www.boostpro.com
Dave Abrahams <dave@boostpro.com> writes:
Yup. But that's not so great either when you start working in someone else's file and it starts adding spurious changes. And yes, I've tried packages that try to be smarter about that. Didn't work for me, so far.
I think I found a package once that only eliminates spurious whitespace in lines that you've changed. But yeah, it wasn't smart enough to be worthwhile. I've never found a good answer to this either. -- John Wiegley BoostPro Computing http://www.boostpro.com
On Tue, May 29, 2012 at 12:57 AM, John Wiegley <jwiegley@gmail.com> wrote:
Dave Abrahams <dave@boostpro.com> writes:
Yup. But that's not so great either when you start working in someone else's file and it starts adding spurious changes. And yes, I've tried packages that try to be smarter about that. Didn't work for me, so far.
I think I found a package once that only eliminates spurious whitespace in lines that you've changed. But yeah, it wasn't smart enough to be worthwhile. I've never found a good answer to this either.
Can't the script be ran once for the entire repo? -- Olaf
On Tue, May 29, 2012 at 09:11:52AM +0200, Olaf van der Spek wrote:
On Tue, May 29, 2012 at 12:57 AM, John Wiegley <jwiegley@gmail.com> wrote:
> Dave Abrahams <dave@boostpro.com> writes:
Yup. But that's not so great either when you start working in someone else's file and it starts adding spurious changes. And yes, I've tried packages that try to be smarter about that. Didn't work for me, so far.
I think I found a package once that only eliminates spurious whitespace in lines that you've changed. But yeah, it wasn't smart enough to be worthwhile. I've never found a good answer to this either.
Can't the script be ran once for the entire repo?
The downside of making "pointless" changes like that is that you get a commit that will look like the file was changed, effectively making all diffs across that commit have lots of modifications obscuring any real changes. The reason you only clean up whitespace around where you're working is that the whitespace changes are tightly coupled with actual real relevant semantical changes. Doing it on every file in the repository will inflict this pain on every single file, not to mention that it ruins the "last modified" field of SVN browsers, claiming that every single file in the repos was "last modified: 9 days ago" or so while they might've not really have been touched in many years. -- Lars Viklund | zao@acc.umu.se
On Tue, May 29, 2012 at 9:27 AM, Lars Viklund <zao@acc.umu.se> wrote:
Can't the script be ran once for the entire repo?
The downside of making "pointless" changes like that is that you get a commit that will look like the file was changed, effectively making all diffs across that commit have lots of modifications obscuring any real changes.
Tell diff to ignore whitespace (changes). But yeah, it might be a bit problematic. Olaf
On Tue, May 29, 2012 at 09:34:02AM +0200, Olaf van der Spek wrote:
On Tue, May 29, 2012 at 9:27 AM, Lars Viklund <zao@acc.umu.se> wrote:
Can't the script be ran once for the entire repo?
The downside of making "pointless" changes like that is that you get a commit that will look like the file was changed, effectively making all diffs across that commit have lots of modifications obscuring any real changes.
Tell diff to ignore whitespace (changes). But yeah, it might be a bit problematic.
I did intentionally not mention that, as there's a lot of tools out there, many of which doesn't have/want such a feature. It still doesn't change that there would be a spurious commit out there muddling the waters, particularly regarding the quite important "last touched" date. I'd expect such a commit to put a fair amount of strain on the repository infrastructure, considering that it'll have over nine thousand source files touched, with a change to possibly every line of every file. Not to mention that such a script cannot deduce the intended size and purpose of tabs, so the process/output would have to be vetted by a human to ensure that it has not screwed up indentation/alignment. -- Lars Viklund | zao@acc.umu.se
On Tue, May 29, 2012 at 9:52 AM, Lars Viklund <zao@acc.umu.se> wrote:
On Tue, May 29, 2012 at 09:34:02AM +0200, Olaf van der Spek wrote:
On Tue, May 29, 2012 at 9:27 AM, Lars Viklund <zao@acc.umu.se> wrote:
Can't the script be ran once for the entire repo?
The downside of making "pointless" changes like that is that you get a commit that will look like the file was changed, effectively making all diffs across that commit have lots of modifications obscuring any real changes.
Tell diff to ignore whitespace (changes). But yeah, it might be a bit problematic.
I did intentionally not mention that, as there's a lot of tools out there, many of which doesn't have/want such a feature.
It still doesn't change that there would be a spurious commit out there muddling the waters, particularly regarding the quite important "last touched" date.
True
I'd expect such a commit to put a fair amount of strain on the repository infrastructure, considering that it'll have over nine thousand source files touched, with a change to possibly every line of every file.
You could do it multiple commits if that's the problem.
Not to mention that such a script cannot deduce the intended size and purpose of tabs, so the process/output would have to be vetted by a human to ensure that it has not screwed up indentation/alignment.
It's about trailing whitespace, so that doesn't apply. -- Olaf
On 28 May 2012 23:57, John Wiegley <jwiegley@gmail.com> wrote:
Dave Abrahams <dave@boostpro.com> writes:
Yup. But that's not so great either when you start working in someone else's file and it starts adding spurious changes. And yes, I've tried packages that try to be smarter about that. Didn't work for me, so far.
I think I found a package once that only eliminates spurious whitespace in lines that you've changed. But yeah, it wasn't smart enough to be worthwhile. I've never found a good answer to this either.
It seems too obvious to mention, but I've found that using colour diffs with git (which highlights whitespace issues that have been introduced) and cleaning them up using 'git rebase --whitespace=fix' works for me. I think I've seen a git hook somewhere for automatically cleaning up new changes.
With the modularisation activity would it be worth rewriting the historical commits to remove all trailing whitespace? Feels like it would be a fairly simple thing to do. Also even if we don't end up applying this to subversion isnt it worth adding a whitespace cleanup step to the packaging process when the release tarballs are rolled?
Personally, I doubt there's a measurable impact on performance of this change, but it improves code clarity. Those extra spaces irritate a little when I read or edit code, so I'm all for it. How do they (trailing whitespace) affect reading?
some people have them highlighted to ensure that the editor does not mix tabs/spaces ...
participants (10)
-
Andrey Semashev -
Brian Wood -
Christopher Schmidt -
Daniel James -
Dave Abrahams -
James Sharpe -
John Wiegley -
Lars Viklund -
Olaf van der Spek -
Tim Blechmann