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

[Jonathan Nieder <jrnieder@gmail.com>]

Junio C Hamano wrote:
> Jonathan Nieder <jrnieder@gmail.com> writes:

>> -	   [-O | --open-files-in-pager]
>> +	   [(-O | --open-files-in-pager) [<pager>]]
[...]
>
> 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?

As Paolo mentioned, it is the latter.  Patch for squashing
is below.

>> +	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?

For a moment, you had me worried: would an (insane) compiler be
allowed to intern strings that appear as arguments, making argv[i]
actually compare equal to default_pager?

Luckily, the answer is no, because the strings pointed to in argv
are guaranteed to be modifiable (see: Execution environments → Hosted
environment → Program startup).

-- 8< --
Subject: grep -O: do not advertize non-"sticked" form in documentation

To avoid ambiguity in option parsing, the -O option will
only take an argument if it is stuck to the option like
-Ovi or --open-files-in-pager=vim.  The un-sticked form
-O vi means to search paths under 'vi' and show hits in
the default pager.

Noticed-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
 Documentation/git-grep.txt |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/Documentation/git-grep.txt b/Documentation/git-grep.txt
index d89ec32..0e12fe4 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) [<pager>]]
+	   [-O[<pager>] | --open-files-in-pager[=<pager>]]
 	   [-z | --null]
 	   [-c | --count] [--all-match] [-q | --quiet]
 	   [--max-depth <depth>]
@@ -106,7 +106,7 @@ OPTIONS
 	synonym for `--files-with-matches`.
 
--O [<pager>]::
---open-files-in-pager [<pager>]::
+-O[<pager>]::
+--open-files-in-pager=[<pager>]::
 	Open the matching files in the pager (not the output of 'grep').
 	If the pager happens to be "less" or "vi", and the user
 	specified only one pattern, the first file is positioned at
-- 
1.7.1.246.g398e5.dirty