Matthieu Moy <Matthieu.Moy@xxxxxxxxxxxxxxx> writes:
> My take on it:
>
> Implement %(if), %(then) and %(else) atoms. Used as
> %(if)...%(then)...%(end) or %(if)...%(then)...%(else)...%(end). If the
> format string between %(if) and %(then) expands to an empty string, or
> to only whitespaces, then the string following %(then) is printed.
> Otherwise, the string following %(else), if any, is printed.
>
>> +When a scripting language specific quoting is in effect,
>
> This may not be immediately clear to the reader. I'd add explicitly:
>
> When a scripting language specific quoting is in effect (i.e. one of
> `--shell`, `--perl`, `--python`, `--tcl` is used), ...
I found all the suggestions very good, except that the distinction
between "expands to" and "is printed" bothers me a bit, as they want
to mean exactly the same thing (imagine this whole thing were inside
another %(if)...%(then)).
>> EXAMPLES
>> --------
>
> This is just the context of the patch, but I read it as a hint that we
> could add some examples with complex --format usage to illustrate the
> theory above.
Yes ;-)
>> +static int is_empty(const char * s){
>
> char * s -> char *s
Also "{" must sit alone on the next line.
>> +static void then_atom_handler(struct atom_value *atomv, struct
>> ref_formatting_state *state)
>> +{
>> + struct ref_formatting_stack *cur = state->stack;
>> + struct if_then_else *if_then_else = (struct if_then_else *)cur->at_end_data;
>> +
>> + if (!if_then_else)
>> + die(_("format: %%(then) atom used without an %%(if) atom"));
>
> You've just casted at_end_data to if_then_else. if_then_else being not
> NULL does not mean that it is properly typed. It can be the at_end_data
> of another opening atom. What happens if you use
> %(align)foo%(then)bar%(end)?
>
> One way to be safer would be to check that cur->at_end does point to
> if_then_else_handler.
Good eyes. Thanks.
> So, while parsing the %(else)...%(end), the stack contains both the
> %(then)...%(else) part, and the %(else)...%(end).
>
> I'm wondering if we can simplify this. We already know if the condition
> is satisfied, and if it's not, we can just drop the %(then) part right
> now, and write to the top of the stack normally (the %(end) atom will
> only have to pop the string normally). But if the condition is not
> satisfied, we need to preserve the %(then) part and need to do something
> about the %(else).
Good suggestion.
Thanks.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html