[PATCH] diff: add interhunk context config option

[PATCH] diff: add interhunk context config option

[Vegard Nossum]

The --inter-hunk-context= option was added in commit 6d0e674a5754
("diff: add option to show context between close hunks"). This patch
allows configuring a default for this option.

Cc: René Scharfe <l.s.r@web.de>
Signed-off-by: Vegard Nossum <vegard.nossum@oracle.com>
---
 Documentation/diff-options.txt | 2 ++
 diff.c                         | 8 ++++++++
 2 files changed, 10 insertions(+)

diff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt
index e6215c372..163b65444 100644
--- a/Documentation/diff-options.txt
+++ b/Documentation/diff-options.txt
@@ -511,6 +511,8 @@ endif::git-format-patch[]
 --inter-hunk-context=<lines>::
 	Show the context between diff hunks, up to the specified number
 	of lines, thereby fusing hunks that are close to each other.
+	Defaults to `diff.interhunkcontext` or 0 if the config option
+	is unset.
 
 -W::
 --function-context::
diff --git a/diff.c b/diff.c
index 84dba60c4..40b4e6afe 100644
--- a/diff.c
+++ b/diff.c
@@ -33,6 +33,7 @@ static int diff_rename_limit_default = 400;
 static int diff_suppress_blank_empty;
 static int diff_use_color_default = -1;
 static int diff_context_default = 3;
+static int diff_interhunk_context_default = 0;
 static const char *diff_word_regex_cfg;
 static const char *external_diff_cmd_cfg;
 static const char *diff_order_file_cfg;
@@ -248,6 +249,12 @@ int git_diff_ui_config(const char *var, const char *value, void *cb)
 			return -1;
 		return 0;
 	}
+	if (!strcmp(var, "diff.interhunkcontext")) {
+		diff_interhunk_context_default = git_config_int(var, value);
+		if (diff_interhunk_context_default < 0)
+			return -1;
+		return 0;
+	}
 	if (!strcmp(var, "diff.renames")) {
 		diff_detect_rename_default = git_config_rename(var, value);
 		return 0;
@@ -3371,6 +3378,7 @@ void diff_setup(struct diff_options *options)
 	options->rename_limit = -1;
 	options->dirstat_permille = diff_dirstat_permille_default;
 	options->context = diff_context_default;
+	options->interhunkcontext = diff_interhunk_context_default;
 	options->ws_error_highlight = ws_error_highlight_default;
 	DIFF_OPT_SET(options, RENAME_EMPTY);
 
-- 
2.11.0.1.gaa10c3f

Re: [PATCH] diff: add interhunk context config option

[Pranit Bauva]

Hey Vegard,

On Tue, Jan 3, 2017 at 5:01 AM, Vegard Nossum <vegard.nossum@oracle.com> wrote:
> The --inter-hunk-context= option was added in commit 6d0e674a5754
> ("diff: add option to show context between close hunks"). This patch
> allows configuring a default for this option.
>
> Cc: René Scharfe <l.s.r@web.de>
> Signed-off-by: Vegard Nossum <vegard.nossum@oracle.com>
> ---
>  Documentation/diff-options.txt | 2 ++
>  diff.c                         | 8 ++++++++
>  2 files changed, 10 insertions(+)
>
> diff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt
> index e6215c372..163b65444 100644
> --- a/Documentation/diff-options.txt
> +++ b/Documentation/diff-options.txt
> @@ -511,6 +511,8 @@ endif::git-format-patch[]
>  --inter-hunk-context=<lines>::
>         Show the context between diff hunks, up to the specified number
>         of lines, thereby fusing hunks that are close to each other.
> +       Defaults to `diff.interhunkcontext` or 0 if the config option
> +       is unset.

Seems good. But I think you have missed adding the documentation of
this to Documentation/diff-config.txt . Generally whatever config
variable one adds, is added to Documentation/config.txt but in this
case, there exists a separate Documentation/diff-config.txt for all
"diff" related things which is included in Documentation/config.txt .
Also, you need to link the link both the parts of this documentation.
Please refer to commit id aaab84203 so as to know what I am talking
about.

>  -W::
>  --function-context::
> diff --git a/diff.c b/diff.c
> index 84dba60c4..40b4e6afe 100644
> --- a/diff.c
> +++ b/diff.c
> @@ -33,6 +33,7 @@ static int diff_rename_limit_default = 400;
>  static int diff_suppress_blank_empty;
>  static int diff_use_color_default = -1;
>  static int diff_context_default = 3;
> +static int diff_interhunk_context_default = 0;
>  static const char *diff_word_regex_cfg;
>  static const char *external_diff_cmd_cfg;
>  static const char *diff_order_file_cfg;
> @@ -248,6 +249,12 @@ int git_diff_ui_config(const char *var, const char *value, void *cb)
>                         return -1;
>                 return 0;
>         }
> +       if (!strcmp(var, "diff.interhunkcontext")) {
> +               diff_interhunk_context_default = git_config_int(var, value);
> +               if (diff_interhunk_context_default < 0)
> +                       return -1;
> +               return 0;
> +       }
>         if (!strcmp(var, "diff.renames")) {
>                 diff_detect_rename_default = git_config_rename(var, value);
>                 return 0;
> @@ -3371,6 +3378,7 @@ void diff_setup(struct diff_options *options)
>         options->rename_limit = -1;
>         options->dirstat_permille = diff_dirstat_permille_default;
>         options->context = diff_context_default;
> +       options->interhunkcontext = diff_interhunk_context_default;
>         options->ws_error_highlight = ws_error_highlight_default;
>         DIFF_OPT_SET(options, RENAME_EMPTY);

On a first look, it seems that we can overwrite the default config
values by using a different command line argument which is good.

Also, tests are missing. It seems that t/t4032 might be a good place
to add those tests.

Rest all is quite good! :)

