git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] diff: add config option relative
@ 2020-05-15 15:57 Laurent Arnoud
  2020-05-15 22:22 ` Philip Oakley
  2020-05-15 23:31 ` brian m. carlson
  0 siblings, 2 replies; 39+ messages in thread
From: Laurent Arnoud @ 2020-05-15 15:57 UTC (permalink / raw)
  To: git

The `diff.relative` boolean option set to `true` to show only changes on the
current directory and show relative pathnames.

Signed-off-by: Laurent Arnoud <laurent@spkdev.net>
---
 Documentation/config/diff.txt   |  4 +++
 diff.c                          |  7 +++++
 t/t9904-diff-relative-config.sh | 48 +++++++++++++++++++++++++++++++++
 3 files changed, 59 insertions(+)
 create mode 100755 t/t9904-diff-relative-config.sh

diff --git Documentation/config/diff.txt Documentation/config/diff.txt
index ff09f1cf73..1d311358d8 100644
--- Documentation/config/diff.txt
+++ 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.
+
 diff.orderFile::
 	File indicating how to order files within a diff.
 	See the '-O' option to linkgit:git-diff[1] for details.
diff --git diff.c diff.c
index d1ad6a3c4a..24b7a0ae08 100644
--- diff.c
+++ diff.c
@@ -48,6 +48,7 @@ static const char *diff_order_file_cfg;
 int diff_auto_refresh_index = 1;
 static int diff_mnemonic_prefix;
 static int diff_no_prefix;
+static int diff_relative;
 static int diff_stat_graph_width;
 static int diff_dirstat_permille_default = 30;
 static struct diff_options default_diff_options;
@@ -386,6 +387,10 @@ int git_diff_ui_config(const char *var, const char *value, void *cb)
 		diff_no_prefix = git_config_bool(var, value);
 		return 0;
 	}
+	if (!strcmp(var, "diff.relative")) {
+		diff_relative = git_config_bool(var, value);
+		return 0;
+	}
 	if (!strcmp(var, "diff.statgraphwidth")) {
 		diff_stat_graph_width = git_config_int(var, value);
 		return 0;
@@ -4558,6 +4563,8 @@ void repo_diff_setup(struct repository *r, struct diff_options *options)
 		options->b_prefix = "b/";
 	}
 
+	options->flags.relative_name = diff_relative;
+
 	options->color_moved = diff_color_moved_default;
 	options->color_moved_ws_handling = diff_color_moved_ws_default;
 
diff --git t/t9904-diff-relative-config.sh t/t9904-diff-relative-config.sh
new file mode 100755
index 0000000000..a92d53ca25
--- /dev/null
+++ t/t9904-diff-relative-config.sh
@@ -0,0 +1,48 @@
+#!/bin/sh
+
+test_description='config diff.relative'
+
+. ./test-lib.sh
+
+test_expect_success 'setup' '
+	git commit --allow-empty -m empty &&
+	echo content >file1 &&
+	mkdir subdir &&
+	echo other content >subdir/file2 &&
+	git add . &&
+	git commit -m one
+'
+
+check_diff () {
+	dir=$1
+	shift
+	expect=$1
+	shift
+	relative_opt=$1
+	shift
+	short_blob=$(git rev-parse --short "$(git hash-object subdir/file2)")
+	cat >expected <<-EOF
+	diff --git a/$expect b/$expect
+	new file mode 100644
+	index 0000000..$short_blob
+	--- /dev/null
+	+++ b/$expect
+	@@ -0,0 +1 @@
+	+other content
+	EOF
+	test_expect_success "-p $*" "
+		test_config -C $dir diff.relative $relative_opt &&
+		git -C '$dir' diff -p $* HEAD^ >actual &&
+		test_cmp expected actual
+	"
+}
+
+check_diff . file2 false --relative=subdir/
+check_diff . file2 false --relative=subdir
+check_diff . file2 true --relative=subdir/
+check_diff . file2 true --relative=subdir
+check_diff subdir file2 false --relative
+check_diff subdir file2 true --relative
+check_diff subdir file2 true
+
+test_done
-- 
2.26.2

^ permalink raw reply related	[flat|nested] 39+ messages in thread

* Re: [PATCH] diff: add config option relative
  2020-05-15 15:57 [PATCH] diff: add config option relative Laurent Arnoud
@ 2020-05-15 22:22 ` Philip Oakley
  2020-05-16 17:30   ` Laurent Arnoud
  2020-05-15 23:31 ` brian m. carlson
  1 sibling, 1 reply; 39+ messages in thread
From: Philip Oakley @ 2020-05-15 22:22 UTC (permalink / raw)
  To: Laurent Arnoud, git

Hi Laurent,

On 15/05/2020 16:57, Laurent Arnoud wrote:
> The `diff.relative` boolean option set to `true` to show only changes on the
> current directory and show relative pathnames.
>
> Signed-off-by: Laurent Arnoud <laurent@spkdev.net>
> ---
>  Documentation/config/diff.txt   |  4 +++
>  diff.c                          |  7 +++++
>  t/t9904-diff-relative-config.sh | 48 +++++++++++++++++++++++++++++++++
>  3 files changed, 59 insertions(+)
>  create mode 100755 t/t9904-diff-relative-config.sh
>
> diff --git Documentation/config/diff.txt Documentation/config/diff.txt
> index ff09f1cf73..1d311358d8 100644
> --- Documentation/config/diff.txt
> +++ 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.

This last part sounded stilted. Maybe "and shows pathnames relative to
the current directory", assuming I've understood what it was meant to
say (and possible the same change in the commit message)

Philip
> +
>  diff.orderFile::
>  	File indicating how to order files within a diff.
>  	See the '-O' option to linkgit:git-diff[1] for details.
> diff --git diff.c diff.c
> index d1ad6a3c4a..24b7a0ae08 100644
> --- diff.c
> +++ diff.c
> @@ -48,6 +48,7 @@ static const char *diff_order_file_cfg;
>  int diff_auto_refresh_index = 1;
>  static int diff_mnemonic_prefix;
>  static int diff_no_prefix;
> +static int diff_relative;
>  static int diff_stat_graph_width;
>  static int diff_dirstat_permille_default = 30;
>  static struct diff_options default_diff_options;
> @@ -386,6 +387,10 @@ int git_diff_ui_config(const char *var, const char *value, void *cb)
>  		diff_no_prefix = git_config_bool(var, value);
>  		return 0;
>  	}
> +	if (!strcmp(var, "diff.relative")) {
> +		diff_relative = git_config_bool(var, value);
> +		return 0;
> +	}
>  	if (!strcmp(var, "diff.statgraphwidth")) {
>  		diff_stat_graph_width = git_config_int(var, value);
>  		return 0;
> @@ -4558,6 +4563,8 @@ void repo_diff_setup(struct repository *r, struct diff_options *options)
>  		options->b_prefix = "b/";
>  	}
>  
> +	options->flags.relative_name = diff_relative;
> +
>  	options->color_moved = diff_color_moved_default;
>  	options->color_moved_ws_handling = diff_color_moved_ws_default;
>  
> diff --git t/t9904-diff-relative-config.sh t/t9904-diff-relative-config.sh
> new file mode 100755
> index 0000000000..a92d53ca25
> --- /dev/null
> +++ t/t9904-diff-relative-config.sh
> @@ -0,0 +1,48 @@
> +#!/bin/sh
> +
> +test_description='config diff.relative'
> +
> +. ./test-lib.sh
> +
> +test_expect_success 'setup' '
> +	git commit --allow-empty -m empty &&
> +	echo content >file1 &&
> +	mkdir subdir &&
> +	echo other content >subdir/file2 &&
> +	git add . &&
> +	git commit -m one
> +'
> +
> +check_diff () {
> +	dir=$1
> +	shift
> +	expect=$1
> +	shift
> +	relative_opt=$1
> +	shift
> +	short_blob=$(git rev-parse --short "$(git hash-object subdir/file2)")
> +	cat >expected <<-EOF
> +	diff --git a/$expect b/$expect
> +	new file mode 100644
> +	index 0000000..$short_blob
> +	--- /dev/null
> +	+++ b/$expect
> +	@@ -0,0 +1 @@
> +	+other content
> +	EOF
> +	test_expect_success "-p $*" "
> +		test_config -C $dir diff.relative $relative_opt &&
> +		git -C '$dir' diff -p $* HEAD^ >actual &&
> +		test_cmp expected actual
> +	"
> +}
> +
> +check_diff . file2 false --relative=subdir/
> +check_diff . file2 false --relative=subdir
> +check_diff . file2 true --relative=subdir/
> +check_diff . file2 true --relative=subdir
> +check_diff subdir file2 false --relative
> +check_diff subdir file2 true --relative
> +check_diff subdir file2 true
> +
> +test_done


^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [PATCH] diff: add config option relative
  2020-05-15 15:57 [PATCH] diff: add config option relative Laurent Arnoud
  2020-05-15 22:22 ` Philip Oakley
@ 2020-05-15 23:31 ` brian m. carlson
  2020-05-16  0:04   ` Junio C Hamano
  1 sibling, 1 reply; 39+ messages in thread
From: brian m. carlson @ 2020-05-15 23:31 UTC (permalink / raw)
  To: Laurent Arnoud; +Cc: git

[-- Attachment #1: Type: text/plain, Size: 538 bytes --]

On 2020-05-15 at 15:57:06, Laurent Arnoud wrote:
> The `diff.relative` boolean option set to `true` to show only changes on the
> current directory and show relative pathnames.

Usually when we implement configuration settings like this, we implement
an option value, such as --no-relative, so that users or scripting tools
can disable this feature if they need to.  However, I don't see that in
this series.  Would adding such a feature be possible?
-- 
brian m. carlson: Houston, Texas, US
OpenPGP: https://keybase.io/bk2204

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 263 bytes --]

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [PATCH] diff: add config option relative
  2020-05-15 23:31 ` brian m. carlson
@ 2020-05-16  0:04   ` Junio C Hamano
  2020-05-16 17:35     ` Laurent Arnoud
  2020-05-16 17:38     ` [PATCH v3] " Laurent Arnoud
  0 siblings, 2 replies; 39+ messages in thread
From: Junio C Hamano @ 2020-05-16  0:04 UTC (permalink / raw)
  To: brian m. carlson; +Cc: Laurent Arnoud, git

"brian m. carlson" <sandals@crustytoothpaste.net> writes:

> On 2020-05-15 at 15:57:06, Laurent Arnoud wrote:
>> The `diff.relative` boolean option set to `true` to show only changes on the
>> current directory and show relative pathnames.
>
> Usually when we implement configuration settings like this, we implement
> an option value, such as --no-relative, so that users or scripting tools
> can disable this feature if they need to.  However, I don't see that in
> this series.  Would adding such a feature be possible?

