threads / patch / 53479

patchdiff: add config option relative

Subject: [PATCH] diff: add config option relative

## tl;dr

39 messages between May 15, 2020 and May 23, 2020. Diffs are folded; open one to read it.

replies: 38people: 8as markdown or json

Laurent Arnoud· May 15, 2020, 15:57 UTC · lore

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
Show changes to diff +59 −0
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
Philip Oakley· May 15, 2020, 22:22 UTC · re: Laurent Arnoud · lore

Re: [PATCH] diff: add config option relative

Hi Laurent,
On 15/05/2020 16:57, Laurent Arnoud wrote:
Show 22 quoted lines
> 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
Show 90 quoted lines
> +
>  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
Laurent Arnoud· May 16, 2020, 17:30 UTC · re: Philip Oakley · lore

Re: [PATCH] diff: add config option relative

Hi Philip,
On Fri, May 15, 2020 at 11:22:53PM +0100, Philip Oakley wrote:
Show 8 quoted lines
> 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
brian m. carlson· May 15, 2020, 23:31 UTC · re: Laurent Arnoud · lore

Re: [PATCH] diff: add config option relative

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
Junio C Hamano· May 16, 2020, 00:04 UTC · re: brian m. carlson · lore

Re: [PATCH] diff: add config option relative

"brian m. carlson" <sandals@crustytoothpaste.net> writes:
Show 8 quoted lines
> 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).

Laurent Arnoud· May 16, 2020, 17:35 UTC · re: Junio C Hamano · lore

Re: [PATCH] diff: add config option relative

On Fri, May 15, 2020 at 05:04:43PM -0700, Junio C Hamano wrote:
Show 16 quoted lines
> "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
Laurent Arnoud· May 16, 2020, 17:38 UTC · re: Junio C Hamano · lore

Re: [PATCH v3] diff: add config option 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  |  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
Show changes to 4 files +109 −1

Documentation/config/diff.txt, Documentation/diff-options.txt, diff.c, 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
Phillip Wood· May 16, 2020, 18:33 UTC · re: Laurent Arnoud · lore

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

Hi Laurent
On 16/05/2020 18:38, Laurent Arnoud wrote:
Show 40 quoted lines
> 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

Show 42 quoted lines
>   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
Show 105 quoted lines
>   		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
> 
Laurent Arnoud· May 16, 2020, 19:40 UTC · re: Phillip Wood · lore

Re: [PATCH v4] diff: add config option 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 +++++++++++++++++++++++++++++++++
 4 files changed, 113 insertions(+), 3 deletions(-)
 create mode 100755 t/t9904-diff-relative-config.sh
Show changes to 4 files +113 −4

Documentation/config/diff.txt, Documentation/diff-options.txt, diff.c, 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
Đoàn Trần Công Danh· May 17, 2020, 02:14 UTC · re: Laurent Arnoud · lore

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

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
Show 12 quoted lines
> 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
Show 13 quoted lines
>  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.
Show 14 quoted lines
> +	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`
Show 11 quoted lines
> @@ -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;
Show 8 quoted lines
> +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*

Show changes to t/t9904-diff-relative-config.sh +12 −11
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
Danh Doan· May 17, 2020, 02:48 UTC · re: Đoàn Trần Công Danh · lore

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

Show 10 quoted lines
> 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.

Junio C Hamano· May 17, 2020, 15:12 UTC · re: Đoàn Trần Công Danh · lore

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

Đoàn Trần Công Danh  <congdanhqx@gmail.com> writes:
Show 8 quoted lines
>>  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.
Show 15 quoted lines
>> 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.
Laurent Arnoud· May 18, 2020, 09:40 UTC · re: Junio C Hamano · lore

Re: [PATCH v5] diff: add config option 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 |  2 +
 diff.c                         | 12 +++--
 t/t4045-diff-relative.sh       | 83 ++++++++++++++++++++++++++++++++--
 4 files changed, 95 insertions(+), 6 deletions(-)
Show changes to 4 files +95 −7

Documentation/config/diff.txt, Documentation/diff-options.txt, diff.c, t/t4045-diff-relative.sh

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
Shourya Shukla· May 18, 2020, 11:46 UTC · re: Laurent Arnoud · lore

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

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`
Show 9 quoted lines
> --- 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:
Show 7 quoted lines
> @@ -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

Đoàn Trần Công Danh· May 18, 2020, 13:04 UTC · re: Shourya Shukla · lore

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

Hi Shourya,
On 2020-05-18 17:16:30+0530, Shourya Shukla <shouryashukla.oo@gmail.com> wrote:
Show 5 quoted lines
> 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
Laurent Arnoud· May 18, 2020, 17:25 UTC · re: Shourya Shukla · lore

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

Hi Shourya,
On Mon, May 18, 2020 at 05:16:30PM +0530, Shourya Shukla wrote:
Show 8 quoted lines
> 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
Show 29 quoted lines
> > --- 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
Đoàn Trần Công Danh· May 18, 2020, 13:56 UTC · re: Laurent Arnoud · lore

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

Hi Laurent,
On 2020-05-18 11:40:21+0200, Laurent Arnoud <laurent@spkdev.net> wrote:
Show 7 quoted lines
> 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(-)
Show changes to 2 files +9 −3

builtin/log.c, t/t4045-diff-relative.sh

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
Junio C Hamano· May 18, 2020, 16:57 UTC · re: Đoàn Trần Công Danh · lore

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

Đoàn Trần Công Danh  <congdanhqx@gmail.com> writes:
Show 20 quoted lines
> 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.

Đoàn Trần Công Danh· May 18, 2020, 19:12 UTC · re: Junio C Hamano · lore

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

