Re: [PATCH v3 2/4] quote: add quote_path() flag to ignore config

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

Justin Tobler <jltobler@gmail.com> writes:

> The `quote_path()` function invokes `quote_c_style_counted()` to handle
> quoting. This means the output `quote_path()` is ultimately affected by
> `core.quotePath` configuration. In a subsequent commit, `quote_path()`
> will be used in a scenario where the output should remain consistent
> regardless of the current configuration.
>
> Introduce the `QUOTE_PATH_IGNORE_CONFIG` flag for `quote_path()`which
> when set instructs the underlying `quote_c_style_counted()` to also
> ignore the `core.quotePath` configuration when executed.
>
> Signed-off-by: Justin Tobler <jltobler@gmail.com>
> ---

I've already read what [3/4] does using this, but I do not quite see
why the scenario the quote_path() function is called requires that
the output should remain consistent.

As it is possible that a path has any byte other than NUL in it, the
recipient MUST be prepared to unquote it before being able to use
it.  So if a run of "rev-list --info" in the same repository with
the same arguments tells a path as "abc" (without quotes) and
another one shows the same path as "abc" (with quotes) or even as
"\141\142\143", the consumer should be able to cope with the
variations and see that they are the same path, no?  At least, I do
not quite see why we require consistency across different settings
of the configuration.

If we drop the first two patches, as long as we use a known value in
core.quotepath in tests in t6022, everything should reliably work,
no?

When we do need to quote SP (i.e. outside the "diff" output, for
which originally cquote machinery was invented), the default cquote
mechanism does not help, so we need to allow _some_ tweak from the
caller's side, but I do not know if I agree that "no, we do not
allow bytes with 8-bit set without quoting" is a good idea in this
age.

Thanks.

>  quote.c | 13 ++++++++++---
>  quote.h |  3 ++-
>  2 files changed, 12 insertions(+), 4 deletions(-)
>
> diff --git a/quote.c b/quote.c
> index d129c1de70..baec34ca94 100644
> --- a/quote.c
> +++ b/quote.c
> @@ -370,10 +370,18 @@ char *quote_path(const char *in, const char *prefix, struct strbuf *out, unsigne
>  {
>  	struct strbuf sb = STRBUF_INIT;
>  	const char *rel = relative_path(in, prefix, &sb);
> -	int force_dq = ((flags & QUOTE_PATH_QUOTE_SP) && strchr(rel, ' '));
> +	unsigned cquote_flags = 0;
> +	int force_dq = 0;
>  
>  	strbuf_reset(out);
>  
> +	if ((flags & QUOTE_PATH_QUOTE_SP) && strchr(rel, ' ')) {
> +		force_dq = 1;
> +		cquote_flags &= CQUOTE_NODQ;
> +	}
> +	if (flags & QUOTE_PATH_IGNORE_CONFIG)
> +		cquote_flags &= CQUOTE_IGNORE_CONFIG;
> +
>  	/*
>  	 * If the caller wants us to enclose the output in a dq-pair
>  	 * whether quote_c_style_counted() needs to, we do it ourselves
> @@ -381,8 +389,7 @@ char *quote_path(const char *in, const char *prefix, struct strbuf *out, unsigne
>  	 */
>  	if (force_dq)
>  		strbuf_addch(out, '"');
> -	quote_c_style_counted(rel, strlen(rel), out, NULL,
> -			      force_dq ? CQUOTE_NODQ : 0);
> +	quote_c_style_counted(rel, strlen(rel), out, NULL, cquote_flags);
>  	if (force_dq)
>  		strbuf_addch(out, '"');
>  	strbuf_release(&sb);
> diff --git a/quote.h b/quote.h
> index 2a793fbef6..84903951ef 100644
> --- a/quote.h
> +++ b/quote.h
> @@ -94,7 +94,8 @@ void write_name_quoted_relative(const char *name, const char *prefix,
>  
>  /* quote path as relative to the given prefix */
>  char *quote_path(const char *in, const char *prefix, struct strbuf *out, unsigned flags);
> -#define QUOTE_PATH_QUOTE_SP 01
> +#define QUOTE_PATH_QUOTE_SP	 (1u << 0)
> +#define QUOTE_PATH_IGNORE_CONFIG (1u << 1)
>  
>  /* quoting as a string literal for other languages */
>  void perl_quote_buf(struct strbuf *sb, const char *src);