I think I saw a variant that does have --[no-]foobar support on the
list, but I may be hallucinating.  You definitely would want it, or
you've broken "format-patch" forever without a knob to countermand
the configuration (well, you can still say "git -c diff.foobar=no"
but that's kind of cheating).


^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [PATCH] diff: add config option relative
  2020-05-15 22:22 ` Philip Oakley
@ 2020-05-16 17:30   ` Laurent Arnoud
  0 siblings, 0 replies; 39+ messages in thread
From: Laurent Arnoud @ 2020-05-16 17:30 UTC (permalink / raw)
  To: Philip Oakley; +Cc: git

Hi Philip,

On Fri, May 15, 2020 at 11:22:53PM +0100, Philip Oakley wrote:
> On 15/05/2020 16:57, Laurent Arnoud wrote:
> > +diff.relative::
> > +	If set to "true", 'git diff' does not show changes outside of the directory
> > +	and show pathnames relative.
> 
> This last part sounded stilted. Maybe "and shows pathnames relative to
> the current directory", assuming I've understood what it was meant to
> say (and possible the same change in the commit message)

Yes that what it was meant by this, I will submit a patch with your suggestion

Cheers

-- 
Laurent

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [PATCH] diff: add config option relative
  2020-05-16  0:04   ` Junio C Hamano
@ 2020-05-16 17:35     ` Laurent Arnoud
  2020-05-16 17:38     ` [PATCH v3] " Laurent Arnoud
  1 sibling, 0 replies; 39+ messages in thread
From: Laurent Arnoud @ 2020-05-16 17:35 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: brian m. carlson, git

On Fri, May 15, 2020 at 05:04:43PM -0700, Junio C Hamano wrote:
> "brian m. carlson" <sandals@crustytoothpaste.net> writes:
> 
> > On 2020-05-15 at 15:57:06, Laurent Arnoud wrote:
> >> The `diff.relative` boolean option set to `true` to show only changes on the
> >> current directory and show relative pathnames.
> >
> > Usually when we implement configuration settings like this, we implement
> > an option value, such as --no-relative, so that users or scripting tools
> > can disable this feature if they need to.  However, I don't see that in
> > this series.  Would adding such a feature be possible?
> 
> I think I saw a variant that does have --[no-]foobar support on the
> list, but I may be hallucinating.  You definitely would want it, or
> you've broken "format-patch" forever without a knob to countermand
> the configuration (well, you can still say "git -c diff.foobar=no"
> but that's kind of cheating).

Yes I've submitted a v2 patch with the `--no-relative` I will send a v3 to this
thread for review

-- 
Laurent

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [PATCH v3] diff: add config option relative
  2020-05-16  0:04   ` Junio C Hamano
  2020-05-16 17:35     ` Laurent Arnoud
@ 2020-05-16 17:38     ` Laurent Arnoud
  2020-05-16 18:33       ` Phillip Wood
  1 sibling, 1 reply; 39+ messages in thread
From: Laurent Arnoud @ 2020-05-16 17:38 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: brian m. carlson, git

The `diff.relative` boolean option set to `true` show only changes on
the current directory and show relative pathnames to the current
directory.

Teach --no-relative to override earlier --relative

Signed-off-by: Laurent Arnoud <laurent@spkdev.net>
---
 Documentation/config/diff.txt   |  4 ++
 Documentation/diff-options.txt  |  2 +
 diff.c                          | 10 ++++
 t/t9904-diff-relative-config.sh | 93 +++++++++++++++++++++++++++++++++
 4 files changed, 109 insertions(+)
 create mode 100755 t/t9904-diff-relative-config.sh

diff --git a/Documentation/config/diff.txt b/Documentation/config/diff.txt
index ff09f1cf73..76537c3b0c 100644
--- 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.
+
 diff.orderFile::
 	File indicating how to order files within a diff.
 	See the '-O' option to linkgit:git-diff[1] for details.
diff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt
index bb31f0c42b..fdc5aefa37 100644
--- a/Documentation/diff-options.txt
+++ b/Documentation/diff-options.txt
@@ -651,6 +651,8 @@ ifndef::git-format-patch[]
 	not in a subdirectory (e.g. in a bare repository), you
 	can name which subdirectory to make the output relative
 	to by giving a <path> as an argument.
+--no-relative::
+	Turn off relative pathnames and include all changes in the repository.
 endif::git-format-patch[]

 -a::
diff --git a/diff.c b/diff.c
index d1ad6a3c4a..c4dcf01ec0 100644
--- a/diff.c
+++ b/diff.c
@@ -48,6 +48,7 @@ static const char *diff_order_file_cfg;
 int diff_auto_refresh_index = 1;
 static int diff_mnemonic_prefix;
 static int diff_no_prefix;
+static int diff_relative;
 static int diff_stat_graph_width;
 static int diff_dirstat_permille_default = 30;
 static struct diff_options default_diff_options;
@@ -386,6 +387,10 @@ int git_diff_ui_config(const char *var, const char *value, void *cb)
 		diff_no_prefix = git_config_bool(var, value);
 		return 0;
 	}
+	if (!strcmp(var, "diff.relative")) {
+		diff_relative = git_config_bool(var, value);
+		return 0;
+	}
 	if (!strcmp(var, "diff.statgraphwidth")) {
 		diff_stat_graph_width = git_config_int(var, value);
 		return 0;
@@ -4558,6 +4563,8 @@ void repo_diff_setup(struct repository *r, struct diff_options *options)
 		options->b_prefix = "b/";
 	}

+	options->flags.relative_name = diff_relative;
+
 	options->color_moved = diff_color_moved_default;
 	options->color_moved_ws_handling = diff_color_moved_ws_default;

@@ -5494,6 +5501,9 @@ static void prep_parse_options(struct diff_options *options)
 			       N_("when run from subdir, exclude changes outside and show relative paths"),
 			       PARSE_OPT_NONEG | PARSE_OPT_OPTARG,
 			       diff_opt_relative),
+		OPT_SET_INT_F(0, "no-relative", &options->flags.relative_name,
+			      N_("disable diff.relative config option"),
+			      0, PARSE_OPT_NONEG),
 		OPT_BOOL('a', "text", &options->flags.text,
 			 N_("treat all files as text")),
 		OPT_BOOL('R', NULL, &options->flags.reverse_diff,
diff --git a/t/t9904-diff-relative-config.sh b/t/t9904-diff-relative-config.sh
new file mode 100755
index 0000000000..23ab1af5e0
--- /dev/null
+++ b/t/t9904-diff-relative-config.sh
@@ -0,0 +1,93 @@
+#!/bin/sh
+
+test_description='config diff.relative'
+
+. ./test-lib.sh
+
+test_expect_success 'setup' '
+	git commit --allow-empty -m empty &&
+	echo content >file1 &&
+	mkdir subdir &&
+	echo other content >subdir/file2 &&
+	git add . &&
+	git commit -m one
+'
+
+check_diff_relative () {
+	dir=$1
+	shift
+	expect=$1
+	shift
+	relative_opt=$1
+	shift
+	short_blob=$(git rev-parse --short "$(git hash-object subdir/file2)")
+	cat >expected <<-EOF
+	diff --git a/$expect b/$expect
+	new file mode 100644
+	index 0000000..$short_blob
+	--- /dev/null
+	+++ b/$expect
+	@@ -0,0 +1 @@
+	+other content
+	EOF
+	test_expect_success "config.relative $relative_opt -p $*" "
+		test_config -C $dir diff.relative $relative_opt &&
+		git -C '$dir' diff -p $* HEAD^ >actual &&
+		test_cmp expected actual
+	"
+}
+
+check_diff_no_relative () {
+	dir=$1
+	shift
+	expect=$1
+	shift
+	relative_opt=$1
+	shift
+	short_blob_file1=$(git rev-parse --short "$(git hash-object file1)")
+	short_blob_file2=$(git rev-parse --short "$(git hash-object subdir/file2)")
+	cat >expected <<-EOF
+	diff --git a/file1 b/file1
+	new file mode 100644
+	index 0000000..$short_blob_file1
+	--- /dev/null
+	+++ b/file1
+	@@ -0,0 +1 @@
+	+content
+	diff --git a/$expect b/$expect
+	new file mode 100644
+	index 0000000..$short_blob_file2
+	--- /dev/null
+	+++ b/$expect
+	@@ -0,0 +1 @@
+	+other content
+	EOF
+	cat expected
+	test_expect_success "config.relative $relative_opt -p $*" "
+		test_config -C $dir diff.relative $relative_opt &&
+		git -C '$dir' diff -p $* HEAD^ >actual &&
+		git -C '$dir' diff -p $* HEAD^ >/tmp/actual &&
+		test_cmp expected actual
+	"
+}
+
+check_diff_relative . file2 false --relative=subdir/
+check_diff_relative . file2 false --relative=subdir
+check_diff_relative . file2 true --relative=subdir/
+check_diff_relative . file2 true --relative=subdir
+check_diff_relative subdir file2 false --relative
+check_diff_relative subdir file2 true --relative
+check_diff_relative subdir file2 true
+check_diff_relative subdir file2 false --no-relative --relative
+check_diff_relative subdir file2 true --no-relative --relative
+check_diff_relative . file2 false --no-relative --relative=subdir
+check_diff_relative . file2 true --no-relative --relative=subdir
+
+check_diff_no_relative . subdir/file2 false
+check_diff_no_relative . subdir/file2 true --no-relative
+check_diff_no_relative . subdir/file2 false --no-relative
+check_diff_no_relative subdir subdir/file2 false
+check_diff_no_relative subdir subdir/file2 true --no-relative
+check_diff_no_relative subdir subdir/file2 false --no-relative
+
+test_done
--
2.26.2

^ permalink raw reply related	[flat|nested] 39+ messages in thread

* Re: [PATCH v3] diff: add config option relative
  2020-05-16 17:38     ` [PATCH v3] " Laurent Arnoud
@ 2020-05-16 18:33       ` Phillip Wood
  2020-05-16 19:40         ` [PATCH v4] " Laurent Arnoud
  2020-05-17 15:07         ` [PATCH v3] " Junio C Hamano
  0 siblings, 2 replies; 39+ messages in thread
From: Phillip Wood @ 2020-05-16 18:33 UTC (permalink / raw)
  To: Laurent Arnoud, Junio C Hamano; +Cc: brian m. carlson, git

Hi Laurent

On 16/05/2020 18:38, Laurent Arnoud wrote:
> The `diff.relative` boolean option set to `true` show only changes on
> the current directory and show relative pathnames to the current
> directory.
> 
> Teach --no-relative to override earlier --relative
> 
> Signed-off-by: Laurent Arnoud <laurent@spkdev.net>
> ---
>   Documentation/config/diff.txt   |  4 ++
>   Documentation/diff-options.txt  |  2 +
>   diff.c                          | 10 ++++
>   t/t9904-diff-relative-config.sh | 93 +++++++++++++++++++++++++++++++++
>   4 files changed, 109 insertions(+)
>   create mode 100755 t/t9904-diff-relative-config.sh
> 
> diff --git a/Documentation/config/diff.txt b/Documentation/config/diff.txt
> index ff09f1cf73..76537c3b0c 100644
> --- 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.
> +
>   diff.orderFile::
>   	File indicating how to order files within a diff.
>   	See the '-O' option to linkgit:git-diff[1] for details.
> diff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt
> index bb31f0c42b..fdc5aefa37 100644
> --- a/Documentation/diff-options.txt
> +++ b/Documentation/diff-options.txt
> @@ -651,6 +651,8 @@ ifndef::git-format-patch[]
>   	not in a subdirectory (e.g. in a bare repository), you
>   	can name which subdirectory to make the output relative
>   	to by giving a <path> as an argument.
> +--no-relative::
> +	Turn off relative pathnames and include all changes in the repository.

It might be helpful to mention that this is the default and this option 
exists to override the config setting

>   endif::git-format-patch[]
> 
>   -a::
> diff --git a/diff.c b/diff.c
> index d1ad6a3c4a..c4dcf01ec0 100644
> --- a/diff.c
> +++ b/diff.c
> @@ -48,6 +48,7 @@ static const char *diff_order_file_cfg;
>   int diff_auto_refresh_index = 1;
>   static int diff_mnemonic_prefix;
>   static int diff_no_prefix;
> +static int diff_relative;
>   static int diff_stat_graph_width;
>   static int diff_dirstat_permille_default = 30;
>   static struct diff_options default_diff_options;
> @@ -386,6 +387,10 @@ int git_diff_ui_config(const char *var, const char *value, void *cb)
>   		diff_no_prefix = git_config_bool(var, value);
>   		return 0;
>   	}
> +	if (!strcmp(var, "diff.relative")) {
> +		diff_relative = git_config_bool(var, value);
> +		return 0;
> +	}
>   	if (!strcmp(var, "diff.statgraphwidth")) {
>   		diff_stat_graph_width = git_config_int(var, value);
>   		return 0;
> @@ -4558,6 +4563,8 @@ void repo_diff_setup(struct repository *r, struct diff_options *options)
>   		options->b_prefix = "b/";
>   	}
> 
> +	options->flags.relative_name = diff_relative;
> +
>   	options->color_moved = diff_color_moved_default;
>   	options->color_moved_ws_handling = diff_color_moved_ws_default;
> 
> @@ -5494,6 +5501,9 @@ static void prep_parse_options(struct diff_options *options)
>   			       N_("when run from subdir, exclude changes outside and show relative paths"),
>   			       PARSE_OPT_NONEG | PARSE_OPT_OPTARG,
>   			       diff_opt_relative),
> +		OPT_SET_INT_F(0, "no-relative", &options->flags.relative_name,
> +			      N_("disable diff.relative config option"),
> +			      0, PARSE_OPT_NONEG),

Rather than adding a new option, it would be better to modify the 
existing --relative option. If you remove PARSE_OPT_NONEG from the 
declaration above it will support --no-relative. You will also need to 
modify diff_opt_relative() to handle clearing the option.

Best Wishes

Phillip

>   		OPT_BOOL('a', "text", &options->flags.text,
>   			 N_("treat all files as text")),
>   		OPT_BOOL('R', NULL, &options->flags.reverse_diff,
> diff --git a/t/t9904-diff-relative-config.sh b/t/t9904-diff-relative-config.sh
> new file mode 100755
> index 0000000000..23ab1af5e0
> --- /dev/null
> +++ b/t/t9904-diff-relative-config.sh
> @@ -0,0 +1,93 @@
> +#!/bin/sh
> +
> +test_description='config diff.relative'
> +
> +. ./test-lib.sh
> +
> +test_expect_success 'setup' '
> +	git commit --allow-empty -m empty &&
> +	echo content >file1 &&
> +	mkdir subdir &&
> +	echo other content >subdir/file2 &&
> +	git add . &&
> +	git commit -m one
> +'
> +
> +check_diff_relative () {
> +	dir=$1
> +	shift
> +	expect=$1
> +	shift
> +	relative_opt=$1
> +	shift
> +	short_blob=$(git rev-parse --short "$(git hash-object subdir/file2)")
> +	cat >expected <<-EOF
> +	diff --git a/$expect b/$expect
> +	new file mode 100644
> +	index 0000000..$short_blob
> +	--- /dev/null
> +	+++ b/$expect
> +	@@ -0,0 +1 @@
> +	+other content
> +	EOF
> +	test_expect_success "config.relative $relative_opt -p $*" "
> +		test_config -C $dir diff.relative $relative_opt &&
> +		git -C '$dir' diff -p $* HEAD^ >actual &&
> +		test_cmp expected actual
> +	"
> +}
> +
> +check_diff_no_relative () {
> +	dir=$1
> +	shift
> +	expect=$1
> +	shift
> +	relative_opt=$1
> +	shift
> +	short_blob_file1=$(git rev-parse --short "$(git hash-object file1)")
> +	short_blob_file2=$(git rev-parse --short "$(git hash-object subdir/file2)")
> +	cat >expected <<-EOF
> +	diff --git a/file1 b/file1
> +	new file mode 100644
> +	index 0000000..$short_blob_file1
> +	--- /dev/null
> +	+++ b/file1
> +	@@ -0,0 +1 @@
> +	+content
> +	diff --git a/$expect b/$expect
> +	new file mode 100644
> +	index 0000000..$short_blob_file2
> +	--- /dev/null
> +	+++ b/$expect
> +	@@ -0,0 +1 @@
> +	+other content
> +	EOF
> +	cat expected
> +	test_expect_success "config.relative $relative_opt -p $*" "
> +		test_config -C $dir diff.relative $relative_opt &&
> +		git -C '$dir' diff -p $* HEAD^ >actual &&
> +		git -C '$dir' diff -p $* HEAD^ >/tmp/actual &&
> +		test_cmp expected actual
> +	"
> +}
> +
> +check_diff_relative . file2 false --relative=subdir/
> +check_diff_relative . file2 false --relative=subdir
> +check_diff_relative . file2 true --relative=subdir/
> +check_diff_relative . file2 true --relative=subdir
> +check_diff_relative subdir file2 false --relative
> +check_diff_relative subdir file2 true --relative
> +check_diff_relative subdir file2 true
> +check_diff_relative subdir file2 false --no-relative --relative
> +check_diff_relative subdir file2 true --no-relative --relative
> +check_diff_relative . file2 false --no-relative --relative=subdir
> +check_diff_relative . file2 true --no-relative --relative=subdir
> +
> +check_diff_no_relative . subdir/file2 false
> +check_diff_no_relative . subdir/file2 true --no-relative
> +check_diff_no_relative . subdir/file2 false --no-relative
> +check_diff_no_relative subdir subdir/file2 false
> +check_diff_no_relative subdir subdir/file2 true --no-relative
> +check_diff_no_relative subdir subdir/file2 false --no-relative
> +
> +test_done
> --
> 2.26.2
> 

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [PATCH v4] diff: add config option relative
  2020-05-16 18:33       ` Phillip Wood
@ 2020-05-16 19:40         ` Laurent Arnoud
  2020-05-17  2:14           ` Đoàn Trần Công Danh
  2020-05-17 15:07         ` [PATCH v3] " Junio C Hamano
  1 sibling, 1 reply; 39+ messages in thread
From: Laurent Arnoud @ 2020-05-16 19:40 UTC (permalink / raw)
  To: phillip.wood; +Cc: Junio C Hamano, brian m. carlson, git

The `diff.relative` boolean option set to `true` show only changes on
the current directory and show relative pathnames to the current
directory.

Teach --no-relative to override earlier --relative

Signed-off-by: Laurent Arnoud <laurent@spkdev.net>
---
 Documentation/config/diff.txt   |  4 ++
 Documentation/diff-options.txt  |  3 ++
 diff.c                          | 16 ++++--
 t/t9904-diff-relative-config.sh | 93 +++++++++++++++++++++++++++++++++
 4 files changed, 113 insertions(+), 3 deletions(-)
 create mode 100755 t/t9904-diff-relative-config.sh

diff --git a/Documentation/config/diff.txt b/Documentation/config/diff.txt
index ff09f1cf73..76537c3b0c 100644
--- 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.
+
 diff.orderFile::
 	File indicating how to order files within a diff.
 	See the '-O' option to linkgit:git-diff[1] for details.
diff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt
index bb31f0c42b..1b279904eb 100644
--- a/Documentation/diff-options.txt
+++ b/Documentation/diff-options.txt
@@ -651,6 +651,9 @@ ifndef::git-format-patch[]
 	not in a subdirectory (e.g. in a bare repository), you
 	can name which subdirectory to make the output relative
 	to by giving a <path> as an argument.
+--no-relative::
+	Turn off relative pathnames and include all changes in the repository.
+	This can be used to override configuration settings `diff.relative`.
 endif::git-format-patch[]

 -a::
diff --git a/diff.c b/diff.c
index d1ad6a3c4a..af28e39785 100644
--- a/diff.c
+++ b/diff.c
@@ -48,6 +48,7 @@ static const char *diff_order_file_cfg;
 int diff_auto_refresh_index = 1;
 static int diff_mnemonic_prefix;
 static int diff_no_prefix;
+static int diff_relative;
 static int diff_stat_graph_width;
 static int diff_dirstat_permille_default = 30;
 static struct diff_options default_diff_options;
@@ -386,6 +387,10 @@ int git_diff_ui_config(const char *var, const char *value, void *cb)
 		diff_no_prefix = git_config_bool(var, value);
 		return 0;
 	}
+	if (!strcmp(var, "diff.relative")) {
+		diff_relative = git_config_bool(var, value);
+		return 0;
+	}
 	if (!strcmp(var, "diff.statgraphwidth")) {
 		diff_stat_graph_width = git_config_int(var, value);
 		return 0;
@@ -4558,6 +4563,8 @@ void repo_diff_setup(struct repository *r, struct diff_options *options)
 		options->b_prefix = "b/";
 	}

+	options->flags.relative_name = diff_relative;
+
 	options->color_moved = diff_color_moved_default;
 	options->color_moved_ws_handling = diff_color_moved_ws_default;

@@ -5195,8 +5202,11 @@ static int diff_opt_relative(const struct option *opt,
 {
 	struct diff_options *options = opt->value;

-	BUG_ON_OPT_NEG(unset);
-	options->flags.relative_name = 1;
+	if (unset) {
+		options->flags.relative_name = 0;
+	} else {
+		options->flags.relative_name = 1;
+	}
 	if (arg)
 		options->prefix = arg;
 	return 0;
@@ -5492,7 +5502,7 @@ static void prep_parse_options(struct diff_options *options)
 		OPT_GROUP(N_("Other diff options")),
 		OPT_CALLBACK_F(0, "relative", options, N_("<prefix>"),
 			       N_("when run from subdir, exclude changes outside and show relative paths"),
-			       PARSE_OPT_NONEG | PARSE_OPT_OPTARG,
+			       PARSE_OPT_OPTARG,
 			       diff_opt_relative),
 		OPT_BOOL('a', "text", &options->flags.text,
 			 N_("treat all files as text")),
diff --git a/t/t9904-diff-relative-config.sh b/t/t9904-diff-relative-config.sh
new file mode 100755
index 0000000000..23ab1af5e0
--- /dev/null
+++ b/t/t9904-diff-relative-config.sh
@@ -0,0 +1,93 @@
+#!/bin/sh
+
+test_description='config diff.relative'
+
+. ./test-lib.sh
+
+test_expect_success 'setup' '
+	git commit --allow-empty -m empty &&
+	echo content >file1 &&
+	mkdir subdir &&
+	echo other content >subdir/file2 &&
+	git add . &&
+	git commit -m one
+'
+
+check_diff_relative () {
+	dir=$1
+	shift
+	expect=$1
+	shift
+	relative_opt=$1
+	shift
+	short_blob=$(git rev-parse --short "$(git hash-object subdir/file2)")
+	cat >expected <<-EOF
+	diff --git a/$expect b/$expect
+	new file mode 100644
+	index 0000000..$short_blob
+	--- /dev/null
+	+++ b/$expect
+	@@ -0,0 +1 @@
+	+other content
+	EOF
+	test_expect_success "config.relative $relative_opt -p $*" "
+		test_config -C $dir diff.relative $relative_opt &&
+		git -C '$dir' diff -p $* HEAD^ >actual &&
+		test_cmp expected actual
+	"
+}
+
+check_diff_no_relative () {
+	dir=$1
+	shift
+	expect=$1
+	shift
+	relative_opt=$1
+	shift
+	short_blob_file1=$(git rev-parse --short "$(git hash-object file1)")
+	short_blob_file2=$(git rev-parse --short "$(git hash-object subdir/file2)")
+	cat >expected <<-EOF
+	diff --git a/file1 b/file1
+	new file mode 100644
+	index 0000000..$short_blob_file1
+	--- /dev/null
+	+++ b/file1
+	@@ -0,0 +1 @@
+	+content
+	diff --git a/$expect b/$expect
+	new file mode 100644
+	index 0000000..$short_blob_file2
+	--- /dev/null
+	+++ b/$expect
+	@@ -0,0 +1 @@
+	+other content
+	EOF
+	cat expected
+	test_expect_success "config.relative $relative_opt -p $*" "
+		test_config -C $dir diff.relative $relative_opt &&
+		git -C '$dir' diff -p $* HEAD^ >actual &&
+		git -C '$dir' diff -p $* HEAD^ >/tmp/actual &&
+		test_cmp expected actual
+	"
+}
+
+check_diff_relative . file2 false --relative=subdir/
+check_diff_relative . file2 false --relative=subdir
+check_diff_relative . file2 true --relative=subdir/
+check_diff_relative . file2 true --relative=subdir
+check_diff_relative subdir file2 false --relative
+check_diff_relative subdir file2 true --relative
+check_diff_relative subdir file2 true
+check_diff_relative subdir file2 false --no-relative --relative
+check_diff_relative subdir file2 true --no-relative --relative
+check_diff_relative . file2 false --no-relative --relative=subdir
+check_diff_relative . file2 true --no-relative --relative=subdir
+
+check_diff_no_relative . subdir/file2 false
+check_diff_no_relative . subdir/file2 true --no-relative
+check_diff_no_relative . subdir/file2 false --no-relative
+check_diff_no_relative subdir subdir/file2 false
+check_diff_no_relative subdir subdir/file2 true --no-relative
+check_diff_no_relative subdir subdir/file2 false --no-relative
+
+test_done
--
2.26.2

^ permalink raw reply related	[flat|nested] 39+ messages in thread

* Re: [PATCH v4] diff: add config option relative
  2020-05-16 19:40         ` [PATCH v4] " Laurent Arnoud
@ 2020-05-17  2:14           ` Đoàn Trần Công Danh
  2020-05-17  2:48             ` Danh Doan
                               ` (2 more replies)
  0 siblings, 3 replies; 39+ messages in thread
From: Đoàn Trần Công Danh @ 2020-05-17  2:14 UTC (permalink / raw)
  To: Laurent Arnoud; +Cc: phillip.wood, Junio C Hamano, brian m. carlson, git

Hi Laurent,

On 2020-05-16 21:40:33+0200, Laurent Arnoud <laurent@spkdev.net> wrote:
> Subject: Re: [PATCH v4] diff: add config option relative

I think the subject should be changed to.

	diff: allow overriding --relative

> The `diff.relative` boolean option set to `true` show only changes on
> the current directory and show relative pathnames to the current
> directory.
> 
> Teach --no-relative to override earlier --relative
> 
> Signed-off-by: Laurent Arnoud <laurent@spkdev.net>
> ---
>  Documentation/config/diff.txt   |  4 ++
>  Documentation/diff-options.txt  |  3 ++
>  diff.c                          | 16 ++++--
>  t/t9904-diff-relative-config.sh | 93 +++++++++++++++++++++++++++++++++

I think t99?? is used for miscellaneous tests.

To me, diff-relative things should be tested in t4045-diff-relative.sh

>  4 files changed, 113 insertions(+), 3 deletions(-)
>  create mode 100755 t/t9904-diff-relative-config.sh
> 
> diff --git a/Documentation/config/diff.txt b/Documentation/config/diff.txt
> index ff09f1cf73..76537c3b0c 100644
> --- 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

I think it's better to change "true" to either:
'true' (generated to <em>true</em>, or
`true` generated to <code>true<code>

Not sure which one is prefered, though.
This file uses both 2 styles.

I _think_ `true` is preferred.

> +	and show pathnames relative to the current directory.
> +
>  diff.orderFile::
>  	File indicating how to order files within a diff.
>  	See the '-O' option to linkgit:git-diff[1] for details.
> diff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt
> index bb31f0c42b..1b279904eb 100644
> --- a/Documentation/diff-options.txt
> +++ b/Documentation/diff-options.txt
> @@ -651,6 +651,9 @@ ifndef::git-format-patch[]
>  	not in a subdirectory (e.g. in a bare repository), you
>  	can name which subdirectory to make the output relative
>  	to by giving a <path> as an argument.
> +--no-relative::

Please merge this option with `--relative[=<path>]` above.
And says something likes:

	`--no-relative` can be used to countermand
	both `diff.relative` and previous `--relative`

> @@ -5195,8 +5202,11 @@ static int diff_opt_relative(const struct option *opt,
>  {
>  	struct diff_options *options = opt->value;
> 
> -	BUG_ON_OPT_NEG(unset);
> -	options->flags.relative_name = 1;
> +	if (unset) {
> +		options->flags.relative_name = 0;
> +	} else {
> +		options->flags.relative_name = 1;
> +	}

Can this block be simplified as:

	options->flags.relative_name = !unset;

> +check_diff_relative () {
> +	dir=$1
> +	shift
> +	expect=$1
> +	shift
> +	relative_opt=$1
> +	shift
> +	short_blob=$(git rev-parse --short "$(git hash-object subdir/file2)")

I think current practice want to have git hash-object as separated.
And all git-related code moved inside test_expect*

diff --git a/t/t9904-diff-relative-config.sh b/t/t9904-diff-relative-config.sh
index 23ab1af5e0..4747647917 100755
--- a/t/t9904-diff-relative-config.sh
+++ b/t/t9904-diff-relative-config.sh
@@ -20,17 +20,18 @@ check_diff_relative () {
 	shift
 	relative_opt=$1
 	shift
-	short_blob=$(git rev-parse --short "$(git hash-object subdir/file2)")
-	cat >expected <<-EOF
-	diff --git a/$expect b/$expect
-	new file mode 100644
-	index 0000000..$short_blob
-	--- /dev/null
-	+++ b/$expect
-	@@ -0,0 +1 @@
-	+other content
-	EOF
-	test_expect_success "config.relative $relative_opt -p $*" "
+	test_expect_success "config.relative $relative_opt -p $* in $dir" "
+		hash=\$(git hash-object subdir/file2) &&
+		short_blob=\$(git rev-parse --short \$hash) &&
+		cat >expected <<-EOF &&
+		diff --git a/$expect b/$expect
+		new file mode 100644
+		index 0000000..\$short_blob
+		--- /dev/null
+		+++ b/$expect
+		@@ -0,0 +1 @@
+		+other content
+		EOF
 		test_config -C $dir diff.relative $relative_opt &&
 		git -C '$dir' diff -p $* HEAD^ >actual &&
 		test_cmp expected actual

> +	short_blob_file1=$(git rev-parse --short "$(git hash-object file1)")
> +	short_blob_file2=$(git rev-parse --short "$(git hash-object subdir/file2)")
> +	cat >expected <<-EOF
> +	diff --git a/file1 b/file1
> +	new file mode 100644
> +	index 0000000..$short_blob_file1
> +	--- /dev/null
> +	+++ b/file1
> +	@@ -0,0 +1 @@
> +	+content
> +	diff --git a/$expect b/$expect
> +	new file mode 100644
> +	index 0000000..$short_blob_file2
> +	--- /dev/null
> +	+++ b/$expect
> +	@@ -0,0 +1 @@
> +	+other content
> +	EOF
> +	cat expected

Above comment also applied here.
And remove this debug cat.

-- 
Danh

^ permalink raw reply related	[flat|nested] 39+ messages in thread

* Re: [PATCH v4] diff: add config option relative
  2020-05-17  2:14           ` Đoàn Trần Công Danh
@ 2020-05-17  2:48             ` Danh Doan
  2020-05-17 15:12             ` Junio C Hamano
  2020-05-18  9:43             ` [PATCH v4] " Laurent Arnoud
  2 siblings, 0 replies; 39+ messages in thread
From: Danh Doan @ 2020-05-17  2:48 UTC (permalink / raw)
  To: Laurent Arnoud; +Cc: phillip.wood, Junio C Hamano, brian m. carlson, git



> On May 17, 2020, at 09:14, Đoàn Trần Công Danh <congdanhqx@gmail.com> wrote:
> 
> Hi Laurent,
> 
>> On 2020-05-16 21:40:33+0200, Laurent Arnoud <laurent@spkdev.net> wrote:
>> Subject: Re: [PATCH v4] diff: add config option relative
> 
> I think the subject should be changed to.
> 
>    diff: allow overriding --relative

I feel stupid about this now.
Please disregard my comment about subject.

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [PATCH v3] diff: add config option relative
  2020-05-16 18:33       ` Phillip Wood
  2020-05-16 19:40         ` [PATCH v4] " Laurent Arnoud
@ 2020-05-17 15:07         ` Junio C Hamano
  1 sibling, 0 replies; 39+ messages in thread
From: Junio C Hamano @ 2020-05-17 15:07 UTC (permalink / raw)
  To: Phillip Wood; +Cc: Laurent Arnoud, brian m. carlson, git

Phillip Wood <phillip.wood123@gmail.com> writes:

>> @@ -5494,6 +5501,9 @@ static void prep_parse_options(struct diff_options *options)
>>   			       N_("when run from subdir, exclude changes outside and show relative paths"),
>>   			       PARSE_OPT_NONEG | PARSE_OPT_OPTARG,
>>   			       diff_opt_relative),
>> +		OPT_SET_INT_F(0, "no-relative", &options->flags.relative_name,
>> +			      N_("disable diff.relative config option"),
>> +			      0, PARSE_OPT_NONEG),
>
> Rather than adding a new option, it would be better to modify the
> existing --relative option. If you remove PARSE_OPT_NONEG from the
> declaration above it will support --no-relative. You will also need to
> modify diff_opt_relative() to handle clearing the option.

