From: Johannes Schindelin <Johannes.Schindelin@gmx.de>
To: Matheus Tavares <matheus.bernardino@usp.br>
Cc: gitster@pobox.com, asheiduk@gmail.com, git@vger.kernel.org,
greg@hurrell.net, l.s.r@web.de
Subject: Re: [PATCH] grep: follow conventions for printing paths w/ unusual chars
Date: Sat, 18 Apr 2020 15:13:11 +0200 (CEST) [thread overview]
Message-ID: <nycvar.QRO.7.76.6.2004181509350.46@tvgsbejvaqbjf.bet> (raw)
In-Reply-To: <eaae7214925189f562056b1ee6972c05dcf76a32.1587103366.git.matheus.bernardino@usp.br>
Hi Matheus,
On Fri, 17 Apr 2020, Matheus Tavares wrote:
> grep does not follow the conventions used by other Git commands when
> printing paths that contain unusual characters (as double-quotes or
> newlines). Commands such as ls-files, commit, status and diff will:
>
> - Quote and escape unusual pathnames, by default.
> - Print names verbatim and unquoted when "-z" is used.
>
> But grep *never* quotes/escapes absolute paths with unusual chars and
> *always* quotes/escapes relative ones, even with "-z". Besides being
> inconsistent in its own output, the deviation from other Git commands
> can be confusing. So let's make it follow the two rules above and add
> some tests for this new behavior. Note that, making grep quote/escape
> all unusual paths by default, also make it fully compliant with the
> core.quotePath configuration, which is currently ignored for absolute
> paths.
>
> Reported-by: Greg Hurrell <greg@hurrell.net>
> Signed-off-by: Matheus Tavares <matheus.bernardino@usp.br>
> ---
> Documentation/git-grep.txt | 6 +++--
> builtin/grep.c | 46 ++++++++++++++++++++++++++++----------
> t/t7810-grep.sh | 44 ++++++++++++++++++++++++++++++++++++
> 3 files changed, 82 insertions(+), 14 deletions(-)
Unfortunately, this causes eight test failures on Windows:
https://dev.azure.com/gitgitgadget/git/_build/results?buildId=38023&view=ms.vss-test-web.build-test-results-tab
Could you please have a look? I suspect that this has something to do with
those new tests needing the `FUNNYNAMES` prereq.
Ciao,
Dscho
>
> diff --git a/Documentation/git-grep.txt b/Documentation/git-grep.txt
> index ddb6acc025..3109ce8fbe 100644
> --- a/Documentation/git-grep.txt
> +++ b/Documentation/git-grep.txt
> @@ -206,8 +206,10 @@ providing this option will cause it to die.
>
> -z::
> --null::
> - Output \0 instead of the character that normally follows a
> - file name.
> + Use \0 as the delimiter for pathnames in the output, and print
> + them verbatim. Without this option, pathnames with "unusual"
> + characters are quoted as explained for the configuration
> + variable core.quotePath (see git-config(1)).
>
> -o::
> --only-matching::
> diff --git a/builtin/grep.c b/builtin/grep.c
> index 99e2685090..bdf1a4bbc9 100644
> --- a/builtin/grep.c
> +++ b/builtin/grep.c
> @@ -295,6 +295,38 @@ static int grep_cmd_config(const char *var, const char *value, void *cb)
> return st;
> }
>
> +static void grep_source_name(struct grep_opt *opt, const char *filename,
> + int tree_name_len, struct strbuf *out)
> +{
> + strbuf_reset(out);
> +
> + if (opt->null_following_name) {
> + if (opt->relative && opt->prefix_length) {
> + struct strbuf rel_buf = STRBUF_INIT;
> + const char *rel_name =
> + relative_path(filename + tree_name_len,
> + opt->prefix, &rel_buf);
> +
> + if (tree_name_len)
> + strbuf_add(out, filename, tree_name_len);
> +
> + strbuf_addstr(out, rel_name);
> + strbuf_release(&rel_buf);
> + } else {
> + strbuf_addstr(out, filename);
> + }
> + return;
> + }
> +
> + if (opt->relative && opt->prefix_length)
> + quote_path_relative(filename + tree_name_len, opt->prefix, out);
> + else
> + quote_c_style(filename + tree_name_len, out, NULL, 0);
> +
> + if (tree_name_len)
> + strbuf_insert(out, 0, filename, tree_name_len);
> +}
> +
> static int grep_oid(struct grep_opt *opt, const struct object_id *oid,
> const char *filename, int tree_name_len,
> const char *path)
> @@ -302,13 +334,7 @@ static int grep_oid(struct grep_opt *opt, const struct object_id *oid,
> struct strbuf pathbuf = STRBUF_INIT;
> struct grep_source gs;
>
> - if (opt->relative && opt->prefix_length) {
> - quote_path_relative(filename + tree_name_len, opt->prefix, &pathbuf);
> - strbuf_insert(&pathbuf, 0, filename, tree_name_len);
> - } else {
> - strbuf_addstr(&pathbuf, filename);
> - }
> -
> + grep_source_name(opt, filename, tree_name_len, &pathbuf);
> grep_source_init(&gs, GREP_SOURCE_OID, pathbuf.buf, path, oid);
> strbuf_release(&pathbuf);
>
> @@ -334,11 +360,7 @@ static int grep_file(struct grep_opt *opt, const char *filename)
> struct strbuf buf = STRBUF_INIT;
> struct grep_source gs;
>
> - if (opt->relative && opt->prefix_length)
> - quote_path_relative(filename, opt->prefix, &buf);
> - else
> - strbuf_addstr(&buf, filename);
> -
> + grep_source_name(opt, filename, 0, &buf);
> grep_source_init(&gs, GREP_SOURCE_FILE, buf.buf, filename, filename);
> strbuf_release(&buf);
>
> diff --git a/t/t7810-grep.sh b/t/t7810-grep.sh
> index 7d7b396c23..ab495dba28 100755
> --- a/t/t7810-grep.sh
> +++ b/t/t7810-grep.sh
> @@ -72,6 +72,8 @@ test_expect_success setup '
> # Still a no-op.
> function dummy() {}
> EOF
> + echo unusual >"\"unusual\" pathname" &&
> + echo unusual >"t/nested \"unusual\" pathname" &&
> git add . &&
> test_tick &&
> git commit -m initial
> @@ -481,6 +483,48 @@ do
> git grep --count -h -e b $H -- ab >actual &&
> test_cmp expected actual
> '
> +
> + test_expect_success "grep $L should quote unusual pathnames" '
> + cat >expected <<-EOF &&
> + ${HC}"\"unusual\" pathname":unusual
> + ${HC}"t/nested \"unusual\" pathname":unusual
> + EOF
> + git grep unusual $H >actual &&
> + test_cmp expected actual
> + '
> +
> + test_expect_success "grep $L in subdir should quote unusual relative pathnames" '
> + cat >expected <<-EOF &&
> + ${HC}"nested \"unusual\" pathname":unusual
> + EOF
> + (
> + cd t &&
> + git grep unusual $H
> + ) >actual &&
> + test_cmp expected actual
> + '
> +
> + test_expect_success "grep -z $L with unusual pathnames" '
> + cat >expected <<-EOF &&
> + ${HC}"unusual" pathname:unusual
> + ${HC}t/nested "unusual" pathname:unusual
> + EOF
> + git grep -z unusual $H >actual &&
> + tr "\0" ":" <actual >actual-replace-null &&
> + test_cmp expected actual-replace-null
> + '
> +
> + test_expect_success "grep -z $L in subdir with unusual relative pathnames" '
> + cat >expected <<-EOF &&
> + ${HC}nested "unusual" pathname:unusual
> + EOF
> + (
> + cd t &&
> + git grep -z unusual $H
> + ) >actual &&
> + tr "\0" ":" <actual >actual-replace-null &&
> + test_cmp expected actual-replace-null
> + '
> done
>
> cat >expected <<EOF
> --
> 2.26.0
>
>
>
next prev parent reply other threads:[~2020-04-18 13:13 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-04-13 21:55 git-grep's "-z" option misbehaves in subdirectory Greg Hurrell
2020-04-13 23:33 ` Junio C Hamano
2020-04-14 7:42 ` Matheus Tavares
2020-04-16 18:59 ` git-grep's "-z" option misbehaves in subdirectory Matheus Tavares Bernardino
2020-04-16 20:07 ` Junio C Hamano
2020-04-17 6:04 ` [PATCH] grep: follow conventions for printing paths w/ unusual chars Matheus Tavares
2020-04-17 6:45 ` Junio C Hamano
2020-04-17 21:19 ` Matheus Tavares Bernardino
2020-04-17 21:35 ` Junio C Hamano
2020-04-18 13:13 ` Johannes Schindelin [this message]
2020-04-18 14:56 ` Johannes Schindelin
2020-04-19 6:27 ` Matheus Tavares Bernardino
2020-04-19 6:33 ` [PATCH v2] " Matheus Tavares
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
List information: http://vger.kernel.org/majordomo-info.html
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=nycvar.QRO.7.76.6.2004181509350.46@tvgsbejvaqbjf.bet \
--to=johannes.schindelin@gmx.de \
--cc=asheiduk@gmail.com \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=greg@hurrell.net \
--cc=l.s.r@web.de \
--cc=matheus.bernardino@usp.br \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this public inbox
https://80x24.org/mirrors/git.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).