[PATCH] grep: correct help string for --exclude-standard

The current help string is about --no-exclude-standard. But "git grep -h"
would show --exclude-standard instead. Flip the string. See 0a93fb8
(grep: teach --untracked and --exclude-standard options - 2011-09-27)
for more info about these options.

Signed-off-by: Nguyễn Thái Ngọc Duy <redacted>
---
 builtin/grep.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/builtin/grep.c b/builtin/grep.c
index 4063882..e77f7cf 100644
--- a/builtin/grep.c
+++ b/builtin/grep.c

@@ -641,7 +641,7 @@ int cmd_grep(int argc, const char **argv, const char *prefix)
 		OPT_BOOL(0, "untracked", &untracked,
 			N_("search in both tracked and untracked files")),
 		OPT_SET_INT(0, "exclude-standard", &opt_exclude,
-			    N_("search also in ignored files"), 1),
+			    N_("ignore files specified via '.gitignore'"), 1),
 		OPT_GROUP(""),
 		OPT_BOOL('v', "invert-match", &opt.invert,
 			N_("show non-matching lines")),

-- 
2.3.0.rc1.137.g477eb31

On Fri, Feb 27, 2015 at 09:01:58PM +0700, Nguyễn Thái Ngọc Duy wrote:

Hmm. If the default is "--exclude-standard", then what expect people to
use is "--no-exclude-standard". Would it make more sense to list that in
the "-h" output? Sadly I think to do that you have to manually specify
"--no-exclude-standard" with OPT_NONEG, and then manually specify
"--exclude-standard" in addition with OPT_HIDDEN.

It might be nice if parseopt had a PARSE_OPT_NEGHELP option or something
to show the "--no-" form. Something like:

diff --git a/builtin/grep.c b/builtin/grep.c
index 9262b73..c03c3e7 100644
--- a/builtin/grep.c
+++ b/builtin/grep.c

@@ -640,8 +640,10 @@ int cmd_grep(int argc, const char **argv, const char *prefix)
 			 N_("find in contents not managed by git"), 1),
 		OPT_BOOL(0, "untracked", &untracked,
 			N_("search in both tracked and untracked files")),
-		OPT_SET_INT(0, "exclude-standard", &opt_exclude,
-			    N_("search also in ignored files"), 1),
+		{ OPTION_SET_INT, 0, "exclude-standard", &opt_exclude,
+		  NULL, N_("search also in ignored files"),
+		  PARSE_OPT_NOARG | PARSE_OPT_NEGHELP,
+		  NULL, 1 },
 		OPT_GROUP(""),
 		OPT_BOOL('v', "invert-match", &opt.invert,
 			N_("show non-matching lines")),

diff --git a/parse-options.c b/parse-options.c
index 80106c0..0ba7dc4 100644
--- a/parse-options.c
+++ b/parse-options.c

@@ -599,8 +599,12 @@ static int usage_with_options_internal(struct parse_opt_ctx_t *ctx,
 		}
 		if (opts->long_name && opts->short_name)
 			pos += fprintf(outfile, ", ");
-		if (opts->long_name)
-			pos += fprintf(outfile, "--%s", opts->long_name);
+		if (opts->long_name) {
+			int neg = opts->flags & PARSE_OPT_NEGHELP;
+			pos += fprintf(outfile, "--%s%s",
+				       neg ? "no-" : "",
+				       opts->long_name);
+		}
 		if (opts->type == OPTION_NUMBER)
 			pos += utf8_fprintf(outfile, _("-NUM"));
 

diff --git a/parse-options.h b/parse-options.h
index 7940bc7..e688c32 100644
--- a/parse-options.h
+++ b/parse-options.h

@@ -37,6 +37,7 @@ enum parse_opt_option_flags {
 	PARSE_OPT_LASTARG_DEFAULT = 16,
 	PARSE_OPT_NODASH = 32,
 	PARSE_OPT_LITERAL_ARGHELP = 64,
+	PARSE_OPT_NEGHELP = 128,
 	PARSE_OPT_SHELL_EVAL = 256
 };
 

