Re: [PATCH 2/2] convert trivial uses of strncmp() to skip_prefix()

[Jeff King <peff@peff.net>]

On Sat, Jan 07, 2023 at 10:29:28PM +0100, René Scharfe wrote:

> > So it will prefix-match any of the options, even if there are
> > ambiguities. E.g.:
> >
> >   git -c core.whitespace=-t show
> >
> > will turn off "trailing-space", even though it would also match
> > "tab-in-indent". It would be easy enough to fix it to require the whole
> > name, but I wasn't sure if this prefix-matching was supposed to be a
> > feature (it doesn't seem to be documented anywhere, though).
> 
> Abbreviations are being used:
> 
>    $ git grep whitespace= .gitattributes
>    .gitattributes:* whitespace=!indent,trail,space
>    .gitattributes:*.[ch] whitespace=indent,trail,space diff=cpp
>    .gitattributes:*.sh whitespace=indent,trail,space eol=lf
> 
> (Full names: trailing-space, space-before-tab, indent-with-non-tab.)

Ah, right, I should have checked to see if _we_ are using them before
guessing whether anyone else might be.

> a9cc857ada (War on whitespace: first, a bit of retreat., 2007-11-02)
> added this function.  Its commit message says:
> 
>    "You can specify the desired types of errors to be detected by
>     listing their names (unique abbreviations are accepted)
>     separated by comma."

Thanks, I dug around for something like that but somehow missed it.

So yeah, we definitely want to keep this abbreviation feature working.
The only question is whether we ought to detect ambiguous ones. I think
something like this would work, though I wonder if is even worth
bothering about. I did not even see this in the wild, but it was just a
curiosity while I was adjusting something else in the function:

diff --git a/ws.c b/ws.c
index 46a77bcad6..f4efd66209 100644
--- a/ws.c
+++ b/ws.c
@@ -29,6 +29,7 @@ unsigned parse_whitespace_rule(const char *string)
 		int i;
 		size_t len;
 		const char *ep;
+		struct whitespace_rule *matched = NULL;
 		int negated = 0;
 
 		string = string + strspn(string, ", \t\n\r");
@@ -43,15 +44,27 @@ unsigned parse_whitespace_rule(const char *string)
 		if (!len)
 			break;
 		for (i = 0; i < ARRAY_SIZE(whitespace_rule_names); i++) {
-			if (strncmp(whitespace_rule_names[i].rule_name,
-				    string, len))
+			struct whitespace_rule *cur = &whitespace_rule_names[i];
+			if (strncmp(cur->rule_name, string, len))
 				continue;
+			if (matched) {
+				warning("ignoring ambiguous whitespace rule '%.*s'"
+					" (matches '%s' and '%s')",
+					(int)len, string,
+					matched->rule_name, cur->rule_name);
+				matched = NULL;
+				break;
+			}
+			matched = cur;
+		}
+
+		if (matched) {
 			if (negated)
-				rule &= ~whitespace_rule_names[i].rule_bits;
+				rule &= ~matched->rule_bits;
 			else
-				rule |= whitespace_rule_names[i].rule_bits;
-			break;
+				rule |= matched->rule_bits;
 		}
+
 		if (strncmp(string, "tabwidth=", 9) == 0) {
 			unsigned tabwidth = atoi(string + 9);
 			if (0 < tabwidth && tabwidth < 0100) {

-Peff