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

[Junio C Hamano <gitster@pobox.com>]

Jeff King <peff@peff.net> writes:

> 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.

Ah, you're absolutely right.  Unescaping would start breaking them.

> 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).

100% agreed.  For that matter, I find "if:equals=%%foo" equally
implausible.

> 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)

Surely, but if nobody screams, I am tempted to suggest fixing the
equals/notequals---we do not have to be bug-to-bug compatible with a
buggy old implementation. After all, we do expand the string being
inspected that appears between %(if) and %(then).  I do not think of
a good excuse for us to limit the string that it gets compared with
to literals.

The implementation may be a bit involved, but shouldn't be too bad.

When .str is an empty string in if_atom_handler(), we can follow
what the current code does.  If .str is not empty, allocate a new
stack element in order to parse the .str to its end by pointing
.at_end of the new stack element to a new handler (call it
if_cond_handler()), and pass the if_then_else structure it allocated
as .at_end_data to it.

And in the if_cond_handler(), grab the cur->output and overwrite the
.str member with it (while being careful to avoid leaks).  At the
end of the if_cond_handler(), pass control to if_then_else_handler()
by arranging the if_then_else_handler is called, imitating the way
how if_atom_handler() passes control to if_then_else_handler() in
the current code.

Then things like

  %(if:equals=%(upstream:lstrip=3))%(refname:short)%(then)...

would work as expected ;-)