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

From: Laurent Arnoud <laurent@spkdev.net>
To: Shourya Shukla <shouryashukla.oo@gmail.com>
Cc: congdanhqx@gmail.com, git@vger.kernel.org, gitster@pobox.com,
	phillip.wood@dunelm.org.uk, sandals@crustytoothpaste.net
Subject: Re: [PATCH v5] diff: add config option relative
Date: Mon, 18 May 2020 19:25:04 +0200	[thread overview]
Message-ID: <20200518172504.GB2110@spk-laptop> (raw)
In-Reply-To: <20200518114630.GA30622@konoha>

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