Excellent.

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [PATCH v4] diff: add config option relative
  2020-05-17  2:14           ` Đoàn Trần Công Danh
  2020-05-17  2:48             ` Danh Doan
@ 2020-05-17 15:12             ` Junio C Hamano
  2020-05-18  9:40               ` [PATCH v5] " Laurent Arnoud
  2020-05-18  9:43             ` [PATCH v4] " Laurent Arnoud
  2 siblings, 1 reply; 39+ messages in thread
From: Junio C Hamano @ 2020-05-17 15:12 UTC (permalink / raw)
  To: Đoàn Trần Công Danh
  Cc: Laurent Arnoud, phillip.wood, brian m. carlson, git

Đoàn Trần Công Danh  <congdanhqx@gmail.com> writes:

>>  Documentation/config/diff.txt   |  4 ++
>>  Documentation/diff-options.txt  |  3 ++
>>  diff.c                          | 16 ++++--
>>  t/t9904-diff-relative-config.sh | 93 +++++++++++++++++++++++++++++++++
>
> I think t99?? is used for miscellaneous tests.
>
> To me, diff-relative things should be tested in t4045-diff-relative.sh

Right.  Thanks for carefully checking what we already have.

>> diff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt
>> index bb31f0c42b..1b279904eb 100644
>> --- a/Documentation/diff-options.txt
>> +++ b/Documentation/diff-options.txt
>> @@ -651,6 +651,9 @@ ifndef::git-format-patch[]
>>  	not in a subdirectory (e.g. in a bare repository), you
>>  	can name which subdirectory to make the output relative
>>  	to by giving a <path> as an argument.
>> +--no-relative::
>
> Please merge this option with `--relative[=<path>]` above.
> And says something likes:
>
> 	`--no-relative` can be used to countermand
> 	both `diff.relative` and previous `--relative`

This, and all the rest of comments are quite good.  Thanks for helping.

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [PATCH v5] diff: add config option relative
  2020-05-17 15:12             ` Junio C Hamano
