On Tue, Nov 05, 2024 at 07:05:13PM -0800, Junio C Hamano wrote: > Jeff King <peff@xxxxxxxx> 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 think "because that is the right thing to do" is what is in question. It is not like we happen to allow "%", but you should be writing "%%" in an if:equals value already. They mean two different things, and anybody who is doing: %(if:equals=%%foo) to match the literal "%%foo" will be broken if we change that. They are not doing anything wrong; that is the only way to make it work now. I wouldn't go so far as to call the current behavior a bug. It's just...not very flexible. I also think it is unlikely that anybody would care in practice (though I find matching refs with ")" in them already a bit far-fetched). If we wanted to be extra careful, we could introduce a variant of "equals" that indicates that it will be expanded before comparison. Or even an extra tag, like: %(if:expand:equals=%%foo) > I am not sure with the wisdom of %) though. Wouldn't "%(foo %)" look > as if %( opens and %) closes a group in our language? Yeah, I agree it is ugly and possibly confusing. Normally I'd suggest "\" for escaping, but it isn't otherwise syntactically important within these formats (I don't think, anyway). The magic character is "%" so that is what we have to work with. -Peff
