>> On 05/26/2013 10:26 PM, Amos Jeffries wrote: >>> Whether macros are supported or not is irrelevant to the string being >>> de-coded into a single token or split on whitespace.
> On 05/27/2013 07:23 PM, Alex Rousskov wrote: >> I agree that we can and perhaps should keep runtime %macros handling >> outside squid.conf tokenization process. If an option supports macros, >> it can validate their syntax _after_ receiving the token from the >> parser. This is no different than validating tokens containing integer >> values or ACL names. >> >> One could argue that integrated/centralized value validation is better, >> but since the current code does not use integrated validation for other >> value types, perhaps this patch should not introduce it. >> >> External ACL and logformat can do something like this: >> >> token = ConfigParser::NextToken(); >> handleRuntimeMacros(token); >> >> This will keep runtime %macros outside ConfigParser scope. The >> MacroUser::handleRuntimeMacros() method can organized all the necessary >> checks in one place, using an already proposed virtual method for >> validating individual macros. ConfigParser would not know about this though. On 06/27/2013 12:03 PM, Tsantilas Christos wrote: > I think that this one it is equivalent to completely remove the %macros > checking from tokens parsing and remove the MacroUser class from patch. > Is it OK? It is not exactly equivalent because MacroUser::handleRuntimeMacros() provides a centralized macro validation place for ACLs and logformat and it uses the new MacroUser API for that. All the macro-validation code remains in one place (just like in your original patch), but that place is now outside the tokenization code (addressing Amos' concern). HTH, Alex.
