On Tue, Feb 14, 2023 at 02:34:01PM -0800, Junio C Hamano wrote:
> Jeff King <peff@xxxxxxxx> writes:
>
> > but I have a feeling that it gets more readable if we flip the break
> > conditional and the loop condition.
>
> Yeah, the somewhoat unusual loop structure was what motivated me to
> look at its corner case. Flipping the logic around may make it more
> straight forward.
It does indeed. I pulled the logic from skip_prefix(), thinking that by
relying on it I would avoid making a stupid mistake. Oh well. :)
Doing it like this is much more readable:
diff --git a/refs.c b/refs.c
index d8ce7e9ee1..725adafcd8 100644
--- a/refs.c
+++ b/refs.c
@@ -1323,12 +1323,12 @@ static const char *match_parse_rule(const char *refname, const char *rule,
* in the rule. This is basically skip_prefix(), but
* ending at percent in the prefix, rather than end-of-string.
*/
- do {
+ while (*rule != '%') {
if (!*rule)
BUG("rev-parse rule did not have percent");
- if (*rule == '%')
- break;
- } while (*refname++ == *rule++);
+ if (*refname++ != *rule++)
+ return 0;
+ }
/*
* Check that we matched all the way to the "%" placeholder,
I'll hold on to that (plus an adjustment to the comment below to match,
and perhaps a test for this negative-match case) for a day or so to give
anybody else a chance to comment, and then send out a v2 tomorrow.
-Peff
