Hi Eric,
On 5 Feb 2023, at 12:50, Eric Sunshine wrote:
> On Sat, Feb 4, 2023 at 11:47 PM John Cai via GitGitGadget
> <gitgitgadget@xxxxxxxxx> wrote:
>> It can be useful to specify diff algorithms per file type. For example,
>> one may want to use the minimal diff algorithm for .json files, another
>> for .c files, etc.
>>
>> Teach the diff machinery to check attributes for a diff algorithm.
>> Enforce precedence by favoring the command line option, then looking at
>> attributes, then finally the config.
>>
>> To enforce precedence order, set the `xdl_opts_command_line` member
>> during options pasing to indicate the diff algorithm was set via command
>> line args.
>>
>> Signed-off-by: John Cai <johncai86@xxxxxxxxx>
>> ---
>> diff --git a/diff.c b/diff.c
>> @@ -3652,6 +3652,27 @@ static void builtin_diff(const char *name_a,
>> + if (!o->xdl_opts_command_line) {
>> + static struct attr_check *check;
>
> `check` is declared static...
>
>> + const char *one_diff_algo;
>> + const char *two_diff_algo;
>> +
>> + check = attr_check_alloc();
>
> ... is allocated here...
>
>> + attr_check_append(check, git_attr("diff-algorithm"));
>> +
>> + git_check_attr(the_repository->index, NULL, one->path, check);
>> + one_diff_algo = check->items[0].value;
>> + git_check_attr(the_repository->index, NULL, two->path, check);
>> + two_diff_algo = check->items[0].value;
>> +
>> + if (!ATTR_UNSET(one_diff_algo) && !ATTR_UNSET(two_diff_algo) &&
>> + !strcmp(one_diff_algo, two_diff_algo))
>> + set_diff_algorithm(o, one_diff_algo);
>> +
>> + attr_check_free(check);
>
> ... and freed here...
>
>> + }
>
> ... so the reason for the `static` declaration is not clear. Am I
> missing something obvious?
No, you are correct. No reason for the static declaration. `check` is not used outside of the scope of this
conditional. I think this made it in from an earlier iteration and I didn't catch the oversight.
thanks
John
