From: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Date: Wed, 16 Feb 2022 09:53:15 GMT
Subject: Re: [PATCH v3 5/5] sparse-checkout: reject arguments in cone-mode that look like patterns
Message-ID: <220216.8635kjuob0.gmgdl@evledraar.gmail.com>
In-Reply-To: <2008542d0c718bbe43388297307a791f94bc73e1.1644985283.git.gitgitgadget@gmail.com>


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

> From: Elijah Newren <newren@gmail.com>
>
> 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?...

> +	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.