On 2020-05-18 09:57:09-0700, Junio C Hamano <gitster@pobox.com> wrote:
Show 11 quoted lines
> 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
Junio C Hamano· May 18, 2020, 20:37 UTC · re: Đoàn Trần Công Danh · lore

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

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

Show 8 quoted lines
> 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
Đoàn Trần Công Danh· May 19, 2020, 00:30 UTC · re: Junio C Hamano · lore

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

On 2020-05-18 13:37:55-0700, Junio C Hamano <gitster@pobox.com> wrote:
Show 9 quoted lines
> Đ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(+)
Show changes to Documentation/diff-options.txt +5 −0
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
Junio C Hamano· May 19, 2020, 18:00 UTC · re: Đoàn Trần Công Danh · lore

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

Đoàn Trần Công Danh  <congdanhqx@gmail.com> writes:
Show 7 quoted lines
> 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.
Show 38 quoted lines
> -----------------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::
Laurent Arnoud· May 19, 2020, 19:39 UTC · re: Junio C Hamano · lore

[PATCH v7] 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`

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(-)
Show changes to 5 files +108 −8

Documentation/config/diff.txt, Documentation/diff-options.txt, diff.c, t/t4014-format-patch.sh, t/t4045-diff-relative.sh

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
Đoàn Trần Công Danh· May 19, 2020, 23:01 UTC · re: Laurent Arnoud · lore

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

On 2020-05-19 21:39:02+0200, Laurent Arnoud <laurent@spkdev.net> wrote:
Show 10 quoted lines
> 
> +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
Laurent Arnoud· May 22, 2020, 10:46 UTC · re: Đoàn Trần Công Danh · lore

[PATCH v8] 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`

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(-)
Show changes to 5 files +108 −8

Documentation/config/diff.txt, Documentation/diff-options.txt, diff.c, t/t4014-format-patch.sh, t/t4045-diff-relative.sh

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
Đoàn Trần Công Danh· May 23, 2020, 02:14 UTC · re: Laurent Arnoud · lore

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

Hi Laurent,
On 2020-05-22 12:46:18+0200, Laurent Arnoud <laurent@spkdev.net> wrote:
Show 11 quoted lines
> 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
Laurent Arnoud· May 22, 2020, 10:48 UTC · re: Đoàn Trần Công Danh · lore

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

On Wed, May 20, 2020 at 06:01:24AM +0700, Đoàn Trần Công Danh wrote:
Show 8 quoted lines
> 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
Junio C Hamano· May 18, 2020, 17:03 UTC · re: Đoàn Trần Công Danh · lore

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

Đoàn Trần Công Danh  <congdanhqx@gmail.com> writes:
Show 18 quoted lines
>> @@ -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.
Show 15 quoted lines
>> 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.

Show 21 quoted lines
>
> 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.
Laurent Arnoud· May 18, 2020, 17:21 UTC · re: Junio C Hamano · lore

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

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(-)
Show changes to 5 files +106 −6

Documentation/config/diff.txt, Documentation/diff-options.txt, diff.c, t/t4014-format-patch.sh, t/t4045-diff-relative.sh

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
Junio C Hamano· May 18, 2020, 17:31 UTC · re: Laurent Arnoud · lore

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

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.
Junio C Hamano· May 18, 2020, 17:33 UTC · re: Junio C Hamano · lore

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

Junio C Hamano <gitster@pobox.com> writes:
Show 8 quoted lines
> 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.
Eric Sunshine· May 18, 2020, 17:34 UTC · re: Laurent Arnoud · lore

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

On Mon, May 18, 2020 at 1:22 PM Laurent Arnoud <laurent@spkdev.net> wrote:
Show 20 quoted lines
> 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.)

Show 14 quoted lines
> +               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
> +       "
> +}
Đoàn Trần Công Danh· May 18, 2020, 19:19 UTC · re: Laurent Arnoud · lore

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

On 2020-05-18 19:21:03+0200, Laurent Arnoud <laurent@spkdev.net> wrote:
Show 10 quoted lines
> 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:

Show changes to Documentation/diff-options.txt +1 −0
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
Junio C Hamano· May 18, 2020, 20:02 UTC · re: Đoàn Trần Công Danh · lore

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

Đoàn Trần Công Danh  <congdanhqx@gmail.com> writes:
Show 29 quoted lines
> 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.
Laurent Arnoud· May 18, 2020, 17:32 UTC · re: Đoàn Trần Công Danh · lore

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

Hi Danh,
On Mon, May 18, 2020 at 08:56:56PM +0700, Đoàn Trần Công Danh wrote:
Show 14 quoted lines
> 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

Show 12 quoted lines
> 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
Show 18 quoted lines
> > @@ -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
Eric Sunshine· May 18, 2020, 16:19 UTC · re: Laurent Arnoud · lore

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

On Mon, May 18, 2020 at 5:40 AM Laurent Arnoud <laurent@spkdev.net> wrote:
Show 32 quoted lines
> 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.
Laurent Arnoud· May 18, 2020, 17:26 UTC · re: Eric Sunshine · lore

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

On Mon, May 18, 2020 at 12:19:20PM -0400, Eric Sunshine wrote:
Show 28 quoted lines
> 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
Laurent Arnoud· May 18, 2020, 09:43 UTC · re: Đoàn Trần Công Danh · lore

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

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
Junio C Hamano· May 17, 2020, 15:07 UTC · re: Phillip Wood · lore

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

Phillip Wood <phillip.wood123@gmail.com> writes:
Show 12 quoted lines
>> @@ -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.

← back to recent threads