Elijah Newren wrote:
> On Wed, Feb 16, 2022 at 1:57 AM Ævar Arnfjörð Bjarmason
> <avarab@xxxxxxxxx> wrote:
>>
>> On Wed, Feb 16 2022, Elijah Newren via GitGitGadget wrote:
>>
>>> From: Elijah Newren <newren@xxxxxxxxx>
>>>
>>> In sparse-checkout add/set under cone mode, the arguments passed are
>>> supposed to be directories rather than gitignore-style patterns.
>>> However, given the amount of effort spent in the manual discussing
>>> patterns, it is easy for users to assume they need to pass patterns such
>>> as
>>> /foo/*
>>> or
>>> !/bar/*/
>>> or perhaps they really do ignore the directory rule and specify a
>>> random gitignore-style pattern like
>>> *.c
>>>
>>> To help catch such mistakes, throw an error if any of the positional
>>> arguments:
>>> * starts with any of '/!'
>>> * contains any of '*\?[]'
>>
>> But not "\" itself, we're just escaping the "?" here?...
>
> Nah, I just missed that one. I should add it below.
>
>>> + if (core_sparse_checkout_cone) {
>>> + for (i = 0; i < argc; i++) {
>>> + if (argv[i][0] == '/')
>>> + die(_("specify directories rather than patterns (no leading slash)"));
>>> + if (argv[i][0] == '!')
>>> + die(_("specify directories rather than patterns. If your directory starts with a '!', pass --skip-checks"));
>>> + if (strchr(argv[i], '*') ||
>>> + strchr(argv[i], '?') ||
>>> + strchr(argv[i], '[') ||
>>> + strchr(argv[i], ']'))
>>> + die(_("specify directories rather than patterns. If your directory really has any of '*?[]' in it, pass --skip-checks"));
>>
>> Isn't this nested || a reinvention of a simpler strtok() or strtok_r()
>> call? I.e. (untested):
>>
>> const char *p;
>> const char *wildcards = "*?[]";
>> if (strtok_r(argv[i], wildcards, &p))
>> die(_("specify... has ony of '%s' in it...", wildcards));
>>
>> That would also allow parameterizing the set of characters for
>> translators.
>
> I considered strtok, but strtok & strtok_r are documented as modifying
> their argument. Perhaps they don't modify the argument if they don't
> find any of the listed tokens, but I didn't want to rely on that since
> I found no guarantees in the documentation.
Maybe `strpbrk` would work? Unless I'm misunderstanding, it should
consolidate the condition to one line without potentially modifying the
arguments. E.g.:
if (!strpbrk(argv[i], "*?[]"))
die(_("specify directories rather than patterns. If your directory really has any of '*?[]' in it, pass --skip-checks"));
