From: Ramsay Jones <ramsay@ramsayjones.plus.com>
Date: Thu, 02 Jun 2016 19:44:05 GMT
Subject: Re: [RFC/PATCH] pathspec: allow escaped query values
Message-ID: <57508C85.1090503@ramsayjones.plus.com>
In-Reply-To: <CAGZ79ka3ku7x7BhBZJ_uHjS0pNLT7NFACUVqNKi=DvtOkSN_bg@mail.gmail.com>



On 02/06/16 20:04, Stefan Beller wrote:
> On Thu, Jun 2, 2016 at 11:42 AM, Ramsay Jones
> <ramsay@ramsayjones.plus.com> wrote:
>>
>>
>> On 02/06/16 17:10, Junio C Hamano wrote:
>>> Ramsay Jones <ramsay@ramsayjones.plus.com> writes:
>>>
>>>> So, at risk of annoying you, let me continue in my ignorance a little
>>>> longer and ask: even if you have to protect all of this 'magic' from
>>>> the shell with '/" quoting, could you not use (nested) quotes to
>>>> protect the <value> part of an <attr>? For example:
>>>>
>>>>     git ls-files ':(attr:whitespace="indent,trail,space",icase)'
>>>
>>> That would be workable, I would think.  Before attr:VAR=VAL
>>> extention, supported pathspec <magic> were only single lowercase-ascii
>>> alphabet tokens, so nobody would have used " as a part of magic.  So
>>> quting with double-quote pair would work.
>>
>> I was thinking about both ' and ", so that you could do:
>>
>>    $ ./args ':(attr:whitespace="indent,trail,space",icase)'
>>     1::(attr:whitespace="indent,trail,space",icase)
>>
>>    $ ./args ":(attr:whitespace='indent,trail,space',icase)"
>>     1::(attr:whitespace='indent,trail,space',icase)
>>
>>    $ p=':(attr:whitespace="indent,trail,space",icase)'
>>    $ ./args "$p"
>>     1::(attr:whitespace="indent,trail,space",icase)
>>
>>    $ p=":(attr:whitespace=\"indent,trail,space\",icase)"
>>    $ ./args "$p"
>>     1::(attr:whitespace="indent,trail,space",icase)
>>
>> but limiting it to " would probably be OK too.
>>
>>> You'd need to come up with a way to quote a double quote that
>>> happens to be a part of VAL somehow, though.
>>
>> Yes I was assuming \ quoting as well - I just want to reduce the
>> need for such quoting (especially on windows).
>>
>>>                                              I think attribute
>>> value is limited to a string with non-whitespace letters; even
>>> though the built-in attributes that have defined meaning to the Git
>>> itself may not use values with letters beyond [-a-zA-Z0-9,], end
>>> users and projects can add arbitrary values within the allowed
>>> syntax, so it is not unconceivable that some project may have a
>>> custom attribute that lists forbidden characters in a path with
>>>
>>>       === .gitattributes ===
>>>         *.txt forbidden=`"
> 
> We restrict the 'forbidden' to follow [-a-zA-Z0-9,], so we could enforce
> it for the values, too.
> 
> 
>>
>>    $ ./args ":(attr:*.txt forbidden=\'\\\",icase)"
>>     1::(attr:*.txt forbidden=\'\",icase)
> 
> You should lose the *.txt in there, but put it at the back

Ah, yes, just shows my ignorance of the attribute system!

> 
>>  $ ./args ":(attr:forbidden=\'\\\",icase)*.txt"
> 
>>
>>    $ ./args ':(attr:*.txt forbidden=\'\''\",icase)'
>>     1::(attr:*.txt forbidden=\'\",icase)
> 
> I see, so quoting by " or ' is preferred. What if the user
> wants to do a

I think Junio wants to go with just " quoting (see other thread).

>     forbidden=',"
> 
> so we have to escape those in there, such as
> 
>     ./args ':(attr:"forbidden=\',\"")'

No, that won't work (" is not terminated), try this:

   $ ./args ':(attr:"forbidden='\'',\"")'
    1::(attr:"forbidden=',\"")
   $ 

   $ ./args ":(attr:\"forbidden=',\\\"\")"
    1::(attr:"forbidden=',\"")
   $ 

[half of the problem is just getting past the shell]

ATB,
Ramsay Jones