Re: [PATCH 4/4] grep -O: allow optional argument specifying the pager (or editor)

From: Junio C Hamano <gitster@pobox.com>
Date: Jun 13, 2010, 16:35 UTC
Message-ID: <7vvd9mevqy.fsf@alter.siamese.dyndns.org>
In-Reply-To: <20100612163946.GD5657@burratino>

Jonathan Nieder <jrnieder@gmail.com> writes:

Show 13 quoted lines

> diff --git a/Documentation/git-grep.txt b/Documentation/git-grep.txt
> index 8fdd8e1..d89ec32 100644
> --- a/Documentation/git-grep.txt
> +++ b/Documentation/git-grep.txt
> @@ -14,7 +14,7 @@ SYNOPSIS
>  	   [-E | --extended-regexp] [-G | --basic-regexp]
>  	   [-F | --fixed-strings] [-n]
>  	   [-l | --files-with-matches] [-L | --files-without-match]
> -	   [-O | --open-files-in-pager]
> +	   [(-O | --open-files-in-pager) [<pager>]]
>  	   [-z | --null]
>  	   [-c | --count] [--all-match] [-q | --quiet]
>  	   [--max-depth <depth>]

Hmm, does "git grep -e Heh -O frotz" look for Heh and show in the frotz pager, or does it look for Heh in paths under frotz/ directory and show hits in the default pager?

Show 10 quoted lines

> diff --git a/builtin/grep.c b/builtin/grep.c
> index 1e8b946..f32fbbc 100644
> --- a/builtin/grep.c
> +++ b/builtin/grep.c
> @@ -828,7 +828,7 @@ int cmd_grep(int argc, const char **argv, const char *prefix)
>  	int cached = 0;
>  	int seen_dashdash = 0;
>  	int external_grep_allowed__ignored;
> -	int show_in_pager = 0;
> +	const char *show_in_pager = NULL, *default_pager = "dummy";

If there were another instance of constant string "dummy" elsewhere in the program, is a clever compiler-linker combo allowed to optimize memory use by allocating one instance of such a string and pointing default_pager pointer to it? IOW, if the patch were:

> +	const char *show_in_pager = NULL, *default_pager = "dummy";
> +	const char *another_dummy = "dummy";

could another_dummy and default_pager start out with the same value?

This is just out of curiosity and does not affect correctness of the code, but I am wondering...