git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "René Scharfe" <rene.scharfe@lsrfire.ath.cx>
To: Mark Lodato <lodatom@gmail.com>
Cc: git@vger.kernel.org
Subject: Re: [PATCH 0/4] grep: add more information to hunk separators
Date: Mon, 26 Mar 2012 18:16:36 +0200	[thread overview]
Message-ID: <4F709664.1060206@lsrfire.ath.cx> (raw)
In-Reply-To: <1332729705-9283-1-git-send-email-lodatom@gmail.com>

Am 26.03.2012 04:41, schrieb Mark Lodato:
> 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é

  parent reply	other threads:[~2012-03-26 16:16 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-03-26  2:41 [PATCH 0/4] grep: add more information to hunk separators Mark Lodato
2012-03-26  2:41 ` [PATCH 1/4] grep doc: add --break / --heading / -W to synopsis Mark Lodato
2012-03-26  2:41 ` [PATCH 2/4] add tests for grep --heading with context Mark Lodato
2012-03-26  2:41 ` [PATCH 3/4] grep: move code to print hunk markers after heading Mark Lodato
2012-03-26  2:41 ` [PATCH 4/4] grep: add --hunk-heading option Mark Lodato
2012-03-26  5:14 ` [PATCH 0/4] grep: add more information to hunk separators Junio C Hamano
2012-03-26 16:16   ` René Scharfe
2012-03-26 18:05     ` Junio C Hamano
2012-03-26 18:48   ` Bert Wesarg
2012-03-26 16:16 ` René Scharfe [this message]
2012-03-26 18:01   ` Junio C Hamano
2012-03-26 21:12     ` René Scharfe
2012-03-26 21:19       ` Junio C Hamano
2012-03-27  5:31 ` [PATCH 5/4] move sane_truncate_line to utf8_truncate_line Mark Lodato
2012-03-27  5:31 ` [PATCH 6/4] add grep.hunkHeadingFunction option Mark Lodato

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=4F709664.1060206@lsrfire.ath.cx \
    --to=rene.scharfe@lsrfire.ath.cx \
    --cc=git@vger.kernel.org \
    --cc=lodatom@gmail.com \
    /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.
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 NNTP newsgroup(s).