From: Junio C Hamano <gitster@pobox•com>
To: Stefan Beller <sbeller@google•com>
Cc: pclouds@gmail•com, git@vger•kernel.org
Subject: Re: [RFC/PATCH] pathspec: allow escaped query values
Date: Wed, 01 Jun 2016 17:33:13 -0700 [thread overview]
Message-ID: <xmqq37owwhti.fsf@gitster.mtv.corp.google.com> (raw)
In-Reply-To: <20160601235233.21040-1-sbeller@google.com> (Stefan Beller's message of "Wed, 1 Jun 2016 16:52:33 -0700")
Stefan Beller <sbeller@google•com> writes:
> This change allows escaping characters by a backslash, such that the query
>
> git ls-files :(attr:whitespace=indent\,trail\,space)
>
> will match all path that have the value "indent,trail,space" for the
> whitespace attribute. To accomplish this, we need to modify two places.
> First `eat_long_magic` needs to not stop early upon seeing a comma or
> closing paren that is escaped. As a second step we need to remove any
> escaping from the attr value. For now we just remove any backslashes.
>
> Caveat: This doesn't allow for querying for values that have backslashes
> in them, e.g.
>
> git ls-files :(attr:backslashes=\\)
>
> that would ask for matches that have the `backslashes` value set to '\'.
> Signed-off-by: Stefan Beller <sbeller@google•com>
> ---
>
> * This applies on top of sb/pathspec-label
> * Junio does this come close to what you imagine for escaped commas?
I, being lazy, would have used %44 for comma, which would have
avoided touching higher level of the callchain. But "a backslash
always quotes the next byte, if you want to express a backslash,
double it" would probably be a more end-user friendly quoting
mechanism.
I do not offhand understand why the second example does not show the
paths with backslash attribute set to a single backslash, though.
> - am->value = xstrdup(&attr[attr_len + 1]);
> + am->value = attr_value_unquote(&attr[attr_len + 1]);
> if (strchr(am->value, '\\'))
> die(_("attr spec values must not contain backslashes"));
IOW, the "backslash is forbidden for now" IIUC was added there so
that we can introduce a quoting like this--once we decided that
quoting mechanism is via backslashes and have quoting support,
shouldn't the "values must not have backslash" just go?
> *copyfrom && *copyfrom != ')';
> copyfrom = nextat) {
> size_t len = strcspn(copyfrom, ",)");
> + while (len > 0 && copyfrom[len - 1] == '\\'
> + && (copyfrom[len] == ',' || copyfrom[len] == ')'))
> + len += strcspn(copyfrom + len + 1, ",)") + 1;
Good that we can use ')' in values, too, but I think this gets this
case wrong:
:(attr:foo=\\,icase)
where the value for 'foo' wants to be a single backslash, and comma
is to introduce another magic, not part of the value for 'foo'.
If you want to do the "backslash unconditionally quotes the next
byte no matter what is quoted", you'd need to lose the strcspn()
and iterate over the string yourself, I would think.
> if (copyfrom[len] == ',')
> nextat = copyfrom + len + 1;
> else
next prev parent reply other threads:[~2016-06-02 0:33 UTC|newest]
Thread overview: 16+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-06-01 23:52 [RFC/PATCH] pathspec: allow escaped query values Stefan Beller
2016-06-02 0:33 ` Junio C Hamano [this message]
2016-06-02 2:23 ` Stefan Beller
2016-06-02 0:38 ` Ramsay Jones
2016-06-02 2:20 ` Stefan Beller
2016-06-02 5:46 ` Junio C Hamano
2016-06-02 15:30 ` Ramsay Jones
2016-06-02 16:10 ` Junio C Hamano
2016-06-02 18:42 ` Ramsay Jones
2016-06-02 18:58 ` Junio C Hamano
2016-06-02 19:29 ` Junio C Hamano
2016-06-02 19:52 ` Ramsay Jones
2016-06-02 19:04 ` Stefan Beller
2016-06-02 19:44 ` Ramsay Jones
2016-06-02 19:46 ` Junio C Hamano
2016-06-02 19:53 ` Ramsay Jones
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=xmqq37owwhti.fsf@gitster.mtv.corp.google.com \
--to=gitster@pobox$(echo .)com \
--cc=git@vger$(echo .)kernel.org \
--cc=pclouds@gmail$(echo .)com \
--cc=sbeller@google$(echo .)com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox