Subject: Re: [Outreachy] Introduction

From: Junio C Hamano <[email-masked]>
Date: Mon, 12 Oct 2020 08:57:27 -0700

Sangeeta NB <[email-masked]> writes:

> A fix for making this as the default behaviour can be:
>
> --- a/diff.c
> +++ b/diff.c
> @@ -422,6 +422,7 @@ int git_diff_ui_config(const char *var, const char
> *value, void *cb)
>         if (git_color_config(var, value, cb) < 0)
>                 return -1;
>
> +       handle_ignore_submodules_arg(&default_diff_options, "untracked");
>         return git_diff_basic_config(var, value, cb);

This function is called for each and every element of configuration
item in your ~/.gitconfig and .git/config; by definition, the
default behaviour is what is used when the user did not specify
anything so what is usually done is to do that kind of defaulting
before the code calls git_config() with a callback function like
this.

And more importantly, the users may have

    [diff] ignoresubmodules=<value>

in their configuration file.  After calling handle_ignore_submodules_arg()
with the value the user desires, the above code will overwrite it with
a hardcoded default---at that point that is no longer "the default"
to be used when the user didn't specify.

I am wondering if the init_diff_ui_defaults() function is the right
location to add the above call.

>  }
>
> But this would also involve a lot of changes in the way tests are
> written as 12 out of 19 tests in t4027-diff-submodule.sh failed after
> adding this patch.

If the tests expect that the -dirty suffix is added at the end of
"Subproject commit 2f256705..." when the submodule directory has a
untracked file, it is expected that such tests need to be updated
to the new world order you are introducing, which is "just like 'git
describe --dirty' does not consider having an untracked file does not
make otherwise clean checkout a dirty one, 'git diff' should not
show that a submodule is dirty in its output if its working tree has
an untracked file but is otherwise clean".

What follows is a note for more experienced developers, but I notice
that over the years, we seems to have done a shoddy job adjusting
the implementation in diff.c file in the hope of adding support to
work in multiple repositories; most file-scope static globals like
default_diff_options and diff_detect_rename_default are still only
read while in the main repository, yet repo_diff_setup() pretends as
if an invocation of the diff machinery in a different repository can
use settings that are repository specific.  Again, this is not
something you need to be worried about.