@ 2020-05-18  9:40               ` Laurent Arnoud
  2020-05-18 11:46                 ` Shourya Shukla
                                   ` (2 more replies)
  0 siblings, 3 replies; 39+ messages in thread
From: Laurent Arnoud @ 2020-05-18  9:40 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: Đoàn Trần Công Danh, phillip.wood,
	brian m. carlson, git

The `diff.relative` boolean option set to `true` show only changes on
the current directory and show relative pathnames to the current
directory.

Teach --no-relative to override earlier --relative

Signed-off-by: Laurent Arnoud <laurent@spkdev.net>
---
 Documentation/config/diff.txt  |  4 ++
 Documentation/diff-options.txt |  2 +
 diff.c                         | 12 +++--
 t/t4045-diff-relative.sh       | 83 ++++++++++++++++++++++++++++++++--
 4 files changed, 95 insertions(+), 6 deletions(-)

diff --git a/Documentation/config/diff.txt b/Documentation/config/diff.txt
index ff09f1cf73..c3ae136eba 100644
--- 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.
+
 diff.orderFile::
 	File indicating how to order files within a diff.
 	See the '-O' option to linkgit:git-diff[1] for details.
diff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt
index bb31f0c42b..167b451b89 100644
--- a/Documentation/diff-options.txt
+++ b/Documentation/diff-options.txt
@@ -651,6 +651,8 @@ ifndef::git-format-patch[]
 	not in a subdirectory (e.g. in a bare repository), you
 	can name which subdirectory to make the output relative
 	to by giving a <path> as an argument.
+	`--no-relative` can be used to countermand both `diff.relative` config
+	option and previous `--relative`.
 endif::git-format-patch[]

 -a::
diff --git a/diff.c b/diff.c
index d1ad6a3c4a..e3f98334e9 100644
--- a/diff.c
+++ b/diff.c
@@ -48,6 +48,7 @@ static const char *diff_order_file_cfg;
 int diff_auto_refresh_index = 1;
 static int diff_mnemonic_prefix;
 static int diff_no_prefix;
+static int diff_relative;
 static int diff_stat_graph_width;
 static int diff_dirstat_permille_default = 30;
 static struct diff_options default_diff_options;
@@ -386,6 +387,10 @@ int git_diff_ui_config(const char *var, const char *value, void *cb)
 		diff_no_prefix = git_config_bool(var, value);
 		return 0;
 	}
+	if (!strcmp(var, "diff.relative")) {
+		diff_relative = git_config_bool(var, value);
+		return 0;
+	}
 	if (!strcmp(var, "diff.statgraphwidth")) {
 		diff_stat_graph_width = git_config_int(var, value);
 		return 0;
@@ -4558,6 +4563,8 @@ void repo_diff_setup(struct repository *r, struct diff_options *options)
 		options->b_prefix = "b/";
 	}

+	options->flags.relative_name = diff_relative;
+
 	options->color_moved = diff_color_moved_default;
 	options->color_moved_ws_handling = diff_color_moved_ws_default;

@@ -5195,8 +5202,7 @@ static int diff_opt_relative(const struct option *opt,
 {
 	struct diff_options *options = opt->value;

-	BUG_ON_OPT_NEG(unset);
-	options->flags.relative_name = 1;
+	options->flags.relative_name = !unset;
 	if (arg)
 		options->prefix = arg;
 	return 0;
@@ -5492,7 +5498,7 @@ static void prep_parse_options(struct diff_options *options)
 		OPT_GROUP(N_("Other diff options")),
 		OPT_CALLBACK_F(0, "relative", options, N_("<prefix>"),
 			       N_("when run from subdir, exclude changes outside and show relative paths"),
-			       PARSE_OPT_NONEG | PARSE_OPT_OPTARG,
+			       PARSE_OPT_OPTARG,
 			       diff_opt_relative),
 		OPT_BOOL('a', "text", &options->flags.text,
 			 N_("treat all files as text")),
diff --git a/t/t4045-diff-relative.sh b/t/t4045-diff-relative.sh
index 258808708e..ac264ccc2a 100755
--- a/t/t4045-diff-relative.sh
+++ b/t/t4045-diff-relative.sh
@@ -8,7 +8,8 @@ test_expect_success 'setup' '
 	echo content >file1 &&
 	mkdir subdir &&
 	echo other content >subdir/file2 &&
-	blob=$(git hash-object subdir/file2) &&
+	blob_file1=$(git hash-object file1) &&
+	blob_file2=$(git hash-object subdir/file2) &&
 	git add . &&
 	git commit -m one
 '
@@ -18,7 +19,7 @@ check_diff () {
 	shift
 	expect=$1
 	shift
-	short_blob=$(git rev-parse --short $blob)
+	short_blob=$(git rev-parse --short $blob_file2)
 	cat >expected <<-EOF
 	diff --git a/$expect b/$expect
 	new file mode 100644
@@ -70,7 +71,7 @@ check_raw () {
 	expect=$1
 	shift
 	cat >expected <<-EOF
-	:000000 100644 $ZERO_OID $blob A	$expect
+	:000000 100644 $ZERO_OID $blob_file2 A	$expect
 	EOF
 	test_expect_success "--raw $*" "
 		git -C '$dir' diff --no-abbrev --raw $* HEAD^ >actual &&
@@ -86,4 +87,80 @@ do
 	check_$type . dir/file2 --relative=sub
 done

+check_diff_relative_option () {
+	dir=$1
+	shift
+	expect=$1
+	shift
+	relative_opt=$1
+	shift
+	short_blob=$(git rev-parse --short "$blob_file2")
+	cat >expected <<-EOF
+	diff --git a/$expect b/$expect
+	new file mode 100644
+	index 0000000..$short_blob
+	--- /dev/null
+	+++ b/$expect
+	@@ -0,0 +1 @@
+	+other content
+	EOF
+	test_expect_success "config diff.relative $relative_opt -p $*" "
+		test_config -C $dir diff.relative $relative_opt &&
+		git -C '$dir' diff -p $* HEAD^ >actual &&
+		test_cmp expected actual
+	"
+}
+
+check_diff_no_relative_option () {
+	dir=$1
+	shift
+	expect=$1
+	shift
+	relative_opt=$1
+	shift
+	short_blob_file1=$(git rev-parse --short "$blob_file1")
+	short_blob_file2=$(git rev-parse --short "$blob_file2")
+	cat >expected <<-EOF
+	diff --git a/file1 b/file1
+	new file mode 100644
+	index 0000000..$short_blob_file1
+	--- /dev/null
+	+++ b/file1
+	@@ -0,0 +1 @@
+	+content
+	diff --git a/$expect b/$expect
+	new file mode 100644
+	index 0000000..$short_blob_file2
+	--- /dev/null
+	+++ b/$expect
+	@@ -0,0 +1 @@
+	+other content
+	EOF
+	test_expect_success "config diff.relative $relative_opt -p $*" "
+		test_config -C $dir diff.relative $relative_opt &&
+		git -C '$dir' diff -p $* HEAD^ >actual &&
+		git -C '$dir' diff -p $* HEAD^ >/tmp/actual &&
+		test_cmp expected actual
+	"
+}
+
+check_diff_no_relative_option . subdir/file2 false
+check_diff_no_relative_option . subdir/file2 true --no-relative
+check_diff_no_relative_option . subdir/file2 false --no-relative
+check_diff_no_relative_option subdir subdir/file2 false
+check_diff_no_relative_option subdir subdir/file2 true --no-relative
+check_diff_no_relative_option subdir subdir/file2 false --no-relative
+
+check_diff_relative_option . file2 false --relative=subdir/
+check_diff_relative_option . file2 false --relative=subdir
+check_diff_relative_option . file2 true --relative=subdir/
+check_diff_relative_option . file2 true --relative=subdir
+check_diff_relative_option subdir file2 false --relative
+check_diff_relative_option subdir file2 true --relative
+check_diff_relative_option subdir file2 true
+check_diff_relative_option subdir file2 false --no-relative --relative
+check_diff_relative_option subdir file2 true --no-relative --relative
+check_diff_relative_option . file2 false --no-relative --relative=subdir
+check_diff_relative_option . file2 true --no-relative --relative=subdir
+
 test_done
--
2.26.2

^ permalink raw reply related	[flat|nested] 39+ messages in thread

* Re: [PATCH v4] diff: add config option relative
  2020-05-17  2:14           ` Đoàn Trần Công Danh
  2020-05-17  2:48             ` Danh Doan
  2020-05-17 15:12             ` Junio C Hamano
@ 2020-05-18  9:43             ` Laurent Arnoud
  2 siblings, 0 replies; 39+ messages in thread
From: Laurent Arnoud @ 2020-05-18  9:43 UTC (permalink / raw)
  To: Đoàn Trần Công Danh
  Cc: phillip.wood, Junio C Hamano, brian m. carlson, git

Hi Danh,

On Sun, May 17, 2020 at 09:14:52AM +0700, Đoàn Trần Công Danh wrote:
> Above comment also applied here.
> And remove this debug cat.

Oops thanks for the review I've submitted a v5 for review let me know if I
responded to all your feedbacks

-- 
Laurent

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [PATCH v5] diff: add config option relative
  2020-05-18  9:40               ` [PATCH v5] " Laurent Arnoud
@ 2020-05-18 11:46                 ` Shourya Shukla
  2020-05-18 13:04                   ` Đoàn Trần Công Danh
  2020-05-18 17:25                   ` Laurent Arnoud
  2020-05-18 13:56                 ` Đoàn Trần Công Danh
  2020-05-18 16:19                 ` Eric Sunshine
  2 siblings, 2 replies; 39+ messages in thread
From: Shourya Shukla @ 2020-05-18 11:46 UTC (permalink / raw)
  To: laurent; +Cc: congdanhqx, git, gitster, phillip.wood, sandals

Hello Laurent,

I have not worked in this part of Git before, please pardon my
ignorance. I went through the conversations held in this thread
Here are my inferences:

> The `diff.relative` boolean option set to `true` show only changes on
> the current directory and show relative pathnames to the current
> directory.

> Teach --no-relative to override earlier --relative

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`

> --- 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.

Also, if I am not mistaken:

> @@ -5195,8 +5202,7 @@ static int diff_opt_relative(const struct option *opt,
>  {
>  	struct diff_options *options = opt->value;
> 
> -	BUG_ON_OPT_NEG(unset);
> -	options->flags.relative_name = 1;
> +	options->flags.relative_name = !unset;

This is the overriding part right?

BTW you mention the `no-relative` setting which will override the
`relative` option. I am not able to see where exactly is the occurence
of the `no-relative` option in the code? What I mean is that I am not
able to observe where exactly does the C code identify a `no-relative`
option from the command line input? Did I miss something out here?

All nits aside, it looks like a very good concept to me. Nice work! :)

Regards,
Shourya Shukla

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [PATCH v5] diff: add config option relative
  2020-05-18 11:46                 ` Shourya Shukla
@ 2020-05-18 13:04                   ` Đoàn Trần Công Danh
  2020-05-18 17:25                   ` Laurent Arnoud
  1 sibling, 0 replies; 39+ messages in thread
From: Đoàn Trần Công Danh @ 2020-05-18 13:04 UTC (permalink / raw)
  To: Shourya Shukla; +Cc: laurent, git, gitster, phillip.wood, sandals

Hi Shourya,

On 2020-05-18 17:16:30+0530, Shourya Shukla <shouryashukla.oo@gmail.com> wrote:
> BTW you mention the `no-relative` setting which will override the
> `relative` option. I am not able to see where exactly is the occurence
> of the `no-relative` option in the code? What I mean is that I am not
> able to observe where exactly does the C code identify a `no-relative`
> option from the command line input? Did I miss something out here?

--no-relative is parsed by removing PARSE_OPT_NONE in
prep_parse_options (diff.c:5495)

-- 
Danh

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [PATCH v5] diff: add config option relative
  2020-05-18  9:40               ` [PATCH v5] " Laurent Arnoud
  2020-05-18 11:46                 ` Shourya Shukla
@ 2020-05-18 13:56                 ` Đoàn Trần Công Danh
  2020-05-18 16:57                   ` Junio C Hamano
                                     ` (2 more replies)
  2020-05-18 16:19                 ` Eric Sunshine
  2 siblings, 3 replies; 39+ messages in thread
From: Đoàn Trần Công Danh @ 2020-05-18 13:56 UTC (permalink / raw)
  To: Laurent Arnoud; +Cc: Junio C Hamano, phillip.wood, brian m. carlson, git

Hi Laurent,

On 2020-05-18 11:40:21+0200, Laurent Arnoud <laurent@spkdev.net> wrote:
> The `diff.relative` boolean option set to `true` show only changes on
> the current directory and show relative pathnames to the current
> directory.
> 
> Teach --no-relative to override earlier --relative
> 
> Signed-off-by: Laurent Arnoud <laurent@spkdev.net>

Thanks for addressing my comment.

Sorry for detecting this late.
Since I usually only look into end of mail-archive to see any
interesting topic.

And sorry for this late email,
I want to run full test before replying with this.

I've just seen this:
https://lore.kernel.org/git/xmqq1rnk923o.fsf@gitster.c.googlers.com/

I've written some test and concluded that we'll need this fix-up
to make sure git-format-patch(1) doesn't generate broken patch:

----------------8<----------------
 builtin/log.c            |  1 +
 t/t4045-diff-relative.sh | 11 ++++++++---
 2 files changed, 9 insertions(+), 3 deletions(-)

diff --git a/builtin/log.c b/builtin/log.c
index d104d5c688..5949a4883e 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -1744,6 +1744,7 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
 	rev.diff = 1;
 	rev.max_parents = 1;
 	rev.diffopt.flags.recursive = 1;
+	rev.diffopt.flags.relative_name = 0;
 	rev.subject_prefix = fmt_patch_subject_prefix;
 	memset(&s_r_opt, 0, sizeof(s_r_opt));
 	s_r_opt.def = "HEAD";
diff --git a/t/t4045-diff-relative.sh b/t/t4045-diff-relative.sh
index ac264ccc2a..a73b104d66 100755
--- a/t/t4045-diff-relative.sh
+++ b/t/t4045-diff-relative.sh
@@ -11,7 +11,8 @@ test_expect_success 'setup' '
 	blob_file1=$(git hash-object file1) &&
 	blob_file2=$(git hash-object subdir/file2) &&
 	git add . &&
-	git commit -m one
+	git commit -m one &&
+	git format-patch -1 --stdout >expect.patch
 '
 
 check_diff () {
@@ -107,7 +108,9 @@ check_diff_relative_option () {
 	test_expect_success "config diff.relative $relative_opt -p $*" "
 		test_config -C $dir diff.relative $relative_opt &&
 		git -C '$dir' diff -p $* HEAD^ >actual &&
-		test_cmp expected actual
+		test_cmp expected actual &&
+		git -C '$dir' format-patch -1 --stdout >actual.patch &&
+		test_cmp expect.patch actual.patch
 	"
 }
 
@@ -140,7 +143,9 @@ check_diff_no_relative_option () {
 		test_config -C $dir diff.relative $relative_opt &&
 		git -C '$dir' diff -p $* HEAD^ >actual &&
 		git -C '$dir' diff -p $* HEAD^ >/tmp/actual &&
-		test_cmp expected actual
+		test_cmp expected actual &&
+		git -C '$dir' format-patch -1 --stdout >actual.patch &&
+		test_cmp expect.patch actual.patch
 	"
 }
--------------------8<------------------------- 
> @@ -4558,6 +4563,8 @@ void repo_diff_setup(struct repository *r, struct diff_options *options)
>  		options->b_prefix = "b/";
>  	}
> 
> +	options->flags.relative_name = diff_relative;
> +

Nitpick:

I don't think this option is too special to add a newline to separate
it from the rest :)

Sorry about not seeing this earlier, I'm a very careless person.

Anyway, I think (just a matter of my _personal_ preference),
it's better to move it up 21 lines, together with:

	options->flags.rename_empty = 1;

> diff --git a/t/t4045-diff-relative.sh b/t/t4045-diff-relative.sh
> index 258808708e..ac264ccc2a 100755
> --- a/t/t4045-diff-relative.sh
> +++ b/t/t4045-diff-relative.sh
> @@ -8,7 +8,8 @@ test_expect_success 'setup' '
>  	echo content >file1 &&
>  	mkdir subdir &&
>  	echo other content >subdir/file2 &&
> -	blob=$(git hash-object subdir/file2) &&
> +	blob_file1=$(git hash-object file1) &&
> +	blob_file2=$(git hash-object subdir/file2) &&

This rename from blob to blob_file2 is a noise to this patch.
Not sure if we should make a preparatory patch to rename, though.

*I* would say yes, and another patch to move all git-related code
into test_expect_* family. Then, all new testing code for git in this
patch should be placed inside test_expect_*, too.

I think it's better to wait for other's opinions :)

> @@ -86,4 +87,80 @@ do
>  	check_$type . dir/file2 --relative=sub
>  done
> 
> +	diff --git a/$expect b/$expect
> +	new file mode 100644
> +	index 0000000..$short_blob_file2
> +	--- /dev/null
> +	+++ b/$expect
> +	@@ -0,0 +1 @@
> +	+other content
> +	EOF
> +	test_expect_success "config diff.relative $relative_opt -p $*" "
> +		test_config -C $dir diff.relative $relative_opt &&
> +		git -C '$dir' diff -p $* HEAD^ >actual &&
> +		git -C '$dir' diff -p $* HEAD^ >/tmp/actual &&

Please this leftover from debugging.

-- 
Danh

^ permalink raw reply related	[flat|nested] 39+ messages in thread

* Re: [PATCH v5] diff: add config option relative
  2020-05-18  9:40               ` [PATCH v5] " Laurent Arnoud
  2020-05-18 11:46                 ` Shourya Shukla
  2020-05-18 13:56                 ` Đoàn Trần Công Danh
@ 2020-05-18 16:19                 ` Eric Sunshine
  2020-05-18 17:26                   ` Laurent Arnoud
  2 siblings, 1 reply; 39+ messages in thread
From: Eric Sunshine @ 2020-05-18 16:19 UTC (permalink / raw)
  To: Laurent Arnoud
  Cc: Junio C Hamano, Đoàn Trần Công Danh,
	Phillip Wood, brian m. carlson, Git List

On Mon, May 18, 2020 at 5:40 AM Laurent Arnoud <laurent@spkdev.net> wrote:
> The `diff.relative` boolean option set to `true` show only changes on
> the current directory and show relative pathnames to the current
> directory.
>
> Teach --no-relative to override earlier --relative
>
> Signed-off-by: Laurent Arnoud <laurent@spkdev.net>
> ---
> diff --git a/t/t4045-diff-relative.sh b/t/t4045-diff-relative.sh
> +check_diff_relative_option () {
> +       dir=$1
> +       shift
> +       expect=$1
> +       shift
> +       relative_opt=$1
> +       shift
> +       short_blob=$(git rev-parse --short "$blob_file2")
> +       cat >expected <<-EOF
> +       diff --git a/$expect b/$expect
> +       new file mode 100644
> +       index 0000000..$short_blob
> +       --- /dev/null
> +       +++ b/$expect
> +       @@ -0,0 +1 @@
> +       +other content
> +       EOF
> +       test_expect_success "config diff.relative $relative_opt -p $*" "
> +               test_config -C $dir diff.relative $relative_opt &&
> +               git -C '$dir' diff -p $* HEAD^ >actual &&
> +               test_cmp expected actual
> +       "
> +}

Let's move all the setup code into the test itself so that a failure
of git-rev-parse will be caught. For instance:

check_diff_relative_option () {
   dir=$1
   shift
   expect=$1
   shift
   relative_opt=$1
   shift
   test_expect_success "config diff.relative $relative_opt -p $*" "
       short_blob=$(git rev-parse --short $blob_file2) &&
       cat >expected <<-EOF &&
       diff --git a/$expect b/$expect
       new file mode 100644
       index 0000000..$short_blob
       --- /dev/null
       +++ b/$expect
       @@ -0,0 +1 @@
       +other content
       EOF
       test_config -C $dir diff.relative $relative_opt &&
       git -C '$dir' diff -p $* HEAD^ >actual &&
       test_cmp expected actual
   "
}

Same comment applies to the other new function added by this patch.

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [PATCH v5] diff: add config option relative
  2020-05-18 13:56                 ` Đoàn Trần Công Danh
@ 2020-05-18 16:57                   ` Junio C Hamano
  2020-05-18 19:12                     ` Đoàn Trần Công Danh
  2020-05-18 17:03                   ` [PATCH v5] " Junio C Hamano
  2020-05-18 17:32                   ` [PATCH v5] " Laurent Arnoud
  2 siblings, 1 reply; 39+ messages in thread
From: Junio C Hamano @ 2020-05-18 16:57 UTC (permalink / raw)
  To: Đoàn Trần Công Danh
  Cc: Laurent Arnoud, phillip.wood, brian m. carlson, git

Đoàn Trần Công Danh  <congdanhqx@gmail.com> writes:

> I've written some test and concluded that we'll need this fix-up
> to make sure git-format-patch(1) doesn't generate broken patch:
>
> ----------------8<----------------
>  builtin/log.c            |  1 +
>  t/t4045-diff-relative.sh | 11 ++++++++---
>  2 files changed, 9 insertions(+), 3 deletions(-)
>
> diff --git a/builtin/log.c b/builtin/log.c
> index d104d5c688..5949a4883e 100644
> --- a/builtin/log.c
> +++ b/builtin/log.c
> @@ -1744,6 +1744,7 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
>  	rev.diff = 1;
>  	rev.max_parents = 1;
>  	rev.diffopt.flags.recursive = 1;
> +	rev.diffopt.flags.relative_name = 0;
>  	rev.subject_prefix = fmt_patch_subject_prefix;
>  	memset(&s_r_opt, 0, sizeof(s_r_opt));
>  	s_r_opt.def = "HEAD";

Hmph, what do you exactly mean by "broken patch"?

I actually do not mind people who set "diff.relative" to do

    $ git config diff.relative true
    $ cd t && git format-patch -1 --stdout

to get an incomplete patch that covers only the t/ subdirectory, as
long as they can ask Git to optionally get the full view with

    $ cd t && git format-patch -1 --stdout --no-relative

Note: this is not limited to format-patch but all the commands in
the diff family like "log --stat", "show", etc.

Of course, diff.relative configuration would have no use for me
personally and I suspect it would be useless for many people, but
those who work inside a deeply nested project (java perhaps?) it
would be handy and it makes sense to match the default behaviour
between "diff", "show" and "format-patch" when the configuration is
active, as long as they have a way to contermand from the command
line.

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [PATCH v5] diff: add config option relative
  2020-05-18 13:56                 ` Đoàn Trần Công Danh
  2020-05-18 16:57                   ` Junio C Hamano
@ 2020-05-18 17:03                   ` Junio C Hamano
  2020-05-18 17:21                     ` [PATCH v6] " Laurent Arnoud
  2020-05-18 17:32                   ` [PATCH v5] " Laurent Arnoud
  2 siblings, 1 reply; 39+ messages in thread
From: Junio C Hamano @ 2020-05-18 17:03 UTC (permalink / raw)
  To: Đoàn Trần Công Danh
  Cc: Laurent Arnoud, phillip.wood, brian m. carlson, git

Đoàn Trần Công Danh  <congdanhqx@gmail.com> writes:

>> @@ -4558,6 +4563,8 @@ void repo_diff_setup(struct repository *r, struct diff_options *options)
>>  		options->b_prefix = "b/";
>>  	}
>> 
>> +	options->flags.relative_name = diff_relative;
>> +
>
> Nitpick:
>
> I don't think this option is too special to add a newline to separate
> it from the rest :)
>
> Sorry about not seeing this earlier, I'm a very careless person.
>
> Anyway, I think (just a matter of my _personal_ preference),
> it's better to move it up 21 lines, together with:
>
> 	options->flags.rename_empty = 1;

