On 11/09/2011 08:18 AM, Henrik Nordström wrote:
>> Regarding making FORMAT optional to external_acl_type, this is a trivial
>> change, just didn't think there was any valid use of it when the
>> directive was implemented. Please try the attached trunk patch which
>> also adds %%{statictext} just in case someone needs to fake format
>> arguments to some helper.
> %% The percent sign. Useful for helpers which need
> - an unchanging input format.
> + an unchanging but not blank input format.
The "Useful for ..." phrase makes no sense in this context for
uninitiated (before and especially after the above quoted change).
Unchanging input format? Blank input format? It may be meaningful in a
commit message, when describing why the %-escape sequence was added but
here it only confuses the reader. I would remove that phrase completely.
> + %%{..} Staic text
It feels wrong to use %% both for specifying raw percent sign and for
the string quoting mechanism. At the very least, it makes %%{foo}
syntactically ambiguous. Please pick a different syntax. %{} or %""
would work better, for example.
BTW, you may be able to support %"quoted strings" syntax where the
phrase inside quotes may contain spaces because we now have a general
purpose quoted string parser. However, I am not sure that quoted string
parser is compatible with the '%' prefix.
> + %%{..} Staic text
"Static" is misspelled. It is also not very clear what "static" means
here, IMHO. Consider:
%{text} Constant text without spaces. The %{} decoration
is removed before passing the text to the helper.
And if you add support for spaces:
%"text" Constant text, possibly containing spaces. The ...
HTH,
Alex.
Received on Wed Nov 09 2011 - 15:53:06 MST
This archive was generated by hypermail 2.2.0 : Fri Nov 11 2011 - 12:00:10 MST