On Fri, Nov 08, 2024 at 09:41:03AM +0530, Kousik Sanagavarapu wrote:
> > Then things like
> >
> > %(if:equals=%(upstream:lstrip=3))%(refname:short)%(then)...
>
> So if I understand correctly, we grab the .str and operate on it so that
> we expand the atom within it and then do the comparision.
>
> This seems nice, but there is a problem. Since we always look for the
> first occurring ')' in our format string to indicate the end of the atom,
> we end up with
>
> .str = %(upstream:lstrip=3
>
> (the call chain is
>
> verify_ref_format() -> parse_ref_filter_atom() -> if_atom_parser()
> )
>
> Since we have now left out a ')', this ')' gets appended to our output
> buf, which would also show up in cur->output when we do the comparision
> in then_atom_handler(). For example, in this case our cur->output would
> be ")master" instead of "master" after we get the value of
> %(refname:short), meaning our comparision always fails.
Yes, though I think the parser _could_ be improved here. This is
different the earlier case of matching "ref-with-)". In that case it is
syntactically ambiguous. For example given:
%(if:equals=ref-with-))foo
you cannot tell the difference between:
- matching "ref-with-)", followed by "foo"
- matching "ref-with-", followed by ")foo"
But if the parenthesis in question is closing a %() item, like:
%(if:equals=%(refname))foo
then we know that the inner ")" is closing %(refname), since parsing it
as "%(refname" followed by ")foo" would leave an unbalanced pair. But
finding that would require a real recursive descent parser, rather than
a blind strchr() for the closing ")"[1].
In the meantime yeah, you'd have to spell it as:
%(if:equals=%(refname%29)
which is...deeply unsatisfying.
I have long dreamed of throwing out all of this format code in favor of
a recursive parser which generates an actual tree of nodes, and
implements all of the ref-filter/pretty.c/cat-file format placeholders.
But I think it's a non-trivial task.
-Peff
[1] Incidentally, the "%)" I proposed earlier would also fall afoul of
this problem. The search for the closing ")" is done blindly without
regard to possible quoting.
