On Thu, Nov 07, 2024 at 11:52:03AM +0900, Junio C Hamano wrote: > Jeff King <peff@xxxxxxxx> writes: > > [...] > > 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)... 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.