Regards,
Pranit Bauva

Re: [PATCH] diff: add interhunk context config option

[Junio C Hamano]

Pranit Bauva <pranit.bauva@gmail.com> writes:

>> diff --git a/diff.c b/diff.c
>> index 84dba60c4..40b4e6afe 100644
>> --- a/diff.c
>> +++ b/diff.c
>> @@ -33,6 +33,7 @@ static int diff_rename_limit_default = 400;
>>  static int diff_suppress_blank_empty;
>>  static int diff_use_color_default = -1;
>>  static int diff_context_default = 3;
>> +static int diff_interhunk_context_default = 0;

Do not explicitly initialize BSS variables to 0 (or NULL).

>>  static const char *diff_word_regex_cfg;
>>  static const char *external_diff_cmd_cfg;
>>  static const char *diff_order_file_cfg;
>> @@ -248,6 +249,12 @@ int git_diff_ui_config(const char *var, const char *value, void *cb)
>>                         return -1;
>>                 return 0;
>>         }
>> +       if (!strcmp(var, "diff.interhunkcontext")) {
>> +               diff_interhunk_context_default = git_config_int(var, value);
>> +               if (diff_interhunk_context_default < 0)
>> +                       return -1;
>> +               return 0;
>> +       }
>>         if (!strcmp(var, "diff.renames")) {
>>                 diff_detect_rename_default = git_config_rename(var, value);
>>                 return 0;
>> @@ -3371,6 +3378,7 @@ void diff_setup(struct diff_options *options)
>>         options->rename_limit = -1;
>>         options->dirstat_permille = diff_dirstat_permille_default;
>>         options->context = diff_context_default;
>> +       options->interhunkcontext = diff_interhunk_context_default;

Will this receive -1 if diff.interhunkcontext configuration variable
is misconfigured and the *_default variable receives the error return
value from git_config_int()?


>>         options->ws_error_highlight = ws_error_highlight_default;
>>         DIFF_OPT_SET(options, RENAME_EMPTY);
>
> On a first look, it seems that we can overwrite the default config
> values by using a different command line argument which is good.
>
> Also, tests are missing. It seems that t/t4032 might be a good place
> to add those tests.
>
> Rest all is quite good! :)
>
> Regards,
> Pranit Bauva