Re: [PATCH v3 2/4] quote: add quote_path() flag to ignore config
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Feb 3, 2025, 22:52 UTC
- Message-ID
- <xmqqtt9azlnx.fsf@gitster.g>
- In-Reply-To
- <20250201201658.11562-3-jltobler@gmail.com>
Justin Tobler <jltobler@gmail.com> writes:
Show 12 quoted lines
> 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.
Show 52 quoted lines
> 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);