Re: Confusing behavior with ignored submodules and `git commit -a`
- From
Stefan Beller <sbeller@google.com>
- Date
- Nov 27, 2018, 00:03 UTC
- Message-ID
- <CAGZ79kax6NpHP-rouezE-A8a2jVN+sC1WnouFWDcaCqYWhs_9w@mail.gmail.com>
- In-Reply-To
- <CAGw6cBvsiW7nwkJ_DBhXOYkgh7ba+rm+pwd4tOAt1ohRvcCY=A@mail.gmail.com>
On Thu, Nov 15, 2018 at 4:31 PM Michael Forney <mforney@mforney.org> wrote:
Show 21 quoted lines
> > On 2018-11-15, Stefan Beller <sbeller@google.com> wrote: > > On Thu, Nov 15, 2018 at 1:33 PM Michael Forney <mforney@mforney.org> wrote: > >> Well, currently the submodule config can be disabled in diff_flags by > >> setting override_submodule_config=1. However, I'm thinking it may be > >> simpler to selectively *enable* the submodule config in diff_flags > >> where it is needed instead of disabling it everywhere else (i.e. > >> use_submodule_config instead of override_submodule_config). > > > > This sounds like undoing the good(?) part of the series that introduced > > this regression, as before that we selectively loaded the submodule > > config, which lead to confusion when you forgot it. Selectively *enabling* > > the submodule config sounds like that state before? > > > > Or do we *only* talk about enabling the ignore flag, while loading the > > rest of the submodule config automatic? > > Yes, that is what I meant. I believe the automatic loading of > submodule config is the right thing to do, it just uncovered cases > where the current handling of override_submodule_config is not quite > sufficient.
That sounds good.
Show 22 quoted lines
>
> My suggestion of replacing override_submodule_config with
> use_submodule_config is because it seems like there are fewer places
> where we want to apply the ignore config than not. I think it should
> only apply in diffs against the working tree and when staging changes
> to the index (unless specified explicitly). The documentation just
> mentions the "diff family", but all but one of the possible values for
> submodule.<name>.ignore ("all") don't make sense unless comparing with
> the working tree. This is also how show/log -p behaved in git <2.15.
> So I think that clarifying that it is about modifications *to the
> working tree* would be a good idea.
>
> >> I'm also starting to see why this is tricky. The only difference that
> >> diff.c:run_diff_files sees between `git add inner` and `git add --all`
> >> is whether the index entry matched the pathspec exactly or not.
> >
> > Unrelated to the trickiness, I think we'd need to document the behavior
> > of the -a flag in git-add and git-commit better as adding the diff below
> > will depart from the "all" rule again, which I thought was a strong
> > motivator for Brandons series (IIRC).
>
> Can you explain what you mean by the "all" rule?-a as short for --all in git commit, and I presumed it to be '--all-content', but documentation tells me it's about files only, so there is no really an "all" rule, but rather me misunderstanding to what it applies.