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

From: Sangeeta <sangunb09@gmail.com>
To: Git List <git@vger.kernel.org>
Cc: Phillip Wood <phillip.wood123@gmail.com>,
	Kaartic Sivaraam <kaartic.sivaraam@gmail.com>,
	Junio C Hamano <gitster@pobox.com>,
	Johannes Schindelin <Johannes.Schindelin@gmx.de>
Subject: Re: [Outreachy][PATCH v6] diff: do not show submodule with untracked files as "-dirty"
Date: Tue, 3 Nov 2020 16:16:49 +0530	[thread overview]
Message-ID: <CAHjREB7W2P9_P4LoMHVVzV_YR5-_51zdbRZ0fpEDVkgkd7sh8w@mail.gmail.com> (raw)
In-Reply-To: <20201026175742.33356-1-sangunb09@gmail.com>

Hey Johannes and Junio,

In continuation of the discussion from here[1]

[1] https://lore.kernel.org/git/nycvar.QRO.7.76.6.2011020251520.18437@tvgsbejvaqbjf.bet/

> >> * sj/untracked-files-in-submodule-directory-is-not-dirty (2020-10-26) 1 commit
> >>  - diff: do not show submodule with untracked files as "-dirty"
> >>
> >>  "git diff" showed a submodule working tree with untracked cruft as
> >>  "Submodule commit <objectname>-dirty", but a natural expectation is
> >>  that the "-dirty" indicator would align with "git describe --dirty",
> >>  which does not consider having untracked files in the working tree
> >>  as source of dirtiness.  The inconsistency has been fixed.
> >>
> >>  Needs doc update.

Do I need to add the doc update in the same patch?

> > I *think* the original rationale for marking submodules with untracked
> > (_un-ignored_) files was to avoid deleting a submodule that has
> > uncommitted (because untracked) files.
>
> I agree with you that that the motivation was exactly that, but I
> have a suspicion that its execution was misguided.

I am really very sorry if I ended up adding some unwanted code.

>
> When one has a subdirectory D where one might or might not have an
> untracked new file that one does not want to lose, but one otherwise
> would want to clean up (perhaps an errant process created tons of
> garbage files in the directory), one would not
>
>         git diff D
>
> to see if there are important changes in that directory before doing
>
>         rm -rf D && git checkout D
>
> to bring it back to pristine state, exactly because one would not
> want to lose newly-created but not added files.  One would instead
> use
>
>         git status D
>
> for checking.  Why would a user change the habit when D happens to
> be a submodule?
>
> So I would say that "git status [D]", if it does not let the user
> notice that there is untracked contents in the submodule working
> tree, would be a bad idea.  "git diff" that ignores untracked paths
> in the submodule working tree, on the other hand, is a good thing.
>
> Now, the patch as-is may change the behaviour of "git status D" in
> this case, and the internal invocation of diff-files made somewhere
> in wt-status.c may have to be adjusted to keep such a submodule with
> forgotten newly created files shown as modified.
>
> Further discussion on this should be done on the original review
> thread for continuity, not here, in any case.
>

So we want that `git status` should behave in the same way as it was
behaving before? Can we do this by passing --ignore-submodules=none as
the default args for status? Another approach might be to figure out
how the diff is being called(like is this being called from git status
or git diff) and then add the --ignore-submodules=untracked behavior
to it accordingly. Though I have no idea how to do that now.

Please correct me if I am completely going in the wrong direction. As
it is my first time contributing to git, so any help would be greatly
beneficial for me.

Thanks and regards,
Sangeeta