Re: [PATCΗ] Quoted values in squid.conf

Hi all,
 I am trying to unfreeze this patch.

One problem is that we did not finish the related discussions.

Currently I implemented the way Alex and Kinki suggested for including
paraameters in directives:
 http_port parameters("/path/to/file.txt");

This is works well.

But I do not know what to do with %macros in tokens.
Please see bellow my comments .

Also I am attaching my latest patch

On 05/27/2013 07:23 PM, Alex Rousskov wrote:
> On 05/26/2013 10:26 PM, Amos Jeffries wrote:
>> On 27/05/2013 2:07 a.m., Tsantilas Christos wrote:
>>> On 05/26/2013 06:30 AM, Amos Jeffries wrote:
>>>> 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.
>
> 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.
>
> On 05/27/2013 04:40 AM, Tsantilas Christos wrote:
>> What are you suggesting for MacroUser class? Just remove it and don't do
>> any check for '%' fmt codes? Replace it with a simple on/off which
>> enables/disable '%' chars in tokens? eg:
>>
>> ConfigParser::EnableMacro();
>> token = ConfigParser::NextToken();
>> ConfigParser::DisableMacro();
>
>
> 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.

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?

>
> Would that make everybody happy?
>
>
> HTH,
>
> Alex.