Re: [PATCH 2/2] difftool: allow running outside Git worktrees with --no-index

From: Jeff King <peff@peff.net>
Date: Mar 15, 2019, 03:08 UTC
Message-ID: <20190315030811.GA28943@sigill.intra.peff.net>
In-Reply-To: <nycvar.QRO.7.76.6.1903141305550.41@tvgsbejvaqbjf.bet>

On Thu, Mar 14, 2019 at 01:16:45PM +0100, Johannes Schindelin wrote:

> In other words, my take is that the ways in which `--no-index` is used are
> probably not very different from one another, and the bugs lurk in
> really rarely exercised code paths.

Yeah, that's more likely (and consistent with the bugs I remember fixing in the last few years).

> > We'd just have to remember to add it back to the argv of diff
> > sub-commands we run.
> 
> It was that "add it back" that I was not keen to implement.

Yes, I saw that we pass on the argv we get back from parse_options() literally to the sub-functions. I was thinking you'd do some trickery with an argv_array. But I like your approach of using OPT_ARGUMENT() much better.

Show 6 quoted lines

> So I was already done with implementing `OPT_ARGUMENT_SEEN()`, based on
> `OPT_ARGUMENT()`, and testing it with my difftool patch, when it occurred
> to me to look what existing users of `OPT_ARGUMENT()` do. Guess what:
> there are none, apart from that test helper used in t0040 to verify that
> `parse_options()` works as intended. And there were none other. In the
> entire commit history.

Heh. That is not the first time I have hit that with the parse-options code.

I see you posted the new patches, so I'll try to give a careful read in that part of the thread.

-Peff