On Wed, Nov 06, 2024 at 01:51:02PM -0500, Jeff King wrote: > 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. True. > 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). Yeah. I really don't think anyone in practice will hit upon this case. As I mentioned already before, I was just trying to pick out a corner case for another implementation in ref-filter and stumbled upon this. > 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) This seems like a nice idea, if we are thinking about not breaking backwards compatibility but then there is also this discussion about the formats being too verbose but I dunno.
