Re: git commit file completion recently broke

From: Jacob Keller <jacob.keller@gmail.com>
Date: Dec 7, 2017, 00:38 UTC
Message-ID: <CA+P7+xpoaa5zaF13ageKKjpPKTECU6XjaRdSZy6WOP7Q0TX+yA@mail.gmail.com>
In-Reply-To: <20171207002439.GB21003@sigill.intra.peff.net>

On Wed, Dec 6, 2017 at 4:24 PM, Jeff King <peff@peff.net> wrote:

Show 25 quoted lines

> On Wed, Dec 06, 2017 at 07:22:35PM -0500, Jeff King wrote:
>
>> On Wed, Dec 06, 2017 at 04:01:51PM -0800, Jacob Keller wrote:
>>
>> > I think I narrowed this down to "git diff-index --name-only --relative
>> > HEAD" producing a list of files *not* relative to the current
>> > directory.
>>
>> Hmm, my guess would have been something funny in the setup code
>> forgetting our original prefix.
>>
>> But nope, it looks like the culprit is f7923a5ece (diff: use
>> skip_to_optional_val(), 2017-12-04), which switched over parsing of
>> "--relative".
>
> Oh, actually, I guess I was half-right. It feeds &options->prefix as the
> "default", meaning that we overwrite it with the empty string. I don't
> think "--relative" works for the semantics of skip_to_optional_value,
> since it needs:
>
>   --relative=foo: set prefix to "foo"
>
>   --relative: leave prefix untouched
>
> -Peff

Yep, and apparently our test suite completely lacked any tests of --relative on its own.

I've sent a patch to add some tests.

I don't know the exact best way to fix this, I guess we could just revert it the changes to relative... but maybe we could add or modify the semantics of skip_to_optional_val()?? What if it was changed so that it left the value alone if no value was provided? This would require callers to pre-set the value they want as default, but that would solve relative's problem.

I'll look into that.

Thanks, Jake