Re: [PATCH] diff.c: offer config option to control ws handling in move detection

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

 



On Wed, Jul 18, 2018 at 10:45 AM Junio C Hamano <gitster@xxxxxxxxx> wrote:
>
> Stefan Beller <sbeller@xxxxxxxxxx> writes:
>
> > diff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt
> > index 143acd9417e..8da7fed4e22 100644
> > --- a/Documentation/diff-options.txt
> > +++ b/Documentation/diff-options.txt
> > @@ -294,8 +294,11 @@ dimmed_zebra::
> >
> >  --color-moved-ws=<modes>::
> >       This configures how white spaces are ignored when performing the
> > -     move detection for `--color-moved`. These modes can be given
> > -     as a comma separated list:
> > +     move detection for `--color-moved`.
> > +ifdef::git-diff[]
> > +     It can be set by the `diff.colorMovedWS` configuration setting.
> > +endif::git-diff[]
>
> The patch to diff.c::git_diff_ui_config() we see below does not seem
> to make any effort to make sure that this new configuration applies
> only to "git diff" and that other commands like "git log" that call
> git_diff_ui_config() are not affected.

That is as intended. (We want to have it in git-log)

> And I do not see a strong reason why "git log --color-moved" should
> not honor this setting, either, so I am not quite sure why we want
> this ifdef/endif pair to hide it from "git log --help".
>
> Or am I totally misunderstanding the reason why we want ifdef/endif
> here?
>
> Puzzled...

I am somewhat puzzled, too, by the use of ifdefs in
Documentation/diff-options.txt, but I rationalized it as
"man git diff" has all the details, whereas the other pages are
a bit shorter and concise:

$ git grep -A 2 "ifdef::git-diff" Documentation/diff-options.txt
ifdef::git-diff[]
     This is the default.
endif::git-diff[]

ifdef::git-diff-core[]
     This is the default.
endif::git-diff-core[]

ifdef::git-diff[]
     It can be changed by the `color.ui` and `color.diff`
     configuration settings.

ifdef::git-diff[]
     This can be used to override configuration settings.
endif::git-diff[]

ifdef::git-diff[]
     It can be changed by the `diff.colorMoved` configuration setting.
endif::git-diff[]

ifdef::git-diff[]
     It can be set by the `diff.colorMovedWS` configuration setting.
endif::git-diff[]

So I followed the style that was already there, specifically
61e89eaae88 (diff: document the new --color-moved setting, 2017-06-30)
which followed the style of
6999c54029b (config.txt,diff-options.txt: porcelain vs. plumbing for
color.diff, 2011-04-27)

So I might have picked up a bad habit there or misunderstood the original?

Stefan



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux