[OPEN-ILS-DEV] Whitespace, copyright (was: [PATCH] Renewal notice hook, cleanup)

[Dan Scott]

On Thu, 2009-09-17 at 16:28 -0400, Jason Etheridge wrote:
> > Also, some other projects that I've worked on have had a rule (informal
> > in some cases, more formal in others - hello PHP / PEAR!) that white
> > space changes should be committed separately from code changes, to make
> > it simpler for other people to focus on the actual code diffs. I don't
> > think we have any such rules (informal or formal) about that in
> > Evergreen today, but it does make sense to me. Anyone else care to weigh
> > in?
> 
> That's how I've been trying to do it, but of course, if I change
> whitespace by accident, I'm too lazy to try to work it out of my
> commit. :)

Right, I've certainly been guilty in the past too. But then, we've never
really talked about it so anything was fair game :) So we could agree to
try to keep whitespace changes (say, more than a couple of contiguous
lines) separate from code changes...

> The .vimrc I tend to use looks like this (I think our expandtab is different?):

Yes, it's different, mostly because the settings differ from file to
file, likely matching each developer's individual preferences (and are
sometimes nicely horribly mixed within each file - see
Open-ILS/src/perlmods/OpenILS/Utils/CStoreEditor.pm for one example).

When I have cleaned whitespace up, I've tried to stick with the
predominant style for a given file (in the absence of any general
agreement) - per the sage advice at
http://open-ils.org/documentation/contributing.html: "Try to make your
patch as readable as possible by following the surrounding code-layout
conventions. This makes it easier for the reviewer, and there's no point
in trying to layout things differently. Also avoid unnecessary
whitespace changes because they just distract the reviewer, and
formatting changes will likely be removed by the committing core team
member."

> set hlsearch
> set tabstop=4
> set softtabstop=4
> set shiftwidth=4
> vnoremap < <gv
> vnoremap > >gv
> set nobk
> set whichwrap=b,s,<,>,[,]
> set backspace=indent,eol,start
> set smartcase
> filetype on
> syntax enable
> set expandtab
> autocmd BufEnter ?akefile* set noet ts=4 sw=4  "use real tabs in Makefiles
> autocmd BufEnter *txt set noet ts=4 sw=4  "use real tabs in txt files
> autocmd BufEnter *out set noet ts=4 sw=4  "use real tabs in txt files
> autocmd BufEnter *csv set noet ts=4 sw=4  "use real tabs in txt files
> au BufNewFile,BufRead *.xhtml setf html
> au BufNewFile,BufRead *.bsh setf java
> au BufNewFile,BufRead *.ftl setf html
> set bg=dark
> let loaded_matchparen = 1
> 
> I have this sitting at http://evergreen-ils.org/~phasefx/.vimrc to
> make it easy whenever I find myself in a new environment.

If I only worked on Evergreen, then I would be happy to adopt the .vimrc
approach, but I do work on a number of different projects which have
conflicting standards. In PEAR / PEARDOC the standard is to include the
vim / emacs comments in each file, so I could work there without having
to worry about changing my editor settings.

And that's the basic reason I advocated the vim comment / emacs comment
(for David and for any other emacs-types that might have joined or will
join the project): it reduces the chance of getting it wrong, at least
for the text editors currently favoured by the contributors, and it
would help newcomers from straying off the path. Yes, it's a bit of
noise in each file, but it seems less draconian to me than globally
setting the options for each developer's editor.

Ultimately, though, it's just whitespace. It would be nice to have a
consistent style in the project source - less potential noise in
patches, and nicely aligned left-hand indentation makes it easier to
skim through code - but if it's going to take much effort to reach
agreement, it probably isn't worthwhile. What is this, Python?

If somebody does go through and makes all of the indenting consistent,
perhaps they could tackle copyright statements and license headers at
the same time. Heh.