Hey everyone, I created a PR at gitgitgadget here[1] but it is failing at three tests of git rm[2]. I looked at the behavior of git status at some other places( by pausing 'git diff HEAD with dirty submodule(untracked)' in t/t4027-diff-submodule.sh and looking at `git status` behavior) but it was working perfectly fine(was giving what output was expected). But here[2] I couldn't understand why is it failing. Can someone please have a look at the PR and give me some pointers? I know I am asking way out of too much but I tried a lot on what could have been missing but couldn't find anything. [1] https://github.com/gitgitgadget/git/pull/751 [2] https://github.com/git/git/blob/master/t/t3600-rm.sh#L691 Regards, Sangeeta On Mon, Oct 12, 2020 at 9:27 PM Junio C Hamano <gitster@xxxxxxxxx> wrote: > > Sangeeta NB <sangunb09@xxxxxxxxx> 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.
