[PATCH] git grep: Add "-Z/--null" option as in GNU's grep.

[PATCH] git grep: Add "-Z/--null" option as in GNU's grep.

[Raphael Zimmerer]

Here's a trivial patch that adds "-Z" and "--null" options to "git
grep" equal to GNU's grep.
So things like 'git grep -l -Z "$FOO" | xargs -0 sed -i "s/$FOO/$BOO/"'
are more comfortable.

Signed-off-by: Raphael Zimmerer <killekulla@rdrz.de>
---
 Documentation/git-grep.txt |    6 ++++++
 builtin-grep.c             |    7 +++++++
 grep.c                     |   14 +++++++++++---
 grep.h                     |    1 +
 4 files changed, 25 insertions(+), 3 deletions(-)

diff --git a/Documentation/git-grep.txt b/Documentation/git-grep.txt
index fa4d133..9317377 100644
--- a/Documentation/git-grep.txt
+++ b/Documentation/git-grep.txt
@@ -15,6 +15,7 @@ SYNOPSIS
 	   [-E | --extended-regexp] [-G | --basic-regexp]
 	   [-F | --fixed-strings] [-n]
 	   [-l | --files-with-matches] [-L | --files-without-match]
+	   [-Z | --null]
 	   [-c | --count] [--all-match]
 	   [-A <post-context>] [-B <pre-context>] [-C <context>]
 	   [-f <file>] [-e] <pattern>
@@ -94,6 +95,11 @@ OPTIONS
 	For better compatibility with 'git-diff', --name-only is a
 	synonym for --files-with-matches.
 
+-Z::
+--null::
+	Output \0 instead of the character that normally follows a
+	file name.
+
 -c::
 --count::
 	Instead of showing every matched line, show the number of
diff --git a/builtin-grep.c b/builtin-grep.c
index 3a51662..fb2abe4 100644
--- a/builtin-grep.c
+++ b/builtin-grep.c
@@ -295,6 +295,8 @@ static int external_grep(struct grep_opt *opt, const char **paths, int cached)
 		push_arg("-l");
 	if (opt->unmatch_name_only)
 		push_arg("-L");
+	if (opt->null_following_name)
+		push_arg("-Z");
 	if (opt->count)
 		push_arg("-c");
 	if (opt->post_context || opt->pre_context) {
@@ -599,6 +601,11 @@ int cmd_grep(int argc, const char **argv, const char *prefix)
 			opt.unmatch_name_only = 1;
 			continue;
 		}
