On Wed, Feb 16, 2022 at 9:20 AM Victoria Dye <vdye@xxxxxxxxxx> wrote:
>
> 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"));
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.)
