Re: [OUTREACHY][PATCH] diff: do not show submodule with untracked files as "-dirty"

From: Sangeeta NB <sangunb09@gmail.com>
Date: Oct 20, 2020, 18:10 UTC
Message-ID: <CAHjREB7Xhga3da9vGyXxJo6MCi9P9CyezzO66Xh2X2+rDFkQjw@mail.gmail.com>
In-Reply-To: <7af89857-5495-d008-3d8f-09b5fed32775@gmail.com>

Hey Phillip,

> I think we require your full name on the From line to match the
> Signed-off-by line below (c.f.
> https://lore.kernel.org/git/xmqqpn5dd5dv.fsf@gitster.c.googlers.com)

> That is a good summary of the issue that this change addresses, we
> normally wrap lines at 72 characters though.

This was sent by gitgitgadget so I am unable to find how I can customize it. Can you help me with this? Or else I have set the Travis so should I send another patch using send-email?

> This unconditionally overrides diff.ignoreSubmodules, grepping seems to
> show that we don't have any tests that test a config value of "none".
> There are a few that check "dirty" which we should look at to consider
> if they still add value now we've changed the default.

Yes, sorry, noticed it now. I have now added a flag in diff_flags. Also, do I have to add tests that check whether diff.ignoreSubmodules is not being overwritten?

Show 18 quoted lines

>       if (diff_no_prefix) {
>               options->a_prefix = options->b_prefix = "";
> diff --git a/submodule.c b/submodule.c
> index b3bb59f066..762298c010 100644
> --- a/submodule.c
> +++ b/submodule.c
> @@ -1678,6 +1678,8 @@ unsigned is_submodule_modified(const char *path, int ignore_untracked)
>       strvec_pushl(&cp.args, "status", "--porcelain=2", NULL);
>       if (ignore_untracked)
>               strvec_push(&cp.args, "-uno");
> +     else
> +        strvec_push (&cp.args, "--ignore-submodules=none");
>
> We need to do this as a consequence of changing the default for
> '--ignore-submodules`, we should think what the consequences are for
> other users of `git status` and whether we need to do something similar
> there if diff.ignoreSubmodules is not set.
>

Oh okay. I understood what you said. But I couldn't figure out how to do that. As all the tests of status were passing so I don't think there might be any issue with this.

Show 6 quoted lines

>
> I think we want to keep testing that we get the correct output when
> --ignore-submodules=none as well as maybe adding a couple of new tests
> that check the default in this file, rather than changing the expected
> output.
>

Thanks, I have added more tests.

Thanks and Regards, Sangeeta