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

[Phillip Wood <phillip.wood123@gmail.com>]

Hi Laurent

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

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

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

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

Best Wishes

Phillip

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