Re: [PATCH 2/2] diff --word-diff: use non-whitespace regex by default

Tay Ray Chuan <rctay89@gmail.com> writes:

Sorry for distracting you with the performance argument; it was mostly the first thing that came to my mind that I could use to ask for the motivation, and evaluation of tradeoffs, that both were missing from the proposed commit message.

In that case (and I should have read the original patch), I am definitely against this change. It turns the default word-diff into character-diff, which is something entirely different, and frequently useless precisely for the reason you state:

Case in point, consider my patch sent out yesterday

  http://article.gmane.org/gmane.comp.version-control.git/188391

It consists of a one-hunk doc update.  word-diff is not brilliant:

  -k::
          Usually the program [-'cleans up'-]{+removes email cruft from+} the Subject:
          header line to extract the title line for the commit log
          [-message,-]
  [-      among which (1) remove 'Re:' or 're:', (2) leading-]
  [-      whitespaces, (3) '[' up to ']', typically '[PATCH]', and-]
  [-      then prepends "[PATCH] ".-]{+message.+}  This [-flag forbids-]{+option prevents+} this munging, and is most
          useful when used to read back 'git format-patch -k' output.
[snip the rest as it's only {+}]

But character-diff tries too hard to find common subsequences:

  $ g show HEAD^^ --word-diff-regex='[^[:space:]]' | xsel
  -k::
          Usually the program [-'cl-]{+remov+}e[-an-]s {+email cr+}u[-p'-]{+ft from+} the Subject:
          header line to extract the title line for the commit log
          message[-,-]
  [-      among which (1) remove 'Re:' or 're:', (2) leading-]
  [-      w-]{+.  T+}hi[-te-]s[-paces, (3) '[' up t-] o[-']', ty-]p[-ically '[PATCH]', and-]t[-he-]{+io+}n pre[-p-]{+v+}en[-ds "[PATCH] ".  This flag forbid-]{+t+}s this munging, and is most
          useful when used to read back 'git format-patch -k' output.
[snip]

Wouldn't you agree that

  w-]{+.  T+}hi[-te-]s[-paces, (3) '[' up t-] o[-']', ty-]p[

is just line noise? The colors don't even help as most of it is removed (red).

Regarding your examples

first please notice that both of them were written before (and actually discussing) the introduction of the wordRegex feature. At this point, we were trying to make up our minds w.r.t. how powerful the feature needs to be. Nowadays (or in fact, starting a few days after those emails) the user can easily achieve everything discussed here by setting the wordRegex to taste.

That being said, I can see some arguments for changing the default to split punctuation into a separate word. That is, whereas the current default is semantically equivalent to a wordRegex of

  [^[:space:]]*

(but has a faster code path) and your proposal is equivalent to

  [^[:space:]]|UTF_8_GUARD

I think there is a case to be made for a default of

  [^[:space:]]|([[:alnum:]]|UTF_8_GUARD)+

or some such. There's a lot of bikeshedding lurking in the (non)extent of the [[:alnum:]] here, however.