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

From: Junio C Hamano <gitster@pobox.com>
Date: Jan 8, 2017, 03:02 UTC
Message-ID: <xmqqtw9a1cfo.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <CAFZEwPMLk5KTeLR9zygU6ZH5zN7TLnQzmJVpyCNig8FrYcOazw@mail.gmail.com>

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

Show 9 quoted lines

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

Show 21 quoted lines

>>  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()?

Show 13 quoted lines

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