On 10/26/11 7:38 PM, Keshav Kini wrote:
Well, the kernel devs try to remove trailing whitespace where it is
found (and while in the process of doing something more meaningful) [1]
and avoid introducing it in the first place at all [2] but do not create
patchbombs by going out of their way to do so. I mean, this just makes
sense anyway.
As far as I know, the general attitude among developers is that trailing
whitespace is always bad (a Google search for "trailing whitespace" will
return rants about it by the dozens as well as plenty of tips on how to
get rid of it using various editors, VCS tools, etc.), but removing
existing trailing whitespace is not worth creating extraneous chunks in
patches/diffs. And I agree with that, just to make it clear to for
example Florent above :) As I mentioned in the OP, git used to by
default refuse to let you commit changes that introduced new trailing
space, but of course didn't complain when you left alone trailing space
that was already there. We could probably do a similar thing by hooking
into trac and not allowing uploads of patches that introduce trailing
whitespace.
But this may not be agreeable to Jason or others who like to put
trailing space on otherwise empty lines. Would it be possible to
configure emacs to pretend there is space there, or something like that?
I imagine it should be easy to add something to ~/.emacs which upon
loading a .py or .pyx file searches for blocks of empty lines, checks
previous and following lines' indentation, and if it's the same both
above and below, apply the same indentation to all the empty lines in
between. Then another hook would, upon save-to-disk, just delete all
trailing whitespace again. This would transparently make the cursor
movement nicer within the open buffer for people who like it that way.
Jason, what do you think? (I CC'd Jason).
Interesting solution. But please. The barrier and standards for
contributions are already high enough. Let's not insist on a
contributor using emacs (as much as I personally like it), or
configuring their editor for such an issue. Let's just view not having
spaces on nonempty lines as good style and leave it at that. And let's
not insist that people who spent a lot of their time to program a patch
be rejected, for example if their editor happened to put a space when it
automatically wrapped/filled a documentation line. I think if I was a
new contributor, with limited time, who had just spent a lot of time
learning python, mercurial, queues, trac, docstrings, rest, doctests,
and everything else we demand for a good patch, if I got rejected
because of an extra space at the end of a line, I may just throw my
hands up in the air and give up.
By the way, there is also a third-party Mercurial extension which bugs
you when you try to commit (or qnew/qrefresh, I imagine) trailing
whitespace. We could... ship this with the built-in Mercurial, for what
it's worth, but I think the above solution (hooking into trac) is better.
Again, sorry for wasting your time with such a trivial matter. I didn't
expect the thread to grow so huge! :)
I agree. As Florent pointed out, the maintenance burden would be huge.
Rejections for such an inconsequential bike-shedding issue that is
just a matter of taste are a huge turn-off for a new developer. The
benefits are practically nil, as I see it. I think the linux policy is
sane---if you happen to be editing a line and remove the trailing
whitespace, good for you. But let's leave it at that.
Sorry, I don't mean to be mean. I just think there are far more
important things that would help Sage, and I don't see any positives to
rejecting patches by people for such an issue.
Thanks,
Jason
--
To post to this group, send an email to sage-devel@googlegroups.com
To unsubscribe from this group, send an email to
sage-devel+unsubscr...@googlegroups.com
For more options, visit this group at http://groups.google.com/group/sage-devel
URL: http://www.sagemath.org
