Re: [PATCH 2/6] quote_path: give flags parameter to quote_path()

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

Jeff King <peff@peff.net> writes:

> On Tue, Sep 08, 2020 at 01:52:20PM -0700, Junio C Hamano wrote:
>
>> -char *quote_path(const char *in, const char *prefix, struct strbuf *out)
>> +char *quote_path(const char *in, const char *prefix, struct strbuf *out, unsigned flags)
>>  {
>>  	struct strbuf sb = STRBUF_INIT;
>>  	const char *rel = relative_path(in, prefix, &sb);
>>  	strbuf_reset(out);
>> -	quote_c_style_counted(rel, strlen(rel), out, NULL, 0);
>> +	quote_c_style_counted(rel, strlen(rel), out, NULL, flags);
>>  	strbuf_release(&sb);
>
> Here we take "flags", but then we pass it along in place of
> quote_c_style_counted()'s "no_dq" parameter. That seems unlikely to be
> the right thing (and indeed, it's reverted in the next commit).

You are seeing a rebase artifact.  It needs to be corrected.  Thanks
for noticing.

As you seem to have guessed correctly, the initial plan was to have
the new logic in quote-c-style-counted and make anything at higher
level of the callchain just relay the "SP must be quoted" bit, and
the final two patches that are no longer necessary in the current
series were placed much earlier in an earlier draft of the series
as preparatory steps for that endgame.

But it turned out that the main loop of quote-c-style-counted needs
a surgery that is a bit larger than I would have liked, so the final
series consumes the "SP must be quoted" bit in quote_path() without
telling quote-c-style-counted about it.

The problem with the main loop of quote-style-counted I saw was that
the next_quote_pos() is designed to return the position of the byte
that must be prefixed with a backslash, and the machinery to help it
(namely, cq_must_quote() and cq_lookup[]) are written with that in
mind quite firmly.  The handling of SP we would be optionally adding
to the mix is somewhat different---it forces the end result to be
enclosed within a dq-pair, but it byitself does not need backslash
quoting.  Which means cq_lookup[] table needs to somehow encode a
bit per byte that says "byte with this value itself does not need to
be backslash-quoted, but the entire thing needs to be placed in a
dq-pair", and/or next_quote_pos() needs be able to say "here is a
byte to cause us to enclose the whole thing in a dq-pair, but the
byte itself need not be backslash-quoted".

Of course none of the above becomes unnecessary if we scan the whole
string for SP before the main loop in quote-c-style-counted, but the
function was written to process the input in a single pass and such
a change defeats its design.  If we need to do it in two passes, we
can have the caller do so anyway, at least for now.  That thinking
lead to the final organization of the series, with two steps that
used to be preparatory for passing the flag down thru to the bottom
layer rebased out as a discardable appendix at the end.

Thanks.