Hi Sangeeta
On 11/10/2020 12:30, Sangeeta NB wrote:
Thanks for the help, Philip.
On Fri, Oct 9, 2020 at 11:59 PM Phillip Wood <phillip.wood123@xxxxxxxxx> wrote:
I struggled to find the mircoprojects page - I must have missed the link
on the outreachy site.
In case anyone else is struggling to find the microprojects page,
here's the link [1]
[1] https://git.github.io/Outreachy-21-Microprojects/
As I understand it if a submodule contains any untracked files (i.e. a
file that has not been added with `git add` and is not ignored by any
.gitignore or .git/info/exclude entries) then running `git diff` in the
superproject will report that the submodule is dirty - there will be a
line something like "+Subproject commit abcdef-dirty". However if we run
`git describe --dirty` in the submodule directory then it will not
append "-dirty" to it's output unless there are changes to tracked files.
On running `git diff HEAD --ignore-submodules=untracked` the submodule
wasn't reported as dirty.
That's great
I guess this is what we are expecting. So should I make it the default
behavior for diff?
I think that is a good route forward, we probably want to change the
default for `diff-index` and `diff-files` as well
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);
}
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. I am working on any other workaround for this. Let
me know whether I am on right path or not. Also any pointers on how to
proceed would be helpful. Thanks!
We'd expect some tests to fail but only the ones that are testing if
untracked files cause the submodule to be considered dirty.
git_diff_ui_config() is a callback that is invoked once per config key
in the config files so I don't think it is a good place to make the
change as it is inefficient and overrides the users'
`diff.ignoreSubmodules` setting . It also only applies to `diff` and not
`diff-index` or `diff-files`. I think it would be better to set the
default in diff_setup() though we need to be careful not to override
`diff.ignoreSubmodules` setting so we might need to add a global flag to
remember if the user has set `diff.ignoreSubmodules` in their config
Best Wishes
Phillip
