Re: [PATCH v3 5/5] add -p: ignore dirty submodules

From: Johannes Schindelin <johannes.schindelin@gmx.de>
Date: Aug 31, 2022, 20:05 UTC
Message-ID: <op19530n-27qo-r35s-r1q7-18s3259ps78o@tzk.qr>
In-Reply-To: <c205f8e2-62f9-d4b2-0ccf-51dcabc12224@gmail.com>

Hi Phillip,

On Tue, 30 Aug 2022, Phillip Wood wrote:

Show 14 quoted lines

> On 29/08/2022 16:11, Johannes Schindelin via GitGitGadget wrote:
> > From: Johannes Schindelin <johannes.schindelin@gmx.de>
> >
> > Thanks to alwyas running `diff-index` and `diff-files` with the
> > `--numstat` option (the latter with `--ignore-submodules=dirty`) before
> > even generating any real diff to parse, the Perl version of `git add -p`
> > simply ignored dirty submodules and does not even offer them up for
> > staging.
>
> I had a bit of a hard time understanding this paragraph. To me the fact that
> the perl version is using --numstat is not that important here, what is
> important is that it is using --ignore-submodules=dirty when it generates its
> list of files to show and that information is consigned to a parenthesized
> aside.

Yes, the `--ignore-submodules=dirty` part is important, but so is the `--numstat` part.

It is important because the Perl script would parse that numstat part and only offer files up for further processing for which it saw those numbers. Then it would go ahead and run a full diff on those files, which basically doubled the work.

And it is that doubling of work that I tried to avoid when implementing the built-in version. The bug came about because the full diff call wasn't using the `--ignore-submodules=dirty` option, and that's what I missed.

This is maybe more interesting a story for the cover letter, to be able to understand how this bug was introduced, and maybe to offer an opportunity for others (in addition to myself) to learn from my mistake.

> The fix itself looks good.

Thanks! Dscho