Re: Coding style and whitespace
Hi Ian,
I've been meaning to send a mail to discuss the coding style issue in
the code base, and a reply to your request for indentation fixes which
was long due, but this being a very sensitive subject (as you already
acknowledged in your mail from last summer) has made me postpone it.
I'm sorry that this has been triggered from your side when doing the
merge and causing you additional work. There's some patches I've not
applied yet (which I want to, ASAP), also due to the coding style stuff
because I didn't want to keep "fixing" it w/o further discussion, or
at least a public notice.
On Wed, 2007-05-30 at 16:24:35 +0100, Ian Jackson wrote:
> While I was doing a merge of recent Debian and Ubuntu changes I found
> a conflict caused by a reforrmatting of the patch I submitted in
> #390916:
> I can see no excuse for the formatting actually applied on any of the
> conventionally applied criteria:
> * When making changes one should generally adopt the style of the
> original code.
I'd reformulate this to, when sending patches to a project one does
not have commit rights, one must try to match the existing coding
style if there's any. And I completely agree with that.
But the problem comes when one wants to change the coding style of an
existing project. I think all approaches have problems. You can fix it
in a massive commit, which destroys 'blame' and touches all files and
creates a kind of diff barrier before and after that commit, making it
both a bit useless. The other approach is incrementaly fixing the
coding style, which implies having mixed and conflicting styles on the
same code base (even on the same file!), but this does not destroy
'blame' nor does it create a diff barrier, it might just take a long
long time.
For now I've been using the latter, but maybe it makes sense to unify
everything to the same, and then we can be clear on the current style
(and also document it).
> (The original code uses tabular formatting of switch
> statements where possible
I have a problem with the "where possible", it's not consistent and
it makes it difficult to infer, more so given the mix of coding styles
in the current code base...
> and does not place a space before assignment `='.)
... and this also applies to spaces around '=', even your reindenting
patch is not consistent in that regard, and is missing spaces after
the '='.
> * One should not change formatting without a good reason. I supplied
> a patch but the formatting actually applied was different.
The reason I've done that is because I find the current coding style
inconsistent and too packed, it cannot breath, variable and type names
massively abbreviated and missing underscores, missing blank lines
delimiting blocks, missing spaces between operators, missing spaces
after commas, multiple statements on the same line, two spaces for
indentation, etc. This makes the eyes to get tired soon.
On the perl side, some of those apply, there's also the problem of
lack of functions (which I've been fixing by refactoring for some),
and usage of old perl constructs (which I've been incrementaly fixing
as well).
> This has caused me needless work during my merge.
And I'm sorry about that.
> * The formatting should be clear and easy to read.
See two paragraphs above.
> * If reformatting, one should do so in a way that ensures that the
> resulting code has a consistent format. Ie, if the new format
> really were more desirable then all of the tabular forms in the
> code should be made similarly illegible (oops my bias is showing)
> and a space should be added before every `=' (except perhaps in
> `for' statements where the current convention generally also omits
> the space after `=').
See my comment on changing the whole coding style.
Personally I value consistency, readability and minimization of diff
size. So I'd really like to unify the style to something ressembling
the Linux CodingStyle. What do others think?
regards,
guillem
Reply to: