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

From: Jeff King <peff@peff.net>
Date: Mar 4, 2015, 10:16 UTC
Message-ID: <20150304101606.GB15788@peff.net>
In-Reply-To: <1425045718-30696-1-git-send-email-pclouds@gmail.com>

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

Show 20 quoted lines

> 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 <pclouds@gmail.com>
> ---
>  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),

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