Re: [PATCH 0/4] grep: add more information to hunk separators

From: René Scharfe <rene.scharfe@lsrfire.ath.cx>
Date: Mar 26, 2012, 16:16 UTC
Message-ID: <4F709664.1060206@lsrfire.ath.cx>
In-Reply-To: <1332729705-9283-1-git-send-email-lodatom@gmail.com>

Am 26.03.2012 04:41, schrieb Mark Lodato:

Show 50 quoted lines

> This patch series adds a new `grep --hunk-heading' option that moves the
> filename and line number to the hunk separator lines ("--") rather than at the
> beginning of each matching (or context) line.  In my opinion, this makes the
> output easier to read, especially when combined with `--heading'.
>
> I am not sure that "hunk-heading" is the best term, so I welcome ideas on
> better names.
>
> Here's an example:
>
>      # Current behavior:
>      $ git grep -p -C1 -n list_common -- git.c
>      git.c=531=int main(int argc, const char **argv)
>      --
>      git.c-570-              printf("usage: %s\n\n", git_usage_string);
>      git.c:571:              list_common_cmds_help();
>      git.c-572-              printf("\n%s\n", git_more_info_string);
>
>      # New option:
>      $ git grep -p -C1 --hunk-heading list_common -- git.c
>      -- git.c:531 --
>      int main(int argc, char argv)
>      -- git.c:570 --
>                      printf("usage: %s\n\n", git_usage_string);
>                      list_common_cmds_help();
>                      printf("\n%s\n", git_more_info_string);
>
>      # New option with --heading:
>      $ git grep -p -C1 --hunk-heading --heading list_common -- git.c
>      git.c
>      -- 531 --
>      int main(int argc, char argv)
>      -- 570 --
>                      printf("usage: %s\n\n", git_usage_string);
>                      list_common_cmds_help();
>                      printf("\n%s\n", git_more_info_string);
>
> Originally, I had envisioned also moving the function name (`-p') to the hunk
> header, similar to the diff context line.  For example:
>
>      -- git.c:570 -- int main(int argc, char argv)
>                      printf("usage: %s\n\n", git_usage_string);
>                      list_common_cmds_help();
>                      printf("\n%s\n", git_more_info_string);
>
> After implementing this feature, I was not happy with the result and
> subsequently removed it.  To me, the output was too cluttered and the line
> number was ambigous.  For example, in the above, it is not obvious to me that
> line 570 is the "printf" line and not the "int main" line.  Still, if you
> would like to see the patch to implement this feature, please let me know.

Interesting.

By the way, I keep this alias in my config (a single line), to mimic ack (http://betterthangrep.com/) -- another way to format results, with similar goals:

	ack = -c color.grep.filename='bold green' \
	-c color.grep.match='black yellow' grep --break --heading -n

Back to your patch: Why the second set of "--" after the line number? I can see it make sense if a section comment follows, but not without one.

Looking at the above, I thought: We have unified diffs between two files, we have combined diffs between more than two, what about showing grep results as one-sided unified diffs? ("What's the sound of one hand clapping?" :-)

	--- a/git.c
	@ -570,3 @ int main(int argc, const char **argv)
	-		printf("usage: %s\n\n", git_usage_string);
	:		list_common_cmds_help();
	-		printf("\n%s\n", git_more_info_string);

Pro: Generalization of an established format for showing interesting 
parts of a file.  Less duplication of meta-information.  Markers that 
tell us the kind of the shown lines are kept ("-" for context, ":" for 
matches).  Machine parsable.

Con: Why the "a/" prefix?  One-sided diffs, srsly?

René