{"thread":{"id":"38666","subject":"[PATCH] grep: correct help string for --exclude-standard","startedAt":"2015-02-27T14:01:58Z","lastAt":"2015-03-04T11:46:57Z","messageCount":5,"participants":["Nguyễn Thái Ngọc Duy","Jeff King","Duy Nguyen"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"256743","messageId":"1425045718-30696-1-git-send-email-pclouds@gmail.com","threadId":"38666","inReplyTo":null,"subject":"[PATCH] grep: correct help string for --exclude-standard","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2015-02-27T14:01:58Z","receivedAt":"2015-02-27T14:01:58Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"The current help string is about --no-exclude-standard. But \"git grep -h\"\nwould show --exclude-standard instead. Flip the string. See 0a93fb8\n(grep: teach --untracked and --exclude-standard options - 2011-09-27)\nfor more info about these options.\n\nSigned-off-by: Nguyễn Thái Ngọc Duy \n---\n builtin/grep.c | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/builtin/grep.c b/builtin/grep.c\nindex 4063882..e77f7cf 100644\n--- a/builtin/grep.c\n+++ b/builtin/grep.c\n@@ -641,7 +641,7 @@ int cmd_grep(int argc, const char **argv, const char *prefix)\n \t\tOPT_BOOL(0, \"untracked\", &untracked,\n \t\t\tN_(\"search in both tracked and untracked files\")),\n \t\tOPT_SET_INT(0, \"exclude-standard\", &opt_exclude,\n-\t\t\t N_(\"search also in ignored files\"), 1),\n+\t\t\t N_(\"ignore files specified via '.gitignore'\"), 1),\n \t\tOPT_GROUP(\"\"),\n \t\tOPT_BOOL('v', \"invert-match\", &opt.invert,\n \t\t\tN_(\"show non-matching lines\")),\n-- \n2.3.0.rc1.137.g477eb31\n"},{"id":"256988","messageId":"20150304101606.GB15788@peff.net","threadId":"38666","inReplyTo":"1425045718-30696-1-git-send-email-pclouds@gmail.com","subject":"Re: [PATCH] grep: correct help string for --exclude-standard","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2015-03-04T10:16:06Z","receivedAt":"2015-03-04T10:16:06Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Feb 27, 2015 at 09:01:58PM +0700, Nguyễn Thái Ngọc Duy wrote:\n\n> The current help string is about --no-exclude-standard. But \"git grep -h\"\n> would show --exclude-standard instead. Flip the string. See 0a93fb8\n> (grep: teach --untracked and --exclude-standard options - 2011-09-27)\n> for more info about these options.\n> \n> Signed-off-by: Nguyễn Thái Ngọc Duy \n> ---\n> builtin/grep.c | 2 +-\n> 1 file changed, 1 insertion(+), 1 deletion(-)\n> \n> diff --git a/builtin/grep.c b/builtin/grep.c\n> index 4063882..e77f7cf 100644\n> --- a/builtin/grep.c\n> +++ b/builtin/grep.c\n> @@ -641,7 +641,7 @@ int cmd_grep(int argc, const char **argv, const char *prefix)\n> \t\tOPT_BOOL(0, \"untracked\", &untracked,\n> \t\t\tN_(\"search in both tracked and untracked files\")),\n> \t\tOPT_SET_INT(0, \"exclude-standard\", &opt_exclude,\n> -\t\t\t N_(\"search also in ignored files\"), 1),\n> +\t\t\t N_(\"ignore files specified via '.gitignore'\"), 1),\n\nHmm. If the default is \"--exclude-standard\", then what expect people to\nuse is \"--no-exclude-standard\". Would it make more sense to list that in\nthe \"-h\" output? Sadly I think to do that you have to manually specify\n\"--no-exclude-standard\" with OPT_NONEG, and then manually specify\n\"--exclude-standard\" in addition with OPT_HIDDEN.\n\nIt might be nice if parseopt had a PARSE_OPT_NEGHELP option or something\nto show the \"--no-\" form. Something like:\n\ndiff --git a/builtin/grep.c b/builtin/grep.c\nindex 9262b73..c03c3e7 100644\n--- a/builtin/grep.c\n+++ b/builtin/grep.c\n@@ -640,8 +640,10 @@ int cmd_grep(int argc, const char **argv, const char *prefix)\n \t\t\t N_(\"find in contents not managed by git\"), 1),\n \t\tOPT_BOOL(0, \"untracked\", &untracked,\n \t\t\tN_(\"search in both tracked and untracked files\")),\n-\t\tOPT_SET_INT(0, \"exclude-standard\", &opt_exclude,\n-\t\t\t N_(\"search also in ignored files\"), 1),\n+\t\t{ OPTION_SET_INT, 0, \"exclude-standard\", &opt_exclude,\n+\t\t NULL, N_(\"search also in ignored files\"),\n+\t\t PARSE_OPT_NOARG | PARSE_OPT_NEGHELP,\n+\t\t NULL, 1 },\n \t\tOPT_GROUP(\"\"),\n \t\tOPT_BOOL('v', \"invert-match\", &opt.invert,\n \t\t\tN_(\"show non-matching lines\")),\ndiff --git a/parse-options.c b/parse-options.c\nindex 80106c0..0ba7dc4 100644\n--- a/parse-options.c\n+++ b/parse-options.c\n@@ -599,8 +599,12 @@ static int usage_with_options_internal(struct parse_opt_ctx_t *ctx,\n \t\t}\n \t\tif (opts->long_name && opts->short_name)\n \t\t\tpos += fprintf(outfile, \", \");\n-\t\tif (opts->long_name)\n-\t\t\tpos += fprintf(outfile, \"--%s\", opts->long_name);\n+\t\tif (opts->long_name) {\n+\t\t\tint neg = opts->flags & PARSE_OPT_NEGHELP;\n+\t\t\tpos += fprintf(outfile, \"--%s%s\",\n+\t\t\t\t neg ? \"no-\" : \"\",\n+\t\t\t\t opts->long_name);\n+\t\t}\n \t\tif (opts->type == OPTION_NUMBER)\n \t\t\tpos += utf8_fprintf(outfile, _(\"-NUM\"));\n \ndiff --git a/parse-options.h b/parse-options.h\nindex 7940bc7..e688c32 100644\n--- a/parse-options.h\n+++ b/parse-options.h\n@@ -37,6 +37,7 @@ enum parse_opt_option_flags {\n \tPARSE_OPT_LASTARG_DEFAULT = 16,\n \tPARSE_OPT_NODASH = 32,\n \tPARSE_OPT_LITERAL_ARGHELP = 64,\n+\tPARSE_OPT_NEGHELP = 128,\n \tPARSE_OPT_SHELL_EVAL = 256\n };\n \n\nThough it is annoying that we have to give up the nice OPT_SET_INT macro\nto specify an extra flag.\n\n-Peff\n"},{"id":"256993","messageId":"CACsJy8AW4=tsFLDcJb=is5UUdVyyFPD+AHi1v-xX=qCXLL_N2g@mail.gmail.com","threadId":"38666","inReplyTo":"20150304101606.GB15788@peff.net","subject":"Re: [PATCH] grep: correct help string for --exclude-standard","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2015-03-04T11:13:32Z","receivedAt":"2015-03-04T11:13:32Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Wed, Mar 4, 2015 at 5:16 PM, Jeff King wrote:\n> Hmm. If the default is \"--exclude-standard\", then what expect people to\n> use is \"--no-exclude-standard\". Would it make more sense to list that in\n> the \"-h\" output?\n\nI thought about it and actually edited git-grep man page to clarify\nthe default, only to find out this. When --untracked is used,\n--exclude-standard is the default. When --no-index is used,\n--no-exclude-standard is the default. Can't have it both ways. This is\nalready mentioned with the subtle phrase \"only useful with...\".\n\n> It might be nice if parseopt had a PARSE_OPT_NEGHELP option or something\n> to show the \"--no-\" form.\n\nRegardless, yes it would be nice to have something like this. I think\nthere are places that can make use of this.\n-- \nDuy\n"},{"id":"256996","messageId":"20150304112603.GA26545@peff.net","threadId":"38666","inReplyTo":"CACsJy8AW4=tsFLDcJb=is5UUdVyyFPD+AHi1v-xX=qCXLL_N2g@mail.gmail.com","subject":"Re: [PATCH] grep: correct help string for --exclude-standard","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2015-03-04T11:26:03Z","receivedAt":"2015-03-04T11:26:03Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Mar 04, 2015 at 06:13:32PM +0700, Duy Nguyen wrote:\n\n> On Wed, Mar 4, 2015 at 5:16 PM, Jeff King wrote:\n> > Hmm. If the default is \"--exclude-standard\", then what expect people to\n> > use is \"--no-exclude-standard\". Would it make more sense to list that in\n> > the \"-h\" output?\n> \n> I thought about it and actually edited git-grep man page to clarify\n> the default, only to find out this. When --untracked is used,\n> --exclude-standard is the default. When --no-index is used,\n> --no-exclude-standard is the default. Can't have it both ways. This is\n> already mentioned with the subtle phrase \"only useful with...\".\n\nYuck. :)\n\nI agree your patch is the right thing to do, then.\n\n> > It might be nice if parseopt had a PARSE_OPT_NEGHELP option or something\n> > to show the \"--no-\" form.\n> \n> Regardless, yes it would be nice to have something like this. I think\n> there are places that can make use of this.\n\nGrepping around, it looks like the best form would be an OPT_NEGBOOL\nthat acts like a boolean but negates the truth value, and advertises the\nnegative form. We have a lot of:\n\n OPT_BOOL('n', \"no-checkout\", &option_no_checkout,\n N_(\"don't create a checkout\"))\n\nwhere countermanding an earlier \"--no-checkout\" has to be spelled as\n\"--no-no-checkout\", rather than \"--checkout\". If we could write:\n\n OPT_NEGBOOL('n', \"checkout\", ...)\n\nthat would be nicer. But the short option is a bit weird. We'd want:\n\n -n: option_no_checkout=true\n --checkout: option_no_checkout=false\n --no-checkout: option_no_checkout=true\n\nThat is, we flip the sense of the long option, but the short option\nstill yields \"true\". I think that would be useful, but it sure is weird\nto explain.\n\n-Peff\n"},{"id":"256997","messageId":"CACsJy8DVG7MZoJcSxkZX5kdE8tLT7HnNqDEins3a2aLThZbWPg@mail.gmail.com","threadId":"38666","inReplyTo":"20150304112603.GA26545@peff.net","subject":"Re: [PATCH] grep: correct help string for --exclude-standard","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2015-03-04T11:46:57Z","receivedAt":"2015-03-04T11:46:57Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Wed, Mar 4, 2015 at 6:26 PM, Jeff King wrote:\n>> > It might be nice if parseopt had a PARSE_OPT_NEGHELP option or something\n>> > to show the \"--no-\" form.\n>>\n>> Regardless, yes it would be nice to have something like this. I think\n>> there are places that can make use of this.\n>\n> Grepping around, it looks like the best form would be an OPT_NEGBOOL\n> that acts like a boolean but negates the truth value, and advertises the\n> negative form. We have a lot of:\n>\n> OPT_BOOL('n', \"no-checkout\", &option_no_checkout,\n> N_(\"don't create a checkout\"))\n>\n> where countermanding an earlier \"--no-checkout\" has to be spelled as\n> \"--no-no-checkout\", rather than \"--checkout\". If we could write:\n>\n> OPT_NEGBOOL('n', \"checkout\", ...)\n>\n> that would be nicer. But the short option is a bit weird. We'd want:\n>\n> -n: option_no_checkout=true\n> --checkout: option_no_checkout=false\n> --no-checkout: option_no_checkout=true\n>\n> That is, we flip the sense of the long option, but the short option\n> still yields \"true\". I think that would be useful, but it sure is weird\n> to explain.\n\nYeah it looks confusing.. How about leaving that first arg as the\nshort option \"checkout\" and move 'n' elsewhere? Something like this\n\nOPT_NEGBOOL(0, \"checkout\", 'n', ....)\n-- \nDuy\n"}]}