Re: Mixing whitespace and topical changes

[Reiner Steib <reinersteib+gmane@imap.cc>]

On Mon, Apr 16 2007, Didier Verna wrote:

> Reiner Steib <reinersteib+gmane@imap.cc> wrote:
>
>> please avoid committing lots of whitespace changes mixed with topical
>> changes.
>
>                 Reiner,
>
>         I can understand your concern (I guess it mainly has to do with
> synchronizing between the GNU Emacs version and the CVS version),

- Synchronizing also happens between the trunk and v5-10 (i.e. 5.10.6
  ... 5.10.nn).  Introducing lots of whitespace changes in the trunk
  will make it much more likely that merging bug-fixes from v5-10 to
  the trunk fails.  This puts additional burden on Miles.

  When I integrated Oort Gnus in Emacs 22, I've spent much time do
  clean up the problems because we didn't follow Emacs conventions
  closely (e.g. properly attributing ChangeLog entries).  As long as
  Miles is doing the syncing semi-automatically, we will not run into
  this trouble again for No Gnus.  Additionally we get longer active
  maintainance of the stable series for free (BTW, also XEmacs
  benefits from this, see 5.10.8 in the prerelease packages).  As the
  syncing is very important for the future development of Gnus, we
  *must not* make Miles' job more complicated than necessary.

  The fact that doing mass whitespace change (and even mixing them
  with topical changes) makes syncing harder, already is a
  k.o. argument against switching to such a policy.

- Unrelated whitespace changes make many CVS operations less useful
  (diff, annotate, ...).  E.g. in your recent commits, it was hard for
  me to identify the topical changes.  I had to add `-w' temporarily
  to `vc-diff-switches' to recognize the topical changes.

- Automatic whitespace removal may even lead to wrong code...

,----[ flow-fill.el ]
| revision 7.13
| date: 2005/10/27 17:55:26;  author: rsteib;  state: Exp;  lines: +35 -30
| (fill-flowed-encode-tests): Use concatenated string to protect trailing
| whitespace.
| ----------------------------
| revision 7.12
| date: 2005/10/27 17:36:02;  author: rsteib;  state: Exp;  lines: +9 -9
| (fill-flowed-encode-tests): Restore trailing
| whitespace removed in revision 7.8.
`----

  Well, this was some code for testing, but it may also change "? " to
  "?" or similar (IIRC).

- As Gnus is part of Emacs we should follow the same policies whenever
  possible.

  * It's consensus among Emacs developers _not_ to do mass whitespace
    removal without prior coordination.

  * It's consensus among Emacs developers to use the Emacs defaults
    for editing Emacs files (e.g. fill-column, tab-width, coding
    style, ...).  `whitespace-cleanup' in `write-file-hooks' is not
    the default in Emacs.

> but I'm not ready to change my setup (all whitespace cleanup is done
> automatically and I never see them in diffs) until this issue has
> been officially resolved. I've tried to raise it several times in
> the past but didn't get any interest from anybody. Note that I will
> accept whatever decision the majority adopts, even if it goes
> against me :-)

Didier, in the past few years, I've never seen any other Gnus
developer committing mixed changes except you.  The only
discussions[*] on ding about this topic were initiated after your
commits, BTW.  In these discussions, at least four developers
(Katsumi, Miles, Romain and me) have expressed concerns about or
argued against these changes.  Beside yourself, nobody argued in favor
of those "by-the-way whitespace changes".

If also Larsi has said "Don't use whitespace.el or equivalent when
changing Gnus." (as Katsumi mentioned), isn't this sufficient to
convince you?

> Let me restate my position: I believe that the right thing to do is to
> have whitespace-cleanup on write-file-hooks for everybody committing to
> Gnus. 

I agree that we should _not introduce_ trailing whitespace.

> This is simple to achieve, this effectively removes any such
> problems and this makes the files [automatically] cleaner. There's
> also the question of the info files on which I only got a "I've
> heard that it is better this way" sort of message.[1]
>
> Sorry to be a little pushy on this, but whitespace.el functions on a
> major-mode basis and not on a file-name basis, so I would need some
> trickery to make it stop working on Gnus files, so I really want a
> discussion about this before.

You are kind of arguing that we should change our policy because of a
bug/missing feature in `whitespace.el'.  BTW, instead of...
(add-hook 'write-file-hooks #'(lambda () (whitespace-cleanup) nil))
... it should be simple enough to add check `default-directory' and
don't switch it on if it is not /foo/bar/gnus.

> Footnotes: 
> [1]  FWIW, texinfo-mode is on whitespace-modes by default, and I'd never
> heard any complaint about this until a few days ago.

As we already have discussed[*], it often makes the formatted info
files ugly (e.g. example lisp code).

Bye, Reiner.

[*] http://thread.gmane.org/gmane.emacs.gnus.general/64459
    http://thread.gmane.org/gmane.emacs.gnus.commits/3222/focus=60498
-- 
       ,,,
      (o o)
---ooO-(_)-Ooo---  |  PGP key available  |  http://rsteib.home.pages.de/