Re: [PATCH] t6300: values containing ')' are broken in ref formats

From: Junio C Hamano <gitster@pobox.com>
Date: Nov 6, 2024, 03:05 UTC
Message-ID: <xmqq8qtxqcye.fsf@gitster.g>
In-Reply-To: <20241106022552.GA816908@coredump.intra.peff.net>

Jeff King <peff@peff.net> writes:

> I am tempted to say the solution is to expand that "equals" value, and
> possibly add some less-arcane version of the character (maybe "%)"?).
> But it be a break in backwards compatibility if somebody is trying to
> match literal %-chars in their "if" block.

If they were trying to write a literal %, wouldn't they be writing %% already, not because % followed by a byte without any special meaning happens to be passed intact by the implementation, but because that is _the_ right thing to do, when % is used as an introducer for escape sequences? So I do agree it would be a change that breaks backward compatibility but I do not think we want to stay bug to bug compatible with the current behaviour here. I am not sure with the wisdom of %) though. Wouldn't "%(foo %)" look as if %( opens and %) closes a group in our language?

So I am very much in favor of this "if condition should be expanded before comparison" solution.