Sounds like a reasonable improvement of readability.

>> diff --git a/t/t4045-diff-relative.sh b/t/t4045-diff-relative.sh
>> index 258808708e..ac264ccc2a 100755
>> --- a/t/t4045-diff-relative.sh
>> +++ b/t/t4045-diff-relative.sh
>> @@ -8,7 +8,8 @@ test_expect_success 'setup' '
>>  	echo content >file1 &&
>>  	mkdir subdir &&
>>  	echo other content >subdir/file2 &&
>> -	blob=$(git hash-object subdir/file2) &&
>> +	blob_file1=$(git hash-object file1) &&
>> +	blob_file2=$(git hash-object subdir/file2) &&
>
> This rename from blob to blob_file2 is a noise to this patch.
>
> Not sure if we should make a preparatory patch to rename, though.

I personally do not mind this one.  It is crystal clear from the
patch text: "We used to use only one and managed to get away without
blob1/blob2 but now we use more than 1, so let's use names with
number suffix".  On the other hand, a "preparatory patch" that
renames blob to blob_file1 before we need the second one is a noise.

> *I* would say yes, and another patch to move all git-related code
> into test_expect_* family. Then, all new testing code for git in this
> patch should be placed inside test_expect_*, too.

The latter clean-up to make sure we won't notice Git failure outside
test_expect_* block may make sense, but I do not know if we want to
make it a preparatory clean-up or "remember to do so later when the
dust settles".  If this single-patch topic needs to touch only a
small part of the existing test to do its job, and such a clean-up
ends up touching far wider parts of the script, then I would say we
can do so as a post-patch clean-up, not as a part of the topic.

>
> I think it's better to wait for other's opinions :)
>
>> @@ -86,4 +87,80 @@ do
>>  	check_$type . dir/file2 --relative=sub
>>  done
>> 
>> +	diff --git a/$expect b/$expect
>> +	new file mode 100644
>> +	index 0000000..$short_blob_file2
>> +	--- /dev/null
>> +	+++ b/$expect
>> +	@@ -0,0 +1 @@
>> +	+other content
>> +	EOF
>> +	test_expect_success "config diff.relative $relative_opt -p $*" "
>> +		test_config -C $dir diff.relative $relative_opt &&
>> +		git -C '$dir' diff -p $* HEAD^ >actual &&
>> +		git -C '$dir' diff -p $* HEAD^ >/tmp/actual &&
>
> Please this leftover from debugging.

Thanks for a careful review, again.

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [PATCH v6] diff: add config option relative
  2020-05-18 17:03                   ` [PATCH v5] " Junio C Hamano
@ 2020-05-18 17:21                     ` Laurent Arnoud
  2020-05-18 17:31                       ` Junio C Hamano
                                         ` (2 more replies)
  0 siblings, 3 replies; 39+ messages in thread
From: Laurent Arnoud @ 2020-05-18 17:21 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: Đoàn Trần Công Danh, phillip.wood,
	brian m. carlson, git

From 478164550f65dfb26e213957d461f0f94fecbcab Mon Sep 17 00:00:00 2001
From: Laurent Arnoud <laurent@spkdev.net>
Date: Fri, 15 May 2020 19:34:42 +0200
Subject: [PATCH v6] diff: add config option relative

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 earlier --relative

Signed-off-by: Laurent Arnoud <laurent@spkdev.net>
---
 Documentation/config/diff.txt  |  4 ++
 Documentation/diff-options.txt |  2 +
 diff.c                         | 11 +++--
 t/t4014-format-patch.sh        | 13 ++++++
 t/t4045-diff-relative.sh       | 82 ++++++++++++++++++++++++++++++++--
 5 files changed, 106 insertions(+), 6 deletions(-)

diff --git a/Documentation/config/diff.txt b/Documentation/config/diff.txt
index ff09f1cf73..c3ae136eba 100644
--- 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.
+
 diff.orderFile::
 	File indicating how to order files within a diff.
 	See the '-O' option to linkgit:git-diff[1] for details.
diff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt
index bb31f0c42b..167b451b89 100644
--- a/Documentation/diff-options.txt
+++ b/Documentation/diff-options.txt
@@ -651,6 +651,8 @@ ifndef::git-format-patch[]
 	not in a subdirectory (e.g. in a bare repository), you
 	can name which subdirectory to make the output relative
 	to by giving a <path> as an argument.
+	`--no-relative` can be used to countermand both `diff.relative` config
+	option and previous `--relative`.
 endif::git-format-patch[]
 
 -a::
diff --git a/diff.c b/diff.c
index d1ad6a3c4a..863da896c0 100644
--- a/diff.c
+++ b/diff.c
@@ -48,6 +48,7 @@ static const char *diff_order_file_cfg;
 int diff_auto_refresh_index = 1;
 static int diff_mnemonic_prefix;
 static int diff_no_prefix;
+static int diff_relative;
 static int diff_stat_graph_width;
 static int diff_dirstat_permille_default = 30;
 static struct diff_options default_diff_options;
@@ -386,6 +387,10 @@ int git_diff_ui_config(const char *var, const char *value, void *cb)
 		diff_no_prefix = git_config_bool(var, value);
 		return 0;
 	}
