Re: [PATCH 5/6] grep: add option -p/--show-function

["René Scharfe" <rene.scharfe@lsrfire.ath.cx>]

Junio C Hamano schrieb:
> René Scharfe <rene.scharfe@lsrfire.ath.cx> writes:
> 
>> The new option -p instructs git grep to print the previous function
>> definition as a context line, similar to diff -p.  Such context lines
>> are marked with an equal sign instead of a dash.  This option
>> complements the existing context options -A, -B, -C.
> 
> I have to say that I am a bit confused with these '==' markers.

You may be glad to hear that initial versions of this patch followed
even stranger rules for showing these markers. ;-)

>     $ ./git-grep -p -n -e pre_context -- '*.c'
>     builtin-grep.c=238=static int flush_grep(struct grep_opt *opt,
>     builtin-grep.c:288:	if (opt->pre_context || opt->post_context ||...
>     ==
>     builtin-grep.c=328=static int external_grep(struct grep_opt *opt, co...
>     builtin-grep.c:363:	if (opt->post_context || opt->pre_context) {
>     builtin-grep.c:364:		if (opt->post_context != opt->pre_co...
>     builtin-grep.c:365:			if (opt->pre_context) {
>     builtin-grep.c:368:						"%u"...
>     ==
>     builtin-grep.c=586=static int context_callback(const struct option *...
> 
> Am I correct to say that '==' tells us that it has been showing the hits
> in the previous function shown on the =$lineno= line, but now it will show
> hits from a different function?

With -p, the previous function of a hit is marked with an "=" (or
"=$lineno=" with -n).  "==" is printed immediately before such a line,
to separate it from other functions, yes.

"--" could be printed between lines 238 and 288, between lines 328 and
363 (after the function lines), and between lines 365 and 368, but that
would bloat the output for no gain, IMHO.  It would mean "here we've
skipped over a few lines", which is correct, but it's also common and
expected.  So I left out print of "--" markers completely if -p is used
without -[ABC].

> When used with -[ABC], it gets even confusing.
> 
>     $ ./git-grep -C1 -p -n -e pre_context -- '*.c'
>     builtin-grep.c=238=static int flush_grep(struct grep_opt *opt,
>     --
>     builtin-grep.c-287-
>     builtin-grep.c:288:	if (opt->pre_context || opt->post_context ||...
>     builtin-grep.c-289-		/*
>     ==
>     builtin-grep.c=328=static int external_grep(struct grep_opt *opt, co...
>     --
>     builtin-grep.c-362-		push_arg("-c");
>     builtin-grep.c:363:	if (opt->post_context || opt->pre_context) {
>     builtin-grep.c:364:		if (opt->post_context != opt->pre_co...
>     builtin-grep.c:365:			if (opt->pre_context) {
>     builtin-grep.c-366-				push_arg("-B");
>     builtin-grep.c-367-				len += snprintf(argp...
>     builtin-grep.c:368:						"%u"...
>     builtin-grep.c-369-				if (sizeof(randarg) ...
>     ==
>     builtin-grep.c=586=static int context_callback(const struct option *...
>     --
> 
> What do "--" lines tell us in these cases?  For example, does the first
> one tells us that the -p output shown, which was from line 238, is outside
> of the context of the hit it will show at line 288?
>
> I am in no way complaining.  My eyes are just not yet used to the output
> format and are having hard time getting used to them and/or appreciating
> how the separation these dividing lines give us is helpful.

Here's another example, which is potentially even more confusing:

	$ git grep -p -n markers grep.c
	grep.c=748=static void clr_hit_marker(struct grep_expr *x)
	grep.c:750:	/* All-hit markers are meaningful only at...
	==
	grep.c=762=static int chk_hit_marker(struct grep_expr *x)
	grep.c:764:	/* Top level nodes have hit markers.  See...
	==
	grep.c=774=int grep_buffer(struct grep_opt *opt, const ch...
	grep.c:784:	 * We first clear hit markers from them.


	$ git grep -p -n -B3 markers grep.c
	grep.c-747-
	grep.c=748=static void clr_hit_marker(struct grep_expr *x)
	grep.c-749-{
	grep.c:750:	/* All-hit markers are meaningful only at...
	--
	grep.c-761-
	grep.c=762=static int chk_hit_marker(struct grep_expr *x)
	grep.c-763-{
	grep.c:764:	/* Top level nodes have hit markers.  See...
	==
	grep.c=774=int grep_buffer(struct grep_opt *opt, const ch...
	--
	grep.c-781-		return grep_buffer_1(opt, name, b...
	grep.c-782-
	grep.c-783-	/* Otherwise the toplevel "or" terms hit ...
	grep.c:784:	 * We first clear hit markers from them.

The "==" after line 750 is changed to a "--" with -B3 or more.


I'm not so sure about the usefulness of these markers myself, but the
rules to generate them are quite simple.  The "==" markers exist only
because I wanted the output of -p alone to be as compact as possible,
but without changing the meaning of the "--" markers.

So the meaning of the "--" markers is the same as before: "lines have
been skipped".  "==" means the same, but also "the next line is a
function line".  And -p alone only shows these specialized markers, for
brevity.

Hmm, even shorter would be to leave out these "==" things entirely:

	# mock-up
	$ git grep -p -n markers grep.c
	grep.c=746=static void clr_hit_marker(struct grep_expr *x)
	grep.c:748:     /* All-hit markers are meaningful only at...
	grep.c=760=static int chk_hit_marker(struct grep_expr *x)
	grep.c:762:     /* Top level nodes have hit markers.  See...
	grep.c=772=int grep_buffer(struct grep_opt *opt, const ch...
	grep.c:782:      * We first clear hit markers from them.

Is that still readable?