Though it is annoying that we have to give up the nice OPT_SET_INT macro
to specify an extra flag.

-Peff

On Wed, Mar 4, 2015 at 5:16 PM, Jeff King [off-list ref] wrote:

Hmm. If the default is "--exclude-standard", then what expect people to
use is "--no-exclude-standard". Would it make more sense to list that in
the "-h" output?

I thought about it and actually edited git-grep man page to clarify
the default, only to find out this.  When --untracked is used,
--exclude-standard is the default. When --no-index is used,
--no-exclude-standard is the default. Can't have it both ways. This is
already mentioned with the subtle phrase "only useful with...".

It might be nice if parseopt had a PARSE_OPT_NEGHELP option or something
to show the "--no-" form.

Regardless, yes it would be nice to have something like this. I think
there are places that can make use of this.
-- 
Duy

On Wed, Mar 04, 2015 at 06:13:32PM +0700, Duy Nguyen wrote:

On Wed, Mar 4, 2015 at 5:16 PM, Jeff King [off-list ref] wrote:

quoted

Hmm. If the default is "--exclude-standard", then what expect people to
use is "--no-exclude-standard". Would it make more sense to list that in
the "-h" output?

I thought about it and actually edited git-grep man page to clarify
the default, only to find out this.  When --untracked is used,
--exclude-standard is the default. When --no-index is used,
--no-exclude-standard is the default. Can't have it both ways. This is
already mentioned with the subtle phrase "only useful with...".

Yuck. :)

I agree your patch is the right thing to do, then.

quoted

It might be nice if parseopt had a PARSE_OPT_NEGHELP option or something
to show the "--no-" form.

Regardless, yes it would be nice to have something like this. I think
there are places that can make use of this.

Grepping around, it looks like the best form would be an OPT_NEGBOOL
that acts like a boolean but negates the truth value, and advertises the
negative form. We have a lot of:

  OPT_BOOL('n', "no-checkout", &option_no_checkout,
           N_("don't create a checkout"))

where countermanding an earlier "--no-checkout" has to be spelled as
"--no-no-checkout", rather than "--checkout". If we could write:

  OPT_NEGBOOL('n', "checkout", ...)

that would be nicer. But the short option is a bit weird. We'd want:

  -n: option_no_checkout=true
  --checkout: option_no_checkout=false
  --no-checkout: option_no_checkout=true

That is, we flip the sense of the long option, but the short option
still yields "true". I think that would be useful, but it sure is weird
to explain.

-Peff

On Wed, Mar 4, 2015 at 6:26 PM, Jeff King [off-list ref] wrote:

quoted

quoted

It might be nice if parseopt had a PARSE_OPT_NEGHELP option or something
to show the "--no-" form.

Regardless, yes it would be nice to have something like this. I think
there are places that can make use of this.

Grepping around, it looks like the best form would be an OPT_NEGBOOL
that acts like a boolean but negates the truth value, and advertises the
negative form. We have a lot of:

  OPT_BOOL('n', "no-checkout", &option_no_checkout,
           N_("don't create a checkout"))

where countermanding an earlier "--no-checkout" has to be spelled as
"--no-no-checkout", rather than "--checkout". If we could write:

  OPT_NEGBOOL('n', "checkout", ...)

that would be nicer. But the short option is a bit weird. We'd want:

  -n: option_no_checkout=true
  --checkout: option_no_checkout=false
  --no-checkout: option_no_checkout=true

That is, we flip the sense of the long option, but the short option
still yields "true". I think that would be useful, but it sure is weird
to explain.

Yeah it looks confusing.. How about leaving that first arg as the
short option "checkout" and move 'n' elsewhere? Something like this

OPT_NEGBOOL(0, "checkout", 'n', ....)
-- 
Duy