Subject: Re: [PATCH v5 3/3] completion: handle unusual characters for sparse-checkout

From: Ævar Arnfjörð Bjarmason <[email-masked]>
Date: Fri, 04 Feb 2022 05:25:38 +0100

On Thu, Feb 03 2022, Elijah Newren wrote:

> On Thu, Feb 3, 2022 at 12:44 PM Lessley Dennington via GitGitGadget
> <[email-masked]> wrote:
>>
>> From: Lessley Dennington <[email-masked]>
>> +# use FUNNYNAMES to avoid running on Windows, which doesn't permit backslashes in paths
>> +test_expect_success FUNNYNAMES 'cone mode sparse-checkout completes directory names with special characters' '
>> +       # reset sparse-checkout
>> +       git -C sparse-checkout sparse-checkout disable &&
>> +       (
>> +               cd sparse-checkout &&
>> +               mkdir "directory with spaces" &&
>> +               mkdir "$(printf "directory\twith\ttabs")" &&
>> +               mkdir "directory\with\backslashes" &&
>> +               mkdir "directory-with-áccent" &&
>> +               >"directory with spaces/randomfile" &&
>> +               >"$(printf "directory\twith\ttabs")/randomfile" &&
>> +               >"directory\with\backslashes/randomfile" &&
>> +               >"directory-with-áccent/randomfile" &&
>> +               git add . &&
>> +               git commit -m "Add directories containing unusual characters" &&
>> +               git sparse-checkout set --cone "directory with spaces" \
>> +                       "$(printf "directory\twith\ttabs")" "directory\with\backslashes" \
>> +                       "directory-with-áccent" &&
>> +               test_completion "git sparse-checkout add dir" <<-\EOF
>> +               directory with spaces/
>> +               directory       with    tabs/
>> +               directory\with\backslashes/
>> +               directory-with-áccent/
>> +               EOF
>> +       )
>> +'
>
> I'm glad you tested with lots of special characters -- spaces, tabs,
> backslashes, and accents.  Newlines might also be nice, but probably
> makes the test hard.  Anyway, looks good to me, other than the
> indentation change.

This looks like its over-skipping tests, the comment says Windows
doesn't like \ in paths, but there's a lo more being skipped here than
that.

Shouldn't e.g. "directory-with-áccent/" be pulled out into another test,
leaving the directory\with\backslashes/ etc. as non-Windows?

Another useful method is to just try to create those paths
unconditionally, but with "mkdir" etc., and if that works run the test
with git. After all if the OS can create a given path, but we're not
handling it that's our bug.

But if the FS just doesn't support the path we'd catch that in the
"mkdir" (or whatever), which we can assume behaves sanely on that
platform (or we're horribly broken anyway...).