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é
next prev 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).