+		if (!strcmp("-Z", arg) ||
+		    !strcmp("--null", arg)) {
+			opt.null_following_name = 1;
+			continue;
+		}
 		if (!strcmp("-c", arg) ||
 		    !strcmp("--count", arg)) {
 			opt.count = 1;
diff --git a/grep.c b/grep.c
index 7063511..2619dbf 100644
--- a/grep.c
+++ b/grep.c
@@ -239,6 +239,8 @@ static int word_char(char ch)
 static void show_line(struct grep_opt *opt, const char *bol, const char *eol,
 		      const char *name, unsigned lno, char sign)
 {
+	if (opt->null_following_name)
+		sign = 0;
 	if (opt->pathname)
 		printf("%s%c", name, sign);
 	if (opt->linenum)
@@ -246,6 +248,11 @@ static void show_line(struct grep_opt *opt, const char *bol, const char *eol,
 	printf("%.*s\n", (int)(eol-bol), bol);
 }
 
+static void show_name(struct grep_opt *opt, const char *name)
+{
+	printf("%s%c", name, opt->null_following_name ? 0 : '\n');
+}
+
 static int fixmatch(const char *pattern, char *line, regmatch_t *match)
 {
 	char *hit = strstr(line, pattern);
@@ -489,7 +496,7 @@ static int grep_buffer_1(struct grep_opt *opt, const char *name,
 				return 1;
 			}
 			if (opt->name_only) {
-				printf("%s\n", name);
+				show_name(opt, name);
 				return 1;
 			}
 			/* Hit at this line.  If we haven't shown the
@@ -555,7 +562,7 @@ static int grep_buffer_1(struct grep_opt *opt, const char *name,
 		return 0;
 	if (opt->unmatch_name_only) {
 		/* We did not see any hit, so we want to show this */
-		printf("%s\n", name);
+		show_name(opt, name);
 		return 1;
 	}
 
@@ -565,7 +572,8 @@ static int grep_buffer_1(struct grep_opt *opt, const char *name,
 	 * make it another option?  For now suppress them.
 	 */
 	if (opt->count && count)
-		printf("%s:%u\n", name, count);
+		printf("%s%c%u\n", name,
+		       opt->null_following_name ? 0 : ':', count);
 	return !!last_hit;
 }
 
diff --git a/grep.h b/grep.h
index 59b3f87..45a222d 100644
--- a/grep.h
+++ b/grep.h
@@ -74,6 +74,7 @@ struct grep_opt {
 	unsigned extended:1;
 	unsigned relative:1;
 	unsigned pathname:1;
+	unsigned null_following_name:1;
 	int regflags;
 	unsigned pre_context;
 	unsigned post_context;
-- 
1.5.6.3

Re: [PATCH] git grep: Add "-Z/--null" option as in GNU's grep.

[Shawn O. Pearce]

Raphael Zimmerer <killekulla@rdrz.de> wrote:
> Here's a trivial patch that adds "-Z" and "--null" options to "git
> grep" equal to GNU's grep.
> So things like 'git grep -l -Z "$FOO" | xargs -0 sed -i "s/$FOO/$BOO/"'
> are more comfortable.

Elsewhere in Git we call this "-z", like "git ls-tree -z", "git
log -z".  Should we match grep or git convention here?
 
>  Documentation/git-grep.txt |    6 ++++++
>  builtin-grep.c             |    7 +++++++
>  grep.c                     |   14 +++++++++++---
>  grep.h                     |    1 +

-- 
Shawn.

Re: [PATCH] git grep: Add "-Z/--null" option as in GNU's grep.

[Raphael Zimmerer]

On Tue, Sep 30, 2008 at 04:16:19PM -0700, Shawn O. Pearce wrote:
> Elsewhere in Git we call this "-z", like "git ls-tree -z", "git
> log -z".  Should we match grep or git convention here?

I'd tend to grep's convention, as most options of git-grep mimic those
of grep. grep uses "-z" for \0 on _input_, so that would be very
confusing for grep users...

- Raphael

Re: [PATCH] git grep: Add "-Z/--null" option as in GNU's grep.

[Pierre Habouzit]

[-- Attachment #1: Type: text/plain, Size: 582 bytes --]

On Tue, Sep 30, 2008 at 11:13:10PM +0000, Raphael Zimmerer wrote:
> +	printf("%s%c", name, opt->null_following_name ? 0 : '\n');

I know I'm nitpicking and I don't know what the git custom on this
really is, but I tend to prefer '\0' when in the context of a char.
There is no confusion here of course, but I believe it to be a sane
habit. (In the same vein that it's ugly to use 0 for NULL ;p).

-- 
·O·  Pierre Habouzit
··O                                                madcoder@debian.org
OOO                                                http://www.madism.org

[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]

Re: [PATCH] git grep: Add "-Z/--null" option as in GNU's grep.

[Johannes Schindelin]

Hi,

On Wed, 1 Oct 2008, Raphael Zimmerer wrote:

> On Tue, Sep 30, 2008 at 04:16:19PM -0700, Shawn O. Pearce wrote:
> > Elsewhere in Git we call this "-z", like "git ls-tree -z", "git log 
> > -z".  Should we match grep or git convention here?
> 
> I'd tend to grep's convention, as most options of git-grep mimic those 
> of grep. grep uses "-z" for \0 on _input_, so that would be very 
> confusing for grep users...

I tend to disagree.  Git is _already_ perceived as too heterogenous, and 
we should not add to that pile.

Ciao,
Dscho

Re: [PATCH] git grep: Add "-Z/--null" option as in GNU's grep.

[Raphael Zimmerer]

On Wed, Oct 01, 2008 at 03:15:34PM +0200, Johannes Schindelin wrote:
> On Wed, 1 Oct 2008, Raphael Zimmerer wrote:
> > On Tue, Sep 30, 2008 at 04:16:19PM -0700, Shawn O. Pearce wrote:
> > > Elsewhere in Git we call this "-z", like "git ls-tree -z", "git log 
> > > -z".  Should we match grep or git convention here?
> > 
> > I'd tend to grep's convention, as most options of git-grep mimic those 
> > of grep. grep uses "-z" for \0 on _input_, so that would be very 
> > confusing for grep users...
> 
> I tend to disagree.  Git is _already_ perceived as too heterogenous, and 
> we should not add to that pile.

How about discarding "-Z" from my patch, and only leave in "--null"?
That removes ambiguity _and_ is compatible to GNU grep.

- Raphael

Re: [PATCH] git grep: Add "-Z/--null" option as in GNU's grep.

[Shawn O. Pearce]

Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
> On Wed, 1 Oct 2008, Raphael Zimmerer wrote:
> 
> > On Tue, Sep 30, 2008 at 04:16:19PM -0700, Shawn O. Pearce wrote:
> > > Elsewhere in Git we call this "-z", like "git ls-tree -z", "git log 
> > > -z".  Should we match grep or git convention here?
> > 
> > I'd tend to grep's convention, as most options of git-grep mimic those 
> > of grep. grep uses "-z" for \0 on _input_, so that would be very 
> > confusing for grep users...
> 
> I tend to disagree.  Git is _already_ perceived as too heterogenous, and 
> we should not add to that pile.

I already have my brain wired that "\0 terminators in Git are -z".
Thus I'd assume "git grep -z  .. | xargs -0" would work.  Today it
doesn't without this patch, but if the patch was added I'd assume
it would work.

Perhaps I'm too close to git as a contributor and experienced user
to realize any brain damage.

I'd rather stick to "-z" in Git.  At least its consistent.

Its not like tools outside of Git are all that consistent.  GNU
grep uses --null/-Z.  xargs and perl use -0.  find uses -print0.
The human at the keyboard already has to navigate this rats nest
between different tools, but within a tool (git) we should be as
consistent as we can.

-- 
Shawn.

Re: [PATCH] git grep: Add "-Z/--null" option as in GNU's grep.

[Shawn O. Pearce]

Pierre Habouzit <madcoder@debian.org> wrote:
> On Tue, Sep 30, 2008 at 11:13:10PM +0000, Raphael Zimmerer wrote:
> > +	printf("%s%c", name, opt->null_following_name ? 0 : '\n');
> 
> I know I'm nitpicking and I don't know what the git custom on this
> really is, but I tend to prefer '\0' when in the context of a char.
> There is no confusion here of course, but I believe it to be a sane
> habit. (In the same vein that it's ugly to use 0 for NULL ;p).

Its a valid nitpick.  NUL is '\0' when dealing with chars in Git,
NULL is NULL, and 0 is the integer 0.  That line should read '\0'
to conform with the other code already in git.git.

-- 
Shawn.

Re: [PATCH] git grep: Add "-Z/--null" option as in GNU's grep.

[Raphael Zimmerer]

On Wed, Oct 01, 2008 at 07:52:07AM -0700, Shawn O. Pearce wrote:
> Its not like tools outside of Git are all that consistent.  GNU
> grep uses --null/-Z.  xargs and perl use -0.  find uses -print0.
> The human at the keyboard already has to navigate this rats nest
> between different tools, but within a tool (git) we should be as
> consistent as we can.

... and git-config uses --null/-z.

I will send an updated patch with --null/-z.

- Raphael

[PATCH v2] git grep: Add "-z/--null" option as in GNU's grep.

[Raphael Zimmerer]

Here's a trivial patch that adds "-z" and "--null" options to "git
grep". It was discussed on the mailing-list that git's "-z"
convention should be used instead of GNU grep's "-Z".
So things like 'git grep -l -z "$FOO" | xargs -0 sed -i "s/$FOO/$BOO/"'
do work now.

Signed-off-by: Raphael Zimmerer <killekulla@rdrz.de>
---

Changes from first patch:
    * "-Z" -> "-z"
    * use '\0' instead of 0
Regards
  Raphael

 Documentation/git-grep.txt |    6 ++++++
 builtin-grep.c             |    8 ++++++++
 grep.c                     |   14 +++++++++++---
 grep.h                     |    1 +
 4 files changed, 26 insertions(+), 3 deletions(-)

diff --git a/Documentation/git-grep.txt b/Documentation/git-grep.txt
index fa4d133..553da6c 100644
--- a/Documentation/git-grep.txt
+++ b/Documentation/git-grep.txt
@@ -15,6 +15,7 @@ SYNOPSIS
 	   [-E | --extended-regexp] [-G | --basic-regexp]
 	   [-F | --fixed-strings] [-n]
 	   [-l | --files-with-matches] [-L | --files-without-match]
+	   [-z | --null]
 	   [-c | --count] [--all-match]
 	   [-A <post-context>] [-B <pre-context>] [-C <context>]
 	   [-f <file>] [-e] <pattern>
@@ -94,6 +95,11 @@ OPTIONS
 	For better compatibility with 'git-diff', --name-only is a
 	synonym for --files-with-matches.
 
+-z::
+--null::
+	Output \0 instead of the character that normally follows a
+	file name.
+
 -c::
 --count::
 	Instead of showing every matched line, show the number of
diff --git a/builtin-grep.c b/builtin-grep.c
index 3a51662..2241324 100644
--- a/builtin-grep.c
+++ b/builtin-grep.c
@@ -295,6 +295,9 @@ static int external_grep(struct grep_opt *opt, const char **paths, int cached)
 		push_arg("-l");
 	if (opt->unmatch_name_only)
 		push_arg("-L");
+	if (opt->null_following_name)
+		// in GNU grep git's "-z" translates to "-Z"
+		push_arg("-Z");
 	if (opt->count)
 		push_arg("-c");
 	if (opt->post_context || opt->pre_context) {
@@ -599,6 +602,11 @@ int cmd_grep(int argc, const char **argv, const char *prefix)
 			opt.unmatch_name_only = 1;
 			continue;
 		}
+		if (!strcmp("-z", arg) ||
+		    !strcmp("--null", arg)) {
+			opt.null_following_name = 1;
+			continue;
+		}
 		if (!strcmp("-c", arg) ||
 		    !strcmp("--count", arg)) {
 			opt.count = 1;
diff --git a/grep.c b/grep.c
index 7063511..e2c190a 100644
--- a/grep.c
+++ b/grep.c
@@ -239,6 +239,8 @@ static int word_char(char ch)
 static void show_line(struct grep_opt *opt, const char *bol, const char *eol,
 		      const char *name, unsigned lno, char sign)
 {
+	if (opt->null_following_name)
+		sign = '\0';
 	if (opt->pathname)
 		printf("%s%c", name, sign);
 	if (opt->linenum)
@@ -246,6 +248,11 @@ static void show_line(struct grep_opt *opt, const char *bol, const char *eol,
 	printf("%.*s\n", (int)(eol-bol), bol);
 }
 
+static void show_name(struct grep_opt *opt, const char *name)
+{
+	printf("%s%c", name, opt->null_following_name ? '\0' : '\n');
+}
+
 static int fixmatch(const char *pattern, char *line, regmatch_t *match)
 {
 	char *hit = strstr(line, pattern);
@@ -489,7 +496,7 @@ static int grep_buffer_1(struct grep_opt *opt, const char *name,
 				return 1;
 			}
 			if (opt->name_only) {
-				printf("%s\n", name);
+				show_name(opt, name);
 				return 1;
 			}
 			/* Hit at this line.  If we haven't shown the
@@ -555,7 +562,7 @@ static int grep_buffer_1(struct grep_opt *opt, const char *name,
 		return 0;
 	if (opt->unmatch_name_only) {
 		/* We did not see any hit, so we want to show this */
-		printf("%s\n", name);
+		show_name(opt, name);
 		return 1;
 	}
 
@@ -565,7 +572,8 @@ static int grep_buffer_1(struct grep_opt *opt, const char *name,
 	 * make it another option?  For now suppress them.
 	 */
 	if (opt->count && count)
-		printf("%s:%u\n", name, count);
+		printf("%s%c%u\n", name,
+		       opt->null_following_name ? '\0' : ':', count);
 	return !!last_hit;
 }
 
diff --git a/grep.h b/grep.h
index 59b3f87..45a222d 100644
--- a/grep.h
+++ b/grep.h
@@ -74,6 +74,7 @@ struct grep_opt {
 	unsigned extended:1;
 	unsigned relative:1;
 	unsigned pathname:1;
+	unsigned null_following_name:1;
 	int regflags;
 	unsigned pre_context;
 	unsigned post_context;
-- 
1.5.6.5

Re: [PATCH v2] git grep: Add "-z/--null" option as in GNU's grep.

[Shawn O. Pearce]

Raphael Zimmerer <killekulla@rdrz.de> wrote:
> diff --git a/builtin-grep.c b/builtin-grep.c
> index 3a51662..2241324 100644
> --- a/builtin-grep.c
> +++ b/builtin-grep.c
> @@ -295,6 +295,9 @@ static int external_grep(struct grep_opt *opt, const char **paths, int cached)
>  		push_arg("-l");
>  	if (opt->unmatch_name_only)
>  		push_arg("-L");
> +	if (opt->null_following_name)
> +		// in GNU grep git's "-z" translates to "-Z"
> +		push_arg("-Z");

We use /* */ style comments in Git.  I've amended the patch with
the simple // -> /* */ translation.

The rest of this change looks good to me.  Its queued for next.

-- 
Shawn.