+	if (!strcmp(var, "diff.relative")) {
+		diff_relative = git_config_bool(var, value);
+		return 0;
+	}
 	if (!strcmp(var, "diff.statgraphwidth")) {
 		diff_stat_graph_width = git_config_int(var, value);
 		return 0;
@@ -4538,6 +4543,7 @@ void repo_diff_setup(struct repository *r, struct diff_options *options)
 	options->interhunkcontext = diff_interhunk_context_default;
 	options->ws_error_highlight = ws_error_highlight_default;
 	options->flags.rename_empty = 1;
+	options->flags.relative_name = diff_relative;
 	options->objfind = NULL;
 
 	/* pathchange left =NULL by default */
@@ -5195,8 +5201,7 @@ static int diff_opt_relative(const struct option *opt,
 {
 	struct diff_options *options = opt->value;
 
-	BUG_ON_OPT_NEG(unset);
-	options->flags.relative_name = 1;
+	options->flags.relative_name = !unset;
 	if (arg)
 		options->prefix = arg;
 	return 0;
@@ -5492,7 +5497,7 @@ static void prep_parse_options(struct diff_options *options)
 		OPT_GROUP(N_("Other diff options")),
 		OPT_CALLBACK_F(0, "relative", options, N_("<prefix>"),
 			       N_("when run from subdir, exclude changes outside and show relative paths"),
-			       PARSE_OPT_NONEG | PARSE_OPT_OPTARG,
+			       PARSE_OPT_OPTARG,
 			       diff_opt_relative),
 		OPT_BOOL('a', "text", &options->flags.text,
 			 N_("treat all files as text")),
diff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh
index db7e733af9..575e079cc2 100755
--- a/t/t4014-format-patch.sh
+++ b/t/t4014-format-patch.sh
@@ -1602,6 +1602,19 @@ test_expect_success 'format patch ignores color.ui' '
 	test_cmp expect actual
 '
 
+test_expect_success 'format patch respects diff.relative' '
+	rm -rf subdir &&
+	mkdir subdir &&
+	echo other content >subdir/file2 &&
+	git add subdir/file2 &&
+	git commit -F msg &&
+	test_unconfig diff.relative &&
+	git format-patch --relative=subdir --stdout -1 >expect &&
+	test_config diff.relative true &&
+	git -C subdir format-patch --stdout -1 >actual &&
+	test_cmp expect actual
+'
+
 test_expect_success 'cover letter with invalid --cover-from-description and config' '
 	test_config branch.rebuild-1.description "config subject
 
diff --git a/t/t4045-diff-relative.sh b/t/t4045-diff-relative.sh
index 258808708e..87606de9ec 100755
--- a/t/t4045-diff-relative.sh
+++ b/t/t4045-diff-relative.sh
@@ -8,7 +8,8 @@ test_expect_success 'setup' '
 	echo content >file1 &&
 	mkdir subdir &&
 	echo other content >subdir/file2 &&
-	blob=$(git hash-object subdir/file2) &&
+	blob_file1=$(git hash-object file1) &&
+	blob_file2=$(git hash-object subdir/file2) &&
 	git add . &&
 	git commit -m one
 '
@@ -18,7 +19,7 @@ check_diff () {
 	shift
 	expect=$1
 	shift
-	short_blob=$(git rev-parse --short $blob)
+	short_blob=$(git rev-parse --short $blob_file2)
 	cat >expected <<-EOF
 	diff --git a/$expect b/$expect
 	new file mode 100644
@@ -70,7 +71,7 @@ check_raw () {
 	expect=$1
 	shift
 	cat >expected <<-EOF
-	:000000 100644 $ZERO_OID $blob A	$expect
+	:000000 100644 $ZERO_OID $blob_file2 A	$expect
 	EOF
 	test_expect_success "--raw $*" "
 		git -C '$dir' diff --no-abbrev --raw $* HEAD^ >actual &&
@@ -86,4 +87,79 @@ do
 	check_$type . dir/file2 --relative=sub
 done
 
+check_diff_relative_option () {
+	dir=$1
+	shift
+	expect=$1
+	shift
+	relative_opt=$1
+	shift
+	test_expect_success "config diff.relative $relative_opt -p $*" "
+		short_blob=$(git rev-parse --short "$blob_file2") &&
+		cat >expected <<-EOF &&
+		diff --git a/$expect b/$expect
+		new file mode 100644
+		index 0000000..\$short_blob
+		--- /dev/null
+		+++ b/$expect
+		@@ -0,0 +1 @@
+		+other content
+		EOF
+		test_config -C $dir diff.relative $relative_opt &&
+		git -C '$dir' diff -p $* HEAD^ >actual &&
+		test_cmp expected actual
+	"
+}
+
+check_diff_no_relative_option () {
+	dir=$1
+	shift
+	expect=$1
+	shift
+	relative_opt=$1
+	shift
+	test_expect_success "config diff.relative $relative_opt -p $*" "
+		short_blob_file1=$(git rev-parse --short "$blob_file1") &&
+		short_blob_file2=$(git rev-parse --short "$blob_file2") &&
+		cat >expected <<-EOF &&
+		diff --git a/file1 b/file1
+		new file mode 100644
+		index 0000000..\$short_blob_file1
+		--- /dev/null
+		+++ b/file1
+		@@ -0,0 +1 @@
+		+content
+		diff --git a/$expect b/$expect
+		new file mode 100644
+		index 0000000..\$short_blob_file2
+		--- /dev/null
+		+++ b/$expect
+		@@ -0,0 +1 @@
+		+other content
+		EOF
+		test_config -C $dir diff.relative $relative_opt &&
+		git -C '$dir' diff -p $* HEAD^ >actual &&
+		test_cmp expected actual
+	"
+}
+
+check_diff_no_relative_option . subdir/file2 false
+check_diff_no_relative_option . subdir/file2 true --no-relative
+check_diff_no_relative_option . subdir/file2 false --no-relative
+check_diff_no_relative_option subdir subdir/file2 false
+check_diff_no_relative_option subdir subdir/file2 true --no-relative
+check_diff_no_relative_option subdir subdir/file2 false --no-relative
+
+check_diff_relative_option . file2 false --relative=subdir/
+check_diff_relative_option . file2 false --relative=subdir
+check_diff_relative_option . file2 true --relative=subdir/
+check_diff_relative_option . file2 true --relative=subdir
+check_diff_relative_option subdir file2 false --relative
+check_diff_relative_option subdir file2 true --relative
+check_diff_relative_option subdir file2 true
+check_diff_relative_option subdir file2 false --no-relative --relative
+check_diff_relative_option subdir file2 true --no-relative --relative
+check_diff_relative_option . file2 false --no-relative --relative=subdir
+check_diff_relative_option . file2 true --no-relative --relative=subdir
+
 test_done
-- 
2.26.2

^ permalink raw reply related	[flat|nested] 39+ messages in thread

* Re: [PATCH v5] diff: add config option relative
  2020-05-18 11:46                 ` Shourya Shukla
  2020-05-18 13:04                   ` Đoàn Trần Công Danh
@ 2020-05-18 17:25                   ` Laurent Arnoud
  1 sibling, 0 replies; 39+ messages in thread
From: Laurent Arnoud @ 2020-05-18 17:25 UTC (permalink / raw)
  To: Shourya Shukla; +Cc: congdanhqx, git, gitster, phillip.wood, sandals

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

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [PATCH v5] diff: add config option relative
  2020-05-18 16:19                 ` Eric Sunshine
@ 2020-05-18 17:26                   ` Laurent Arnoud
  0 siblings, 0 replies; 39+ messages in thread
From: Laurent Arnoud @ 2020-05-18 17:26 UTC (permalink / raw)
  To: Eric Sunshine
  Cc: Junio C Hamano, Đoàn Trần Công Danh,
	Phillip Wood, brian m. carlson, Git List

On Mon, May 18, 2020 at 12:19:20PM -0400, Eric Sunshine wrote:
> Let's move all the setup code into the test itself so that a failure
> of git-rev-parse will be caught. For instance:
> 
> check_diff_relative_option () {
>    dir=$1
>    shift
>    expect=$1
>    shift
>    relative_opt=$1
>    shift
>    test_expect_success "config diff.relative $relative_opt -p $*" "
>        short_blob=$(git rev-parse --short $blob_file2) &&
>        cat >expected <<-EOF &&
>        diff --git a/$expect b/$expect
>        new file mode 100644
>        index 0000000..$short_blob
>        --- /dev/null
>        +++ b/$expect
>        @@ -0,0 +1 @@
>        +other content
>        EOF
>        test_config -C $dir diff.relative $relative_opt &&
>        git -C '$dir' diff -p $* HEAD^ >actual &&
>        test_cmp expected actual
>    "
> }
> 
> Same comment applies to the other new function added by this patch.

Thanks its applied on the v6 patch

-- 
Laurent

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [PATCH v6] diff: add config option relative
  2020-05-18 17:21                     ` [PATCH v6] " Laurent Arnoud
@ 2020-05-18 17:31                       ` Junio C Hamano
  2020-05-18 17:33                         ` Junio C Hamano
  2020-05-18 17:34                       ` Eric Sunshine
  2020-05-18 19:19                       ` Đoàn Trần Công Danh
  2 siblings, 1 reply; 39+ messages in thread
From: Junio C Hamano @ 2020-05-18 17:31 UTC (permalink / raw)
  To: Laurent Arnoud
  Cc: Đoàn Trần Công Danh, phillip.wood,
	brian m. carlson, git

Laurent Arnoud <laurent@spkdev.net> writes:

> Subject: Re: [PATCH v6] diff: add config option relative

This is not a new problem with this round, but can you stop starting
a new iteration that is v6 with "Subject: Re: [PATCH v6]"? 

It is good to make v6 a reply to v6, but this is the *first* message
that begins v6 iteration, so it is easier to find it without "Re:"
prefix.

Thanks.

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [PATCH v5] diff: add config option relative
  2020-05-18 13:56                 ` Đoàn Trần Công Danh
  2020-05-18 16:57                   ` Junio C Hamano
  2020-05-18 17:03                   ` [PATCH v5] " Junio C Hamano
@ 2020-05-18 17:32                   ` Laurent Arnoud
  2 siblings, 0 replies; 39+ messages in thread
From: Laurent Arnoud @ 2020-05-18 17:32 UTC (permalink / raw)
  To: Đoàn Trần Công Danh
  Cc: Junio C Hamano, phillip.wood, brian m. carlson, git

Hi Danh,

On Mon, May 18, 2020 at 08:56:56PM +0700, Đoàn Trần Công Danh wrote:
> Thanks for addressing my comment.
> 
> Sorry for detecting this late.
> Since I usually only look into end of mail-archive to see any
> interesting topic.
> 
> And sorry for this late email,
> I want to run full test before replying with this.
> 
> I've just seen this:
> https://lore.kernel.org/git/xmqq1rnk923o.fsf@gitster.c.googlers.com/
> 
> I've written some test and concluded that we'll need this fix-up
> to make sure git-format-patch(1) doesn't generate broken patch:

No worries this Junio is ok for this I've added a test instead in
t/t4014-format-patch.sh ensuring that format-patch respects the config
diff.relative on the v6 of the patch, let me know what you think

> Nitpick:
> 
> I don't think this option is too special to add a newline to separate
> it from the rest :)
> 
> Sorry about not seeing this earlier, I'm a very careless person.
> 
> Anyway, I think (just a matter of my _personal_ preference),
> it's better to move it up 21 lines, together with:
> 
> 	options->flags.rename_empty = 1;
> 

Applied in the v6

> > @@ -86,4 +87,80 @@ do
> >  	check_$type . dir/file2 --relative=sub
> >  done
> > 
> > +	diff --git a/$expect b/$expect
> > +	new file mode 100644
> > +	index 0000000..$short_blob_file2
> > +	--- /dev/null
> > +	+++ b/$expect
> > +	@@ -0,0 +1 @@
> > +	+other content
> > +	EOF
> > +	test_expect_success "config diff.relative $relative_opt -p $*" "
> > +		test_config -C $dir diff.relative $relative_opt &&
> > +		git -C '$dir' diff -p $* HEAD^ >actual &&
> > +		git -C '$dir' diff -p $* HEAD^ >/tmp/actual &&
> 
> Please this leftover from debugging.

Opps fixed and thanks again for the review

-- 
Laurent

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [PATCH v6] diff: add config option relative
  2020-05-18 17:31                       ` Junio C Hamano
@ 2020-05-18 17:33                         ` Junio C Hamano
  0 siblings, 0 replies; 39+ messages in thread
From: Junio C Hamano @ 2020-05-18 17:33 UTC (permalink / raw)
  To: Laurent Arnoud
  Cc: Đoàn Trần Công Danh, phillip.wood,
	brian m. carlson, git

Junio C Hamano <gitster@pobox.com> writes:

> Laurent Arnoud <laurent@spkdev.net> writes:
>
>> Subject: Re: [PATCH v6] diff: add config option relative
>
> This is not a new problem with this round, but can you stop starting
> a new iteration that is v6 with "Subject: Re: [PATCH v6]"? 
>
> It is good to make v6 a reply to v6, but this is the *first* message

Sorry, "make v6 a reply to v5" is what I meant X-<.

> that begins v6 iteration, so it is easier to find it without "Re:"
> prefix.
>
> Thanks.

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [PATCH v6] diff: add config option relative
  2020-05-18 17:21                     ` [PATCH v6] " Laurent Arnoud
  2020-05-18 17:31                       ` Junio C Hamano
@ 2020-05-18 17:34                       ` Eric Sunshine
  2020-05-18 19:19                       ` Đoàn Trần Công Danh
  2 siblings, 0 replies; 39+ messages in thread
From: Eric Sunshine @ 2020-05-18 17:34 UTC (permalink / raw)
  To: Laurent Arnoud
  Cc: Junio C Hamano, Đoàn Trần Công Danh,
	Phillip Wood, brian m. carlson, Git List

On Mon, May 18, 2020 at 1:22 PM Laurent Arnoud <laurent@spkdev.net> wrote:
> 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 earlier --relative
>
> Signed-off-by: Laurent Arnoud <laurent@spkdev.net>
> ---
> diff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh
> @@ -86,4 +87,79 @@ do
> +check_diff_relative_option () {
> +       dir=$1
> +       shift
> +       expect=$1
> +       shift
> +       relative_opt=$1
> +       shift
> +       test_expect_success "config diff.relative $relative_opt -p $*" "
> +               short_blob=$(git rev-parse --short "$blob_file2") &&

You're using double quotes inside a double-quote context. (Note that I
dropped the quotes around $blob_file2 in the example I gave in order
to avoid this problem.)

> +               cat >expected <<-EOF &&
> +               diff --git a/$expect b/$expect
> +               new file mode 100644
> +               index 0000000..\$short_blob
> +               --- /dev/null
> +               +++ b/$expect
> +               @@ -0,0 +1 @@
> +               +other content
> +               EOF
> +               test_config -C $dir diff.relative $relative_opt &&
> +               git -C '$dir' diff -p $* HEAD^ >actual &&
> +               test_cmp expected actual
> +       "
> +}

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [PATCH v5] diff: add config option relative
  2020-05-18 16:57                   ` Junio C Hamano
@ 2020-05-18 19:12                     ` Đoàn Trần Công Danh
  2020-05-18 20:37                       ` Junio C Hamano
  0 siblings, 1 reply; 39+ messages in thread
From: Đoàn Trần Công Danh @ 2020-05-18 19:12 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Laurent Arnoud, phillip.wood, brian m. carlson, git

On 2020-05-18 09:57:09-0700, Junio C Hamano <gitster@pobox.com> wrote:
> Hmph, what do you exactly mean by "broken patch"?
> 
> I actually do not mind people who set "diff.relative" to do
> 
>     $ git config diff.relative true
>     $ cd t && git format-patch -1 --stdout
> 
> to get an incomplete patch that covers only the t/ subdirectory, as
> long as they can ask Git to optionally get the full view with
> 
>     $ cd t && git format-patch -1 --stdout --no-relative

Well, git-format-patch has never understood --relative,
which I think is a sane behaviour,
hence git-format-patch will never understand --no-relative after this
patch.

Oh no, to my surprise, git-format-patch does accept --[no-]relative, now.

We don't document --relative for git-format-patch, the --relative
documentation is placed under the guard, for that matter:

Documentation/diff-options.txt 


ifndef::git-format-patch[]
-R::
        Swap two inputs; that is, show differences from index or
        on-disk file to tree contents.

--relative[=<path>]::
        When run from a subdirectory of the project, it can be
        told to exclude changes outside the directory and show
        pathnames relative to it with this option.  When you are
        not in a subdirectory (e.g. in a bare repository), you
        can name which subdirectory to make the output relative
        to by giving a <path> as an argument.
endif::git-format-patch[]

Maybe time to fix the documentation.

-- 
Danh

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [PATCH v6] diff: add config option relative
  2020-05-18 17:21                     ` [PATCH v6] " Laurent Arnoud
  2020-05-18 17:31                       ` Junio C Hamano
  2020-05-18 17:34                       ` Eric Sunshine
@ 2020-05-18 19:19                       ` Đoàn Trần Công Danh
  2020-05-18 20:02                         ` Junio C Hamano
  2 siblings, 1 reply; 39+ messages in thread
From: Đoàn Trần Công Danh @ 2020-05-18 19:19 UTC (permalink / raw)
  To: Laurent Arnoud; +Cc: Junio C Hamano, phillip.wood, brian m. carlson, git

On 2020-05-18 19:21:03+0200, Laurent Arnoud <laurent@spkdev.net> wrote:
> diff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt
> index bb31f0c42b..167b451b89 100644
> --- a/Documentation/diff-options.txt
> +++ b/Documentation/diff-options.txt
> @@ -651,6 +651,8 @@ ifndef::git-format-patch[]
>  	not in a subdirectory (e.g. in a bare repository), you
>  	can name which subdirectory to make the output relative
>  	to by giving a <path> as an argument.
> +	`--no-relative` can be used to countermand both `diff.relative` config
> +	option and previous `--relative`.

Nitpick:
Please mentions --no-relative in option list:

diff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt
index 167b451b89..8761c04bc2 100644
--- a/Documentation/diff-options.txt
+++ b/Documentation/diff-options.txt
@@ -645,6 +645,7 @@ ifndef::git-format-patch[]
 	on-disk file to tree contents.
 
 --relative[=<path>]::
+--no-relative::
 	When run from a subdirectory of the project, it can be
 	told to exclude changes outside the directory and show
 	pathnames relative to it with this option.  When you are

Together with Eric suggesstion, this patch looks good to me.

-- 
Danh

^ permalink raw reply related	[flat|nested] 39+ messages in thread

* Re: [PATCH v6] diff: add config option relative
  2020-05-18 19:19                       ` Đoàn Trần Công Danh
@ 2020-05-18 20:02                         ` Junio C Hamano
  0 siblings, 0 replies; 39+ messages in thread
From: Junio C Hamano @ 2020-05-18 20:02 UTC (permalink / raw)
  To: Đoàn Trần Công Danh
  Cc: Laurent Arnoud, phillip.wood, brian m. carlson, git

Đoàn Trần Công Danh  <congdanhqx@gmail.com> writes:

> On 2020-05-18 19:21:03+0200, Laurent Arnoud <laurent@spkdev.net> wrote:
>> diff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt
>> index bb31f0c42b..167b451b89 100644
>> --- a/Documentation/diff-options.txt
>> +++ b/Documentation/diff-options.txt
>> @@ -651,6 +651,8 @@ ifndef::git-format-patch[]
>>  	not in a subdirectory (e.g. in a bare repository), you
>>  	can name which subdirectory to make the output relative
>>  	to by giving a <path> as an argument.
>> +	`--no-relative` can be used to countermand both `diff.relative` config
>> +	option and previous `--relative`.
>
> Nitpick:
> Please mentions --no-relative in option list:
>
> diff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt
> index 167b451b89..8761c04bc2 100644
> --- a/Documentation/diff-options.txt
> +++ b/Documentation/diff-options.txt
> @@ -645,6 +645,7 @@ ifndef::git-format-patch[]
>  	on-disk file to tree contents.
>  
>  --relative[=<path>]::
> +--no-relative::
>  	When run from a subdirectory of the project, it can be
>  	told to exclude changes outside the directory and show
>  	pathnames relative to it with this option.  When you are
>
> Together with Eric suggesstion, this patch looks good to me.

Yeah, good suggestions all.

Thanks.

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [PATCH v5] diff: add config option relative
  2020-05-18 19:12                     ` Đoàn Trần Công Danh
@ 2020-05-18 20:37                       ` Junio C Hamano
  2020-05-19  0:30                         ` Đoàn Trần Công Danh
  0 siblings, 1 reply; 39+ messages in thread
From: Junio C Hamano @ 2020-05-18 20:37 UTC (permalink / raw)
  To: Đoàn Trần Công Danh
  Cc: Laurent Arnoud, phillip.wood, brian m. carlson, git

Đoàn Trần Công Danh  <congdanhqx@gmail.com> writes:

> We don't document --relative for git-format-patch, the --relative
> documentation is placed under the guard, for that matter:

Yup, since it does not make much sense or "diff.relative" for that
matter in the context of the "format-patch".  In the same sense, -R
makes little sense, so sane people would not want to read about the
option in the documentation, I would think.

> ifndef::git-format-patch[]
> -R::
>         Swap two inputs; that is, show differences from index or
>         on-disk file to tree contents.
>
> --relative[=<path>]::
>         When run from a subdirectory of the project, it can be
>         told to exclude changes outside the directory and show

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [PATCH v5] diff: add config option relative
  2020-05-18 20:37                       ` Junio C Hamano
@ 2020-05-19  0:30                         ` Đoàn Trần Công Danh
  2020-05-19 18:00                           ` Junio C Hamano
  0 siblings, 1 reply; 39+ messages in thread
From: Đoàn Trần Công Danh @ 2020-05-19  0:30 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Laurent Arnoud, phillip.wood, brian m. carlson, git

On 2020-05-18 13:37:55-0700, Junio C Hamano <gitster@pobox.com> wrote:
> Đoàn Trần Công Danh  <congdanhqx@gmail.com> writes:
> 
> > We don't document --relative for git-format-patch, the --relative
> > documentation is placed under the guard, for that matter:
> 
> Yup, since it does not make much sense or "diff.relative" for that
> matter in the context of the "format-patch".  In the same sense, -R
> makes little sense, so sane people would not want to read about the
> option in the documentation, I would think.

Sane people wouldn't want to read that document when we don't have the
config for diff.relative

But, now, we have. Let's document --no-relative, specifically for
git-format-patch(1).

Perhaps something like this (maybe just fixing up to Laurent's)
-----------------8<--------------
From: =?UTF-8?q?=C4=90o=C3=A0n=20Tr=E1=BA=A7n=20C=C3=B4ng=20Danh?=
 <congdanhqx@gmail.com>
Date: Tue, 19 May 2020 07:24:48 +0700
Subject: [PATCH] Documentation: document --no-relative for format-patch
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

git-format-patch(1) accepts --relative for a long time, but the
generated patch couldn't be applied without accompanying information.

Hence, we've never document --relative for git-format-patch(1).

Now, we've introduced `diff.relative` config, let's tell Git's users
that they have an option to override it.

Signed-off-by: Đoàn Trần Công Danh <congdanhqx@gmail.com>
---
 Documentation/diff-options.txt | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt
index 167b451b89..0f5c108324 100644
--- a/Documentation/diff-options.txt
+++ b/Documentation/diff-options.txt
@@ -654,6 +654,11 @@ ifndef::git-format-patch[]
 	`--no-relative` can be used to countermand both `diff.relative` config
 	option and previous `--relative`.
 endif::git-format-patch[]
+ifdef::git-format-patch[]
+--no-relative::
+	Countermand `diff.relative` config. Show all changes in the repository
+	with pathname relative to top-level directory of the repository.
+endif::git-format-patch[]
 
 -a::
 --text::
-- 

Danh


^ permalink raw reply related	[flat|nested] 39+ messages in thread

* Re: [PATCH v5] diff: add config option relative
  2020-05-19  0:30                         ` Đoàn Trần Công Danh
@ 2020-05-19 18:00                           ` Junio C Hamano
  2020-05-19 19:39                             ` [PATCH v7] " Laurent Arnoud
  0 siblings, 1 reply; 39+ messages in thread
From: Junio C Hamano @ 2020-05-19 18:00 UTC (permalink / raw)
  To: Đoàn Trần Công Danh
  Cc: Laurent Arnoud, phillip.wood, brian m. carlson, git

Đoàn Trần Công Danh  <congdanhqx@gmail.com> writes:

> Sane people wouldn't want to read that document when we don't have the
> config for diff.relative
>
> But, now, we have. Let's document --no-relative, specifically for
> git-format-patch(1).
>
> Perhaps something like this (maybe just fixing up to Laurent's)

I love it when a contributor pushes back with a well thought out
counter-argument.  Yes, I think the reasoning along the line makes
sense.

If a project binds another project with -Xsubtree=<path>, a change
to that part would be a good candidate to use "format-patch" with
the "--relative=<path>".  We didn't consider it a use case that is
interesting enough, but it may be, which gives another reason why
documenting "--[no-]relative[=<path>]" for the command may not be a
bad thing, not just for the purpose of defeating "diff.relative".

For that reason, it may even make sense to use the same text for
--no-relative and --relative[=<path>] we use for other commands in
the diff family, i.e. leaving only -R inside the ifndef block.

Thanks.

> -----------------8<--------------
> From: =?UTF-8?q?=C4=90o=C3=A0n=20Tr=E1=BA=A7n=20C=C3=B4ng=20Danh?=
>  <congdanhqx@gmail.com>
> Date: Tue, 19 May 2020 07:24:48 +0700
> Subject: [PATCH] Documentation: document --no-relative for format-patch
> MIME-Version: 1.0
> Content-Type: text/plain; charset=UTF-8
> Content-Transfer-Encoding: 8bit
>
> git-format-patch(1) accepts --relative for a long time, but the
> generated patch couldn't be applied without accompanying information.
>
> Hence, we've never document --relative for git-format-patch(1).
>
> Now, we've introduced `diff.relative` config, let's tell Git's users
> that they have an option to override it.
>
> Signed-off-by: Đoàn Trần Công Danh <congdanhqx@gmail.com>
> ---
>  Documentation/diff-options.txt | 5 +++++
>  1 file changed, 5 insertions(+)
>
> diff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt
> index 167b451b89..0f5c108324 100644
> --- a/Documentation/diff-options.txt
> +++ b/Documentation/diff-options.txt
> @@ -654,6 +654,11 @@ ifndef::git-format-patch[]
>  	`--no-relative` can be used to countermand both `diff.relative` config
>  	option and previous `--relative`.
>  endif::git-format-patch[]
> +ifdef::git-format-patch[]
> +--no-relative::
> +	Countermand `diff.relative` config. Show all changes in the repository
> +	with pathname relative to top-level directory of the repository.
> +endif::git-format-patch[]
>  
>  -a::
>  --text::

^ permalink raw reply	[flat|nested] 39+ messages in thread

* [PATCH v7] diff: add config option relative
  2020-05-19 18:00                           ` Junio C Hamano
@ 2020-05-19 19:39                             ` Laurent Arnoud
  2020-05-19 23:01                               ` Đoàn Trần Công Danh
  0 siblings, 1 reply; 39+ messages in thread
From: Laurent Arnoud @ 2020-05-19 19:39 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: Đoàn Trần Công Danh, phillip.wood,
	brian m. carlson, git

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 earlier `--relative`

Add for git-format-patch(1) options documentation `--relative` and
`--no-relative`

Signed-off-by: Laurent Arnoud <laurent@spkdev.net>
---
 Documentation/config/diff.txt  |  4 ++
 Documentation/diff-options.txt |  5 ++-
 diff.c                         | 11 +++--
 t/t4014-format-patch.sh        | 13 ++++++
 t/t4045-diff-relative.sh       | 82 ++++++++++++++++++++++++++++++++--
 5 files changed, 108 insertions(+), 7 deletions(-)

diff --git a/Documentation/config/diff.txt b/Documentation/config/diff.txt
index ff09f1cf73..c3ae136eba 100644
--- 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.
+
 diff.orderFile::
 	File indicating how to order files within a diff.
 	See the '-O' option to linkgit:git-diff[1] for details.
diff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt
index bb31f0c42b..7987d72b02 100644
--- a/Documentation/diff-options.txt
+++ b/Documentation/diff-options.txt
@@ -643,15 +643,18 @@ ifndef::git-format-patch[]
 -R::
 	Swap two inputs; that is, show differences from index or
 	on-disk file to tree contents.
+endif::git-format-patch[]

 --relative[=<path>]::
+--no-relative::
 	When run from a subdirectory of the project, it can be
 	told to exclude changes outside the directory and show
 	pathnames relative to it with this option.  When you are
 	not in a subdirectory (e.g. in a bare repository), you
 	can name which subdirectory to make the output relative
 	to by giving a <path> as an argument.
-endif::git-format-patch[]
+	`--no-relative` can be used to countermand both `diff.relative` config
+	option and previous `--relative`.

 -a::
 --text::
diff --git a/diff.c b/diff.c
index d1ad6a3c4a..863da896c0 100644
--- a/diff.c
+++ b/diff.c
@@ -48,6 +48,7 @@ static const char *diff_order_file_cfg;
 int diff_auto_refresh_index = 1;
 static int diff_mnemonic_prefix;
 static int diff_no_prefix;
+static int diff_relative;
 static int diff_stat_graph_width;
 static int diff_dirstat_permille_default = 30;
 static struct diff_options default_diff_options;
@@ -386,6 +387,10 @@ int git_diff_ui_config(const char *var, const char *value, void *cb)
 		diff_no_prefix = git_config_bool(var, value);
 		return 0;
 	}
+	if (!strcmp(var, "diff.relative")) {
+		diff_relative = git_config_bool(var, value);
+		return 0;
+	}
 	if (!strcmp(var, "diff.statgraphwidth")) {
 		diff_stat_graph_width = git_config_int(var, value);
 		return 0;
@@ -4538,6 +4543,7 @@ void repo_diff_setup(struct repository *r, struct diff_options *options)
 	options->interhunkcontext = diff_interhunk_context_default;
 	options->ws_error_highlight = ws_error_highlight_default;
 	options->flags.rename_empty = 1;
+	options->flags.relative_name = diff_relative;
 	options->objfind = NULL;

 	/* pathchange left =NULL by default */
@@ -5195,8 +5201,7 @@ static int diff_opt_relative(const struct option *opt,
 {
 	struct diff_options *options = opt->value;

-	BUG_ON_OPT_NEG(unset);
-	options->flags.relative_name = 1;
+	options->flags.relative_name = !unset;
 	if (arg)
 		options->prefix = arg;
 	return 0;
@@ -5492,7 +5497,7 @@ static void prep_parse_options(struct diff_options *options)
 		OPT_GROUP(N_("Other diff options")),
 		OPT_CALLBACK_F(0, "relative", options, N_("<prefix>"),
 			       N_("when run from subdir, exclude changes outside and show relative paths"),
-			       PARSE_OPT_NONEG | PARSE_OPT_OPTARG,
+			       PARSE_OPT_OPTARG,
 			       diff_opt_relative),
 		OPT_BOOL('a', "text", &options->flags.text,
 			 N_("treat all files as text")),
diff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh
index db7e733af9..575e079cc2 100755
--- a/t/t4014-format-patch.sh
+++ b/t/t4014-format-patch.sh
@@ -1602,6 +1602,19 @@ test_expect_success 'format patch ignores color.ui' '
 	test_cmp expect actual
 '

+test_expect_success 'format patch respects diff.relative' '
+	rm -rf subdir &&
+	mkdir subdir &&
+	echo other content >subdir/file2 &&
+	git add subdir/file2 &&
+	git commit -F msg &&
+	test_unconfig diff.relative &&
+	git format-patch --relative=subdir --stdout -1 >expect &&
+	test_config diff.relative true &&
+	git -C subdir format-patch --stdout -1 >actual &&
+	test_cmp expect actual
+'
+
 test_expect_success 'cover letter with invalid --cover-from-description and config' '
 	test_config branch.rebuild-1.description "config subject

diff --git a/t/t4045-diff-relative.sh b/t/t4045-diff-relative.sh
index 258808708e..6b026c36a5 100755
--- a/t/t4045-diff-relative.sh
+++ b/t/t4045-diff-relative.sh
@@ -8,7 +8,8 @@ test_expect_success 'setup' '
 	echo content >file1 &&
 	mkdir subdir &&
 	echo other content >subdir/file2 &&
-	blob=$(git hash-object subdir/file2) &&
+	blob_file1=$(git hash-object file1) &&
+	blob_file2=$(git hash-object subdir/file2) &&
 	git add . &&
 	git commit -m one
 '
@@ -18,7 +19,7 @@ check_diff () {
 	shift
 	expect=$1
 	shift
-	short_blob=$(git rev-parse --short $blob)
+	short_blob=$(git rev-parse --short $blob_file2)
 	cat >expected <<-EOF
 	diff --git a/$expect b/$expect
 	new file mode 100644
@@ -70,7 +71,7 @@ check_raw () {
 	expect=$1
 	shift
 	cat >expected <<-EOF
-	:000000 100644 $ZERO_OID $blob A	$expect
+	:000000 100644 $ZERO_OID $blob_file2 A	$expect
 	EOF
 	test_expect_success "--raw $*" "
 		git -C '$dir' diff --no-abbrev --raw $* HEAD^ >actual &&
@@ -86,4 +87,79 @@ do
 	check_$type . dir/file2 --relative=sub
 done

+check_diff_relative_option () {
+	dir=$1
+	shift
+	expect=$1
+	shift
+	relative_opt=$1
+	shift
+	test_expect_success "config diff.relative $relative_opt -p $*" "
+		short_blob=$(git rev-parse --short $blob_file2) &&
+		cat >expected <<-EOF &&
+		diff --git a/$expect b/$expect
+		new file mode 100644
+		index 0000000..\$short_blob
+		--- /dev/null
+		+++ b/$expect
+		@@ -0,0 +1 @@
+		+other content
+		EOF
+		test_config -C $dir diff.relative $relative_opt &&
+		git -C '$dir' diff -p $* HEAD^ >actual &&
+		test_cmp expected actual
+	"
+}
+
+check_diff_no_relative_option () {
+	dir=$1
+	shift
+	expect=$1
+	shift
+	relative_opt=$1
+	shift
+	test_expect_success "config diff.relative $relative_opt -p $*" "
+		short_blob_file1=$(git rev-parse --short $blob_file1) &&
+		short_blob_file2=$(git rev-parse --short $blob_file2) &&
+		cat >expected <<-EOF &&
+		diff --git a/file1 b/file1
+		new file mode 100644
+		index 0000000..\$short_blob_file1
+		--- /dev/null
+		+++ b/file1
+		@@ -0,0 +1 @@
+		+content
+		diff --git a/$expect b/$expect
+		new file mode 100644
+		index 0000000..\$short_blob_file2
+		--- /dev/null
+		+++ b/$expect
+		@@ -0,0 +1 @@
+		+other content
+		EOF
+		test_config -C $dir diff.relative $relative_opt &&
+		git -C '$dir' diff -p $* HEAD^ >actual &&
+		test_cmp expected actual
+	"
+}
+
+check_diff_no_relative_option . subdir/file2 false
+check_diff_no_relative_option . subdir/file2 true --no-relative
+check_diff_no_relative_option . subdir/file2 false --no-relative
+check_diff_no_relative_option subdir subdir/file2 false
+check_diff_no_relative_option subdir subdir/file2 true --no-relative
+check_diff_no_relative_option subdir subdir/file2 false --no-relative
+
+check_diff_relative_option . file2 false --relative=subdir/
+check_diff_relative_option . file2 false --relative=subdir
+check_diff_relative_option . file2 true --relative=subdir/
+check_diff_relative_option . file2 true --relative=subdir
+check_diff_relative_option subdir file2 false --relative
+check_diff_relative_option subdir file2 true --relative
+check_diff_relative_option subdir file2 true
+check_diff_relative_option subdir file2 false --no-relative --relative
+check_diff_relative_option subdir file2 true --no-relative --relative
+check_diff_relative_option . file2 false --no-relative --relative=subdir
+check_diff_relative_option . file2 true --no-relative --relative=subdir
+
 test_done
--
2.26.2

^ permalink raw reply related	[flat|nested] 39+ messages in thread

* Re: [PATCH v7] diff: add config option relative
  2020-05-19 19:39                             ` [PATCH v7] " Laurent Arnoud
@ 2020-05-19 23:01                               ` Đoàn Trần Công Danh
  2020-05-22 10:46                                 ` [PATCH v8] " Laurent Arnoud
  2020-05-22 10:48                                 ` [PATCH v7] " Laurent Arnoud
  0 siblings, 2 replies; 39+ messages in thread
From: Đoàn Trần Công Danh @ 2020-05-19 23:01 UTC (permalink / raw)
  To: Laurent Arnoud; +Cc: Junio C Hamano, phillip.wood, brian m. carlson, git

On 2020-05-19 21:39:02+0200, Laurent Arnoud <laurent@spkdev.net> wrote:
> 
> +check_diff_relative_option () {
> +	dir=$1
> +	shift
> +	expect=$1
> +	shift
> +	relative_opt=$1
> +	shift
> +	test_expect_success "config diff.relative $relative_opt -p $*" "
> +		short_blob=$(git rev-parse --short $blob_file2) &&

We need to quote the first dollar, in order to run that command
inside the test.  Current version will run that git-rev-parse
outside of the test and interpolated into this command by outer shell.

	short_blob=\$(git rev-parse --short $blob_file2) &&

Sorry, I didn't think about this earlier.

> +	test_expect_success "config diff.relative $relative_opt -p $*" "
> +		short_blob_file1=$(git rev-parse --short $blob_file1) &&
> +		short_blob_file2=$(git rev-parse --short $blob_file2) &&

This test also needs to quote that dollar.
Beside that, LGTM.


-- 
Danh

^ permalink raw reply	[flat|nested] 39+ messages in thread

* [PATCH v8] diff: add config option relative
  2020-05-19 23:01                               ` Đoàn Trần Công Danh
@ 2020-05-22 10:46                                 ` Laurent Arnoud
  2020-05-23  2:14                                   ` Đoàn Trần Công Danh
  2020-05-22 10:48                                 ` [PATCH v7] " Laurent Arnoud
  1 sibling, 1 reply; 39+ messages in thread
From: Laurent Arnoud @ 2020-05-22 10:46 UTC (permalink / raw)
  To: Đoàn Trần Công Danh
  Cc: Junio C Hamano, phillip.wood, brian m. carlson, git

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 earlier `--relative`

Add for git-format-patch(1) options documentation `--relative` and
`--no-relative`

Signed-off-by: Laurent Arnoud <laurent@spkdev.net>
---
 Documentation/config/diff.txt  |  4 ++
 Documentation/diff-options.txt |  5 ++-
 diff.c                         | 11 +++--
 t/t4014-format-patch.sh        | 13 ++++++
 t/t4045-diff-relative.sh       | 82 ++++++++++++++++++++++++++++++++--
 5 files changed, 108 insertions(+), 7 deletions(-)

diff --git a/Documentation/config/diff.txt b/Documentation/config/diff.txt
index ff09f1cf73..c3ae136eba 100644
--- 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.
+
 diff.orderFile::
 	File indicating how to order files within a diff.
 	See the '-O' option to linkgit:git-diff[1] for details.
diff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt
index bb31f0c42b..7987d72b02 100644
--- a/Documentation/diff-options.txt
+++ b/Documentation/diff-options.txt
@@ -643,15 +643,18 @@ ifndef::git-format-patch[]
 -R::
 	Swap two inputs; that is, show differences from index or
 	on-disk file to tree contents.
+endif::git-format-patch[]

 --relative[=<path>]::
+--no-relative::
 	When run from a subdirectory of the project, it can be
 	told to exclude changes outside the directory and show
 	pathnames relative to it with this option.  When you are
 	not in a subdirectory (e.g. in a bare repository), you
 	can name which subdirectory to make the output relative
 	to by giving a <path> as an argument.
-endif::git-format-patch[]
+	`--no-relative` can be used to countermand both `diff.relative` config
+	option and previous `--relative`.

 -a::
 --text::
diff --git a/diff.c b/diff.c
index d1ad6a3c4a..863da896c0 100644
--- a/diff.c
+++ b/diff.c
@@ -48,6 +48,7 @@ static const char *diff_order_file_cfg;
 int diff_auto_refresh_index = 1;
 static int diff_mnemonic_prefix;
 static int diff_no_prefix;
+static int diff_relative;
 static int diff_stat_graph_width;
 static int diff_dirstat_permille_default = 30;
 static struct diff_options default_diff_options;
@@ -386,6 +387,10 @@ int git_diff_ui_config(const char *var, const char *value, void *cb)
 		diff_no_prefix = git_config_bool(var, value);
 		return 0;
 	}
+	if (!strcmp(var, "diff.relative")) {
+		diff_relative = git_config_bool(var, value);
+		return 0;
+	}
 	if (!strcmp(var, "diff.statgraphwidth")) {
 		diff_stat_graph_width = git_config_int(var, value);
 		return 0;
@@ -4538,6 +4543,7 @@ void repo_diff_setup(struct repository *r, struct diff_options *options)
 	options->interhunkcontext = diff_interhunk_context_default;
 	options->ws_error_highlight = ws_error_highlight_default;
 	options->flags.rename_empty = 1;
+	options->flags.relative_name = diff_relative;
 	options->objfind = NULL;

 	/* pathchange left =NULL by default */
@@ -5195,8 +5201,7 @@ static int diff_opt_relative(const struct option *opt,
 {
 	struct diff_options *options = opt->value;

-	BUG_ON_OPT_NEG(unset);
-	options->flags.relative_name = 1;
+	options->flags.relative_name = !unset;
 	if (arg)
 		options->prefix = arg;
 	return 0;
@@ -5492,7 +5497,7 @@ static void prep_parse_options(struct diff_options *options)
 		OPT_GROUP(N_("Other diff options")),
 		OPT_CALLBACK_F(0, "relative", options, N_("<prefix>"),
 			       N_("when run from subdir, exclude changes outside and show relative paths"),
-			       PARSE_OPT_NONEG | PARSE_OPT_OPTARG,
+			       PARSE_OPT_OPTARG,
 			       diff_opt_relative),
 		OPT_BOOL('a', "text", &options->flags.text,
 			 N_("treat all files as text")),
diff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh
index db7e733af9..575e079cc2 100755
--- a/t/t4014-format-patch.sh
+++ b/t/t4014-format-patch.sh
@@ -1602,6 +1602,19 @@ test_expect_success 'format patch ignores color.ui' '
 	test_cmp expect actual
 '

+test_expect_success 'format patch respects diff.relative' '
+	rm -rf subdir &&
+	mkdir subdir &&
+	echo other content >subdir/file2 &&
+	git add subdir/file2 &&
+	git commit -F msg &&
+	test_unconfig diff.relative &&
+	git format-patch --relative=subdir --stdout -1 >expect &&
+	test_config diff.relative true &&
+	git -C subdir format-patch --stdout -1 >actual &&
+	test_cmp expect actual
+'
+
 test_expect_success 'cover letter with invalid --cover-from-description and config' '
 	test_config branch.rebuild-1.description "config subject

diff --git a/t/t4045-diff-relative.sh b/t/t4045-diff-relative.sh
index 258808708e..7be1de736d 100755
--- a/t/t4045-diff-relative.sh
+++ b/t/t4045-diff-relative.sh
@@ -8,7 +8,8 @@ test_expect_success 'setup' '
 	echo content >file1 &&
 	mkdir subdir &&
 	echo other content >subdir/file2 &&
-	blob=$(git hash-object subdir/file2) &&
+	blob_file1=$(git hash-object file1) &&
+	blob_file2=$(git hash-object subdir/file2) &&
 	git add . &&
 	git commit -m one
 '
@@ -18,7 +19,7 @@ check_diff () {
 	shift
 	expect=$1
 	shift
-	short_blob=$(git rev-parse --short $blob)
+	short_blob=$(git rev-parse --short $blob_file2)
 	cat >expected <<-EOF
 	diff --git a/$expect b/$expect
 	new file mode 100644
@@ -70,7 +71,7 @@ check_raw () {
 	expect=$1
 	shift
 	cat >expected <<-EOF
-	:000000 100644 $ZERO_OID $blob A	$expect
+	:000000 100644 $ZERO_OID $blob_file2 A	$expect
 	EOF
 	test_expect_success "--raw $*" "
 		git -C '$dir' diff --no-abbrev --raw $* HEAD^ >actual &&
@@ -86,4 +87,79 @@ do
 	check_$type . dir/file2 --relative=sub
 done

+check_diff_relative_option () {
+	dir=$1
+	shift
+	expect=$1
+	shift
+	relative_opt=$1
+	shift
+	test_expect_success "config diff.relative $relative_opt -p $*" "
+		short_blob=\$(git rev-parse --short $blob_file2) &&
+		cat >expected <<-EOF &&
+		diff --git a/$expect b/$expect
+		new file mode 100644
+		index 0000000..\$short_blob
+		--- /dev/null
+		+++ b/$expect
+		@@ -0,0 +1 @@
+		+other content
+		EOF
+		test_config -C $dir diff.relative $relative_opt &&
+		git -C '$dir' diff -p $* HEAD^ >actual &&
+		test_cmp expected actual
+	"
+}
+
+check_diff_no_relative_option () {
+	dir=$1
+	shift
+	expect=$1
+	shift
+	relative_opt=$1
+	shift
+	test_expect_success "config diff.relative $relative_opt -p $*" "
+		short_blob_file1=\$(git rev-parse --short $blob_file1) &&
+		short_blob_file2=\$(git rev-parse --short $blob_file2) &&
+		cat >expected <<-EOF &&
+		diff --git a/file1 b/file1
+		new file mode 100644
+		index 0000000..\$short_blob_file1
+		--- /dev/null
+		+++ b/file1
+		@@ -0,0 +1 @@
+		+content
+		diff --git a/$expect b/$expect
+		new file mode 100644
+		index 0000000..\$short_blob_file2
+		--- /dev/null
+		+++ b/$expect
+		@@ -0,0 +1 @@
+		+other content
+		EOF
+		test_config -C $dir diff.relative $relative_opt &&
+		git -C '$dir' diff -p $* HEAD^ >actual &&
+		test_cmp expected actual
+	"
+}
+
+check_diff_no_relative_option . subdir/file2 false
+check_diff_no_relative_option . subdir/file2 true --no-relative
+check_diff_no_relative_option . subdir/file2 false --no-relative
+check_diff_no_relative_option subdir subdir/file2 false
+check_diff_no_relative_option subdir subdir/file2 true --no-relative
+check_diff_no_relative_option subdir subdir/file2 false --no-relative
+
+check_diff_relative_option . file2 false --relative=subdir/
+check_diff_relative_option . file2 false --relative=subdir
+check_diff_relative_option . file2 true --relative=subdir/
+check_diff_relative_option . file2 true --relative=subdir
+check_diff_relative_option subdir file2 false --relative
+check_diff_relative_option subdir file2 true --relative
+check_diff_relative_option subdir file2 true
+check_diff_relative_option subdir file2 false --no-relative --relative
+check_diff_relative_option subdir file2 true --no-relative --relative
+check_diff_relative_option . file2 false --no-relative --relative=subdir
+check_diff_relative_option . file2 true --no-relative --relative=subdir
+
 test_done
--
2.26.2

^ permalink raw reply related	[flat|nested] 39+ messages in thread

* Re: [PATCH v7] diff: add config option relative
  2020-05-19 23:01                               ` Đoàn Trần Công Danh
  2020-05-22 10:46                                 ` [PATCH v8] " Laurent Arnoud
@ 2020-05-22 10:48                                 ` Laurent Arnoud
  1 sibling, 0 replies; 39+ messages in thread
From: Laurent Arnoud @ 2020-05-22 10:48 UTC (permalink / raw)
  To: Đoàn Trần Công Danh
  Cc: Junio C Hamano, phillip.wood, brian m. carlson, git

On Wed, May 20, 2020 at 06:01:24AM +0700, Đoàn Trần Công Danh wrote:
> Sorry, I didn't think about this earlier.
> 
> > +	test_expect_success "config diff.relative $relative_opt -p $*" "
> > +		short_blob_file1=$(git rev-parse --short $blob_file1) &&
> > +		short_blob_file2=$(git rev-parse --short $blob_file2) &&
> 
> This test also needs to quote that dollar.
> Beside that, LGTM.

No worries I've submitted a v8 patch, thanks everyone for the reviews

-- 
Laurent

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [PATCH v8] diff: add config option relative
  2020-05-22 10:46                                 ` [PATCH v8] " Laurent Arnoud
@ 2020-05-23  2:14                                   ` Đoàn Trần Công Danh
  0 siblings, 0 replies; 39+ messages in thread
From: Đoàn Trần Công Danh @ 2020-05-23  2:14 UTC (permalink / raw)
  To: Laurent Arnoud; +Cc: Junio C Hamano, phillip.wood, brian m. carlson, git

Hi Laurent,

On 2020-05-22 12:46:18+0200, Laurent Arnoud <laurent@spkdev.net> wrote:
> 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 earlier `--relative`
> 
> Add for git-format-patch(1) options documentation `--relative` and
> `--no-relative`
> 
> Signed-off-by: Laurent Arnoud <laurent@spkdev.net>

Look good to me, I also run those tests and manually tested some
usecase I'm interested in.

-- 
Danh

^ permalink raw reply	[flat|nested] 39+ messages in thread

end of thread, other threads:[~2020-05-23  2:14 UTC | newest]

Thread overview: 39+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-05-15 15:57 [PATCH] diff: add config option relative Laurent Arnoud
2020-05-15 22:22 ` Philip Oakley
2020-05-16 17:30   ` Laurent Arnoud
2020-05-15 23:31 ` brian m. carlson
2020-05-16  0:04   ` Junio C Hamano
2020-05-16 17:35     ` Laurent Arnoud
2020-05-16 17:38     ` [PATCH v3] " Laurent Arnoud
2020-05-16 18:33       ` Phillip Wood
2020-05-16 19:40         ` [PATCH v4] " Laurent Arnoud
2020-05-17  2:14           ` Đoàn Trần Công Danh
2020-05-17  2:48             ` Danh Doan
2020-05-17 15:12             ` Junio C Hamano
2020-05-18  9:40               ` [PATCH v5] " Laurent Arnoud
2020-05-18 11:46                 ` Shourya Shukla
2020-05-18 13:04                   ` Đoàn Trần Công Danh
2020-05-18 17:25                   ` Laurent Arnoud
2020-05-18 13:56                 ` Đoàn Trần Công Danh
2020-05-18 16:57                   ` Junio C Hamano
2020-05-18 19:12                     ` Đoàn Trần Công Danh
2020-05-18 20:37                       ` Junio C Hamano
2020-05-19  0:30                         ` Đoàn Trần Công Danh
2020-05-19 18:00                           ` Junio C Hamano
2020-05-19 19:39                             ` [PATCH v7] " Laurent Arnoud
2020-05-19 23:01                               ` Đoàn Trần Công Danh
2020-05-22 10:46                                 ` [PATCH v8] " Laurent Arnoud
2020-05-23  2:14                                   ` Đoàn Trần Công Danh
2020-05-22 10:48                                 ` [PATCH v7] " Laurent Arnoud
2020-05-18 17:03                   ` [PATCH v5] " Junio C Hamano
2020-05-18 17:21                     ` [PATCH v6] " Laurent Arnoud
2020-05-18 17:31                       ` Junio C Hamano
2020-05-18 17:33                         ` Junio C Hamano
2020-05-18 17:34                       ` Eric Sunshine
2020-05-18 19:19                       ` Đoàn Trần Công Danh
2020-05-18 20:02                         ` Junio C Hamano
2020-05-18 17:32                   ` [PATCH v5] " Laurent Arnoud
2020-05-18 16:19                 ` Eric Sunshine
2020-05-18 17:26                   ` Laurent Arnoud
2020-05-18  9:43             ` [PATCH v4] " Laurent Arnoud
2020-05-17 15:07         ` [PATCH v3] " Junio C Hamano

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).