On 27/05/2013 2:07 a.m., Tsantilas Christos wrote:
On 05/26/2013 06:30 AM, Amos Jeffries wrote:
On 24/05/2013 7:38 a.m., Tsantilas Christos wrote:

<snip strings>


Secondly, multiple features in one patch:

  The MacroUser system you are adding here is combining some, but not
enough, of the next step in the libformat project. Thank you for doing
that, but I do not think it appropriate to merge it into this whitespace
and quote handling patch. It was planned to be done as a followup once
the parser was updated in a way such as what this patch is doing.
The MacroUser just only check if %macros supported or not, and which
macros supported. Also the user of this class may select to not do any
check at all, just use it to enable macros in the next parsed token.
I do not think that it is related to libformat ...

Whether macros are supported or not is irrelevant to the string being de-coded into a single token or split on whitespace. Which is what the rest of this patch is doing. The one potential crossover is that %"<h is a valid macro/code/token which contains a quote. But your update needs to ensure that quoting always is at the beginnning or end of a word when it toggles anyway.

The only code you have pulling in macros is code which is or will be shortly using libformat to parse those macros. So I'm seeing all this macro code (as currently implemented) as part of the libformat project, possibly the very next step. But still a different scope to the string quoting patch. That is all.


  Also by naming the class MacroUser you are adding a fourth term (macro)
to describe these %things, we already have %-tokens, %-tags, %-format,
and %-codes in use today. I think we should start to redux those terms
usage rather than expanding with another variation on the theme.
The naming is not something important, we can always rename this class,
before commit the patch or later when we find a better name ...

==> Please separate the MacroUser functionality out into a followup patch.


Amos



Reply via email to