Re: [PATCH 01/11] t9902-completion: add tests demonstrating issues with quoted pathnames

From: SZEDER Gábor <szeder.dev@gmail.com>
Date: Apr 26, 2018, 00:25 UTC
Message-ID: <CAM0VKjkQfTm+qnurvZ_545VXJH2PwuPfkhXaa1sLj5ePSPjBwA@mail.gmail.com>
In-Reply-To: <xmqqlgdljok5.fsf@gitster-ct.c.googlers.com>

On Wed, Apr 18, 2018 at 3:22 AM, Junio C Hamano <gitster@pobox.com> wrote:

Show 24 quoted lines

> SZEDER Gábor <szeder.dev@gmail.com> writes:
>>> Do we want to test a more common case of a filename that is two
>>> words with SP in between, i.e.
>>>
>>>         $ >'hello world' && git add hel<TAB>
>>>
>>> or is it known to work just fine without quoting/escaping (because
>>> the funny we care about is output from ls-files and SP is not special
>>> in its one-item-at-a-time-on-a-line output) and not worth checking?
>>
>> This particular case already works, even without this patch series.
>
> I was more wondering about preventing regressions---"it worked
> without this patch series, but now it is broken" is what I was
> worried about.
>
>> The problems start when you want to complete the filename after a space,
>> e.g. 'hello\ w<TAB', as discussed in detail in patch 5.  Actually, this
>> was the first thing I tried to write a test for, but it didn't work out:
>> inside the 'test_completion' helper function the space acts as
>> separator, and the completion script then sees 'hello\' and 'w' as two
>> separate words.
>
> Hmph.  That is somewhat unfortunate.

Actually, I used 'test_completion' in these new tests, because there is that big test checking file completion for various commands, and it already uses 'test_completion', so I just followed suit. Now, that test checks that the right type(s) of files are listed for various git commands, e.g. modified and untracked for 'git add', IOW that the caller of __git_complete_index_file() specifies the appropriate 'git ls-files' options. For those kind of checks 'test_completion' is great.

These new tests, however, are primarily interested in the inner workings of __git_complete_index_file() in the presence of escapes and/or quotes in the path to be completed and/or in the output of 'git ls-files'. For these kind of tests we could simply invoke __git_complete_index_file() directly, like we call __git_refs() directly to test refs completion. Then we could set the current path to be completed to whatever we want, including spaces, because it won't be subject to field splitting like the command line given to 'test_completion'.

So, I think for v2 I will rewrite these tests to call __git_complete_index_file() directly instead of using 'test_completion', and will include a test with spaces in path names.