Re: [PATCH v12 03/13] ref-filter: introduce the ref_formatting_state stack machinery

[Junio C Hamano <gitster@pobox.com>]

Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> writes:

> Karthik Nayak <karthik.188@gmail.com> writes:
>
>> Speaking of quote_value, The quote doesn't work well with color's
>> for e.g.
>> git for-each-ref --shell --format="%(color:green)%(refname)"
>> '''refs/heads/allow-unknown-type'''
>> Seems like an simple fix, probably after GSoC I'll do this :)
>
> Anyway, the %(color) is really meant to be displayed on-screen, and the
> quoting is really meant to feed the value to another program, so I can
> hardly imagine a use-case where you would want both.
>
> But the current behavior seems fine to me: the color escape sequence is
> quoted, which is good. For example, you can
>
> x=$(git for-each-ref --shell --format="nocolor%(color:green)%(refname)" | head -n 1)
> sh -c "echo $x"
>
> it will actually display "nocolor" without color, then a green refname.
> I'm not sure the quoting is really necessary, but it doesn't harm and it
> makes sense since the escape sequence contains a '[' which is a shell
> metacharacter.

The point of --shell/--tcl/... is so that you can have --format
safely write executable scripts in the specified language.  Your
format string might look like this:

        --format="short=%(refname:short) long=%(refname)"

and one entry in the output in "--shell" mode would expand to

        short='master' long='refs/heads/master'

that can be eval'ed as a script safely without having to worry about
expanded atom values having characters that have special meanings in
the target language.  Your "nocolor" example works the same way:

        --format="var=%(color:green)%(refname)" --shell

would scan 'var=', emit it as literal, see %(color:green) atom, show
it quoted, see %(refname), show it quoted, notice that color is not
terminated and pretend as if it saw %(color:reset) and show it
quoted, which would result in something like:

        var='ESC[32m''master''ESC[m'

Note that the example _knows_ that the quoting rule of the target
language, namely, two 'quoted' 'strings' next to each other are
simply concatenated.  When using a hypothetical target language
whose quoting rule is different, e.g. "type two single-quotes inside
a pair of single-quote to represent a literal single-quote", then
you would write something like this to produce a script in that
language:

        --format="
            var1=%(color:green);
            var2=%(refname);
            var=var1+var2;
        "

as your format string (and it will not be used with --shell).  And
the atom-quoting code that knows the language specific rules would
quote %(atom) properly.  Perhaps the language uses `' for its string
quoting, in which case one entry of the output might look like

	var1=`ESC[32m';
        var2=`refs/heads/master';
        var=var1+var2;

which would be in the valid syntax of that hypothetical language.

Maybe you have an atom %(headstar) that expands to an asterisk for
the currently checked out branch, in order to mimick 'git branch -l'.

Using that, you might use --shell --format to invent a shorter output
format that does not show the asterisk but indicates the current
branch only with color, like so:

        --format='
            if test -z %(headstar)
            then
                echo %(refname:short)
            else
                echo %(color:green)%(refname:short)%(color:reset)
            fi
        '

and you would want %(headstar)'s expansion to be '*' or ''.

If we introduce %(if:empty)%(then)%(else)%(end), the above may
become something like this, removing the need for --shell
altogether:

        %(if:empty)%(headstar)%(then
        )%(refname:short)%(else
        )%(color:green)%(refname:short)%(color:reset)%(end)

With the current implementation, it is likely that this needs to be
a single long line; we may want to extend parsing of atoms to allow
a LF+whitespace before the close parenthesis to make the string more
readable like the above example, but that is an unrelated tangent.

But you should still be able to use "--shell" this way, assigning
the whole thing to a variable:
    
        --format='
                line=%(if:empty)%(headstar)%(then
                )%(refname:short)%(else
                )%(color:green)%(refname:short)%(color:reset)%(end)
                echo "$line"
        '

So I think 'quote' should apply only to the top-level atoms in the
nested %(magic)...%(end) world.  Expand %(if:empty)...%(end) and
then apply the quoting rule specific to the target language to make
the result safe to use as the RHS of the target language.  None of
the atoms that appear internally (e.g. %(headstar) that is being
tested for emptyness) must NOT be quoted.

If you have %(align:40)%(atom) and string%(end), the same logic
applies.  %(atom) is not a top level item (it is inside %(align))
so you would expand "%(atom) and string" without quoting, measure
its display width, align to 40-cols and then if --shell or any
quoting is in effect, applyl that, so that the user can do:

	--format='
            right=%(align:40)%(refname)%(end)
            left=%(align:20,right)%(refname:short)%(end)
            echo "$left $right"
	'

one entry of the output from which would expand to

	right='refs/heads/master                       '
        left='              master'
        echo "$left $right"

because the rule is to quote the whole %(align)...%(end) construct
only once.