Re: [PATCH v5] diff: add config option relative

[Laurent Arnoud <laurent@spkdev.net>]

Hi Shourya,

On Mon, May 18, 2020 at 05:16:30PM +0530, Shourya Shukla wrote:
> I think the message can be written better, maybe something like:
> 
>     The `diff.relative` boolean option set to `true` shows only changes
>     in the current directory/value specified by the `path` argument of
>     the `relative` option and shows pathnames relative to the
>     aforementioned directory.
> 
>     Teach `--no-relative` to override the earlier `--relative`

Thanks I've applied your suggestion to the v6 patch

> > --- a/Documentation/config/diff.txt
> > +++ b/Documentation/config/diff.txt
> > @@ -105,6 +105,10 @@ diff.mnemonicPrefix::
> >  diff.noprefix::
> >  	If set, 'git diff' does not show any source or destination prefix.
> > 
> > +diff.relative::
> > +	If set to 'true', 'git diff' does not show changes outside of the directory
> > +	and show pathnames relative to the current directory.
> 
> In the second last line, it would be better to write is as:
> 
>     ... does not show changes outside of the current directory/the path
>     provided as the argument in `relative`?
> 
> Even my version of the above line seems very crude and innaccurate, but
> I think that we should take into account the `path` passed down to us
> by the `--relative[=<path>]` option.
> 
> Moving on, what I wondered was that maybe making it a `true/false`
> option would be better? Something like:
> 
>     git diff --relative=false //override the diff.relative setting
>     git diff --relative=true <path> //works like the usual `relative`
> 
> And by the usual, I mean :
>     git diff --relative[=<path>]
> 
> What do you think? I think it would be better than adding a new option.

I think this would be complicated to check if true/false is a directory of not;
it has been mentioned by Junio on another thread for a patch alike

Cheers

-- 
Laurent