On Wed, Feb 16, 2022 at 9:20 AM Victoria Dye [off-list ref] wrote:

Elijah Newren wrote:

quoted

On Wed, Feb 16, 2022 at 1:57 AM Ævar Arnfjörð Bjarmason
[off-list ref] wrote:

quoted

On Wed, Feb 16 2022, Elijah Newren via GitGitGadget wrote:

quoted

From: Elijah Newren <redacted>

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.

quoted

quoted

+     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"));

Ah, perfect -- thanks for the pointer!  (Though I think the logic
needs to be reversed; error if we find any of those characters rather
than error if we don't.)