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

[Johannes Schindelin <Johannes.Schindelin@gmx.de>]

Hi Phillip,

On Tue, 30 Aug 2022, Phillip Wood wrote:

> 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