[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Coding style and whitespace

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

In my supplied patch I wrote:
    case dep_depends:     fmt= _("%s depends on %s");     break;
    case dep_predepends:  fmt= _("%s pre-depends on %s"); break;
    case dep_recommends:  fmt= _("%s recommends %s");     break;

However in dpkg 1.14.4 I see:
    case dep_depends:
      fmt = _("%s depends on %s");
    case dep_predepends:
      fmt = _("%s pre-depends on %s");
    case dep_recommends:
      fmt = _("%s recommends %s");

There are two formatting differences here:
 * Tabular formatting versus vertically-laid-out formatting;
 * Space before `='.

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.  (The original code uses tabular formatting of switch
   statements where possible and does not place a space before
   assignment `='.)

 * One should not change formatting without a good reason.  I supplied
   a patch but the formatting actually applied was different.  This
   has caused me needless work during my merge.

 * The formatting should be clear and easy to read.

   It is plain that the tabular formatting is easier to read.  You can
   trivially see that the code has a perfectly regular structure and
   simply read off in each case what the details are.  We use one
   visual dimension for the different cases and the other for the
   different details and furniture required for each case.

   In the vertically-separated case it is much more difficult to check
   that all of the cases are in fact identical and the boundaries
   between cases are marked only by and indent and no longer by the
   rows of the tabular structure.

   The case of the space before `=' is less clear on this point but I
   prefer the missing space beforehand so as to emphasise the
   asymmetric nature of the operator.

 * 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 `=').

Obviously one can give different weights to these criteria but since
they all agree in this case I can't see what possible excuse there
could be.

Thank you for your attention.


Reply to: