{"thread":{"id":"41715","subject":"parse-options does not recognize \"unspecified\" behavior","startedAt":"2016-03-16T19:51:49Z","lastAt":"2016-03-25T15:03:06Z","messageCount":18,"participants":["Pranit Bauva","Jeff King","Eric Sunshine","Stefan Beller","SZEDER Gábor"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"280954","messageId":"CAFZEwPPd2wFqFq2LFEzN2CzhTV6C420SLPcXi1SWE=z2epOYLw@mail.gmail.com","threadId":"41715","inReplyTo":null,"subject":"parse-options does not recognize \"unspecified\" behavior","fromName":"Pranit Bauva","fromEmail":"pranit.bauva@gmail.com","sentAt":"2016-03-16T19:51:49Z","receivedAt":"2016-03-16T19:51:49Z","isPatch":false,"sender":{"key":"pranit.bauva@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2959938?v=4"},"body":"Hey!\n\nI noticed that parse-options does not recognize the variable which is\nset to -1 so as to denote the \"unspecified\" value.\n\nI did the following changes in builtin/commit.c (in master branch not\nthe patch I am working on) :\n - static int verbose = -1\n - introduced a printf statement after parsing the options to print\nthe value of verbose.\n\nWhen I ran `git commit` :\n I get the output that verbose is set to -1.\n\nWhen I ran `git commit -v` :\nI get the output that verbose is set to 0.\n\nWhen I ran `git commit -v -v` :\nI get the output that verbose is set to 1.\n\nWhen I ran `git commit --no-verbose` :\nI get the out that verbose is set to 0.\n\nThen I set verbose = -2.\nAnd the output was obtained in a similar format.\n -2 with `git commit`\n -1 with `git commit -v`\n 0 with `git commit -v -v`\n 0 with `git commit --no-verbose`\n 1 with `git commit -v -v -v`\n\nIt seems that parse-options just increments the value without\nconsidering the -1 flag to denote \"unspecified value\".\n\nIs this a bug?\n"},{"id":"280960","messageId":"20160316204912.GA1890@sigill.intra.peff.net","threadId":"41715","inReplyTo":"CAFZEwPPd2wFqFq2LFEzN2CzhTV6C420SLPcXi1SWE=z2epOYLw@mail.gmail.com","subject":"Re: parse-options does not recognize \"unspecified\" behavior","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-03-16T20:49:13Z","receivedAt":"2016-03-16T20:49:13Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Mar 17, 2016 at 01:21:49AM +0530, Pranit Bauva wrote:\n\n> I noticed that parse-options does not recognize the variable which is\n> set to -1 so as to denote the \"unspecified\" value.\n\nRight. Like all of the stock parse-options handlers, it does not ever\nread or understand the value passed to it by the caller. It only\nincrements or decrements.\n\n> I did the following changes in builtin/commit.c (in master branch not\n> the patch I am working on) :\n> - static int verbose = -1\n> - introduced a printf statement after parsing the options to print\n> the value of verbose.\n> \n> When I ran `git commit` :\n> I get the output that verbose is set to -1.\n> \n> When I ran `git commit -v` :\n> I get the output that verbose is set to 0.\n> \n> When I ran `git commit -v -v` :\n> I get the output that verbose is set to 1.\n> \n> When I ran `git commit --no-verbose` :\n> I get the out that verbose is set to 0.\n> [...]\n> It seems that parse-options just increments the value without\n> considering the -1 flag to denote \"unspecified value\".\n> \n> Is this a bug?\n\nNot in parse-options, though I think setting verbose to \"-1\" in the\nfirst place is wrong.\n\nIn general, parse-options does not know or care about the default values\nthat callers assign to variables; it just writes to them based on the\noption-type specified by the caller. So the behavior for \"commit\",\n\"commit -v\", and \"commit -v -v\" you show are perfectly reasonable.\n\nBut the one for \"--no-verbose\" is wrong. Parse-options has to write some\n\"reset\" value, and it does not know what the initial default was. So it\nwrites 0. This is the same for options like OPT_SET_INT, and similar for\nstring options (where we set it to NULL).\n\nSo I think the caller choosing \"-1\" here as the \"not set\" value is the\nbug.\n\n-Peff\n"},{"id":"280962","messageId":"CAFZEwPMa3GZS6pvFwr8PLVDqKm5xmMd307nbjhpZSC_ndpw8vw@mail.gmail.com","threadId":"41715","inReplyTo":"20160316204912.GA1890@sigill.intra.peff.net","subject":"Re: parse-options does not recognize \"unspecified\" behavior","fromName":"Pranit Bauva","fromEmail":"pranit.bauva@gmail.com","sentAt":"2016-03-16T21:06:51Z","receivedAt":"2016-03-16T21:06:51Z","isPatch":false,"sender":{"key":"pranit.bauva@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2959938?v=4"},"body":"On Thu, Mar 17, 2016 at 2:19 AM, Jeff King wrote:\n> On Thu, Mar 17, 2016 at 01:21:49AM +0530, Pranit Bauva wrote:\n>\n>> I noticed that parse-options does not recognize the variable which is\n>> set to -1 so as to denote the \"unspecified\" value.\n>\n> Right. Like all of the stock parse-options handlers, it does not ever\n> read or understand the value passed to it by the caller. It only\n> increments or decrements.\n>\n>> I did the following changes in builtin/commit.c (in master branch not\n>> the patch I am working on) :\n>> - static int verbose = -1\n>> - introduced a printf statement after parsing the options to print\n>> the value of verbose.\n>>\n>> When I ran `git commit` :\n>> I get the output that verbose is set to -1.\n>>\n>> When I ran `git commit -v` :\n>> I get the output that verbose is set to 0.\n>>\n>> When I ran `git commit -v -v` :\n>> I get the output that verbose is set to 1.\n>>\n>> When I ran `git commit --no-verbose` :\n>> I get the out that verbose is set to 0.\n>> [...]\n>> It seems that parse-options just increments the value without\n>> considering the -1 flag to denote \"unspecified value\".\n>>\n>> Is this a bug?\n>\n> Not in parse-options, though I think setting verbose to \"-1\" in the\n> first place is wrong.\n>\n> In general, parse-options does not know or care about the default values\n> that callers assign to variables; it just writes to them based on the\n> option-type specified by the caller. So the behavior for \"commit\",\n> \"commit -v\", and \"commit -v -v\" you show are perfectly reasonable.\n>\n> But the one for \"--no-verbose\" is wrong. Parse-options has to write some\n> \"reset\" value, and it does not know what the initial default was. So it\n> writes 0. This is the same for options like OPT_SET_INT, and similar for\n> string options (where we set it to NULL).\n>\n> So I think the caller choosing \"-1\" here as the \"not set\" value is the\n> bug.\n>\n> -Peff\n\nI agree to you on the point that parse-options should not care about\nthe value passed to it. But I think plainly incrementing the value of\nthe variable is not a very nice way. I have an another approach to it.\nThe parse-options will first store a temporary structure. If there is\nsome changes (not the \"--no-\" ones) then it sets the respective\nvariable in temporary structure to the set value. If \"--no-\" is passed\nthen it writes the \"reset\" value to the respective variable in\ntemporary structure. If nothing about that options is specified then\nit copies the respective variable from original to temporary. After\ncompleting the entire process, it can copy temporary structure to the\noriginal structure.\n\nWhat are your opinions about this?\n"},{"id":"280969","messageId":"20160316212308.GA4538@sigill.intra.peff.net","threadId":"41715","inReplyTo":"CAFZEwPMa3GZS6pvFwr8PLVDqKm5xmMd307nbjhpZSC_ndpw8vw@mail.gmail.com","subject":"Re: parse-options does not recognize \"unspecified\" behavior","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-03-16T21:23:09Z","receivedAt":"2016-03-16T21:23:09Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Mar 17, 2016 at 02:36:51AM +0530, Pranit Bauva wrote:\n\n> > So I think the caller choosing \"-1\" here as the \"not set\" value is the\n> > bug.\n> >\n> > -Peff\n> \n> I agree to you on the point that parse-options should not care about\n> the value passed to it. But I think plainly incrementing the value of\n> the variable is not a very nice way. I have an another approach to it.\n> The parse-options will first store a temporary structure. If there is\n> some changes (not the \"--no-\" ones) then it sets the respective\n> variable in temporary structure to the set value. If \"--no-\" is passed\n> then it writes the \"reset\" value to the respective variable in\n> temporary structure. If nothing about that options is specified then\n> it copies the respective variable from original to temporary. After\n> completing the entire process, it can copy temporary structure to the\n> original structure.\n> \n> What are your opinions about this?\n\nI don't think that would produce the wrong behavior, but it seems like a\nvery complicated solution to a problem that can easily be solved by just\nfollowing the usual conventions (that verbose starts at 0, options make\nit go up or down, and \"--no-\" resets it to zero).\n\nPerhaps it would make more sense if I understood what your goal was in\nsetting verbose to -1 in the first place.\n\n-Peff\n"},{"id":"280971","messageId":"CAPig+cRKyaUefz0qj6unkaiPg25=Xi2WorQE4Fm46CCf00UbHQ@mail.gmail.com","threadId":"41715","inReplyTo":"20160316212308.GA4538@sigill.intra.peff.net","subject":"Re: parse-options does not recognize \"unspecified\" behavior","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2016-03-16T21:37:03Z","receivedAt":"2016-03-16T21:37:03Z","isPatch":false,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Wed, Mar 16, 2016 at 5:23 PM, Jeff King wrote:\n> On Thu, Mar 17, 2016 at 02:36:51AM +0530, Pranit Bauva wrote:\n>> I agree to you on the point that parse-options should not care about\n>> the value passed to it. But I think plainly incrementing the value of\n>> the variable is not a very nice way. I have an another approach to it.\n>> The parse-options will first store a temporary structure. If there is\n>> some changes (not the \"--no-\" ones) then it sets the respective\n>> variable in temporary structure to the set value. If \"--no-\" is passed\n>> then it writes the \"reset\" value to the respective variable in\n>> temporary structure. If nothing about that options is specified then\n>> it copies the respective variable from original to temporary. After\n>> completing the entire process, it can copy temporary structure to the\n>> original structure.\n>>\n>> What are your opinions about this?\n>\n> I don't think that would produce the wrong behavior, but it seems like a\n> very complicated solution to a problem that can easily be solved by just\n> following the usual conventions (that verbose starts at 0, options make\n> it go up or down, and \"--no-\" resets it to zero).\n\nI agree that this is overly complicated.\n\n> Perhaps it would make more sense if I understood what your goal was in\n> setting verbose to -1 in the first place.\n\nThe goal comes from his GSoC microproject. Specifically, Pranit wants\nan \"unspecified\" value. The reason is that he is adding a\ncommit.verbose= config variable to back the existing git-commit\n--verbose option. Any use of --verbose (one or more times) or\n--no-verbose should override the config.verbose value altogether, so\nhe wants a way to know if --verbose or --no-verbose was used; hence\nthe \"unspecified\" value. And, really, this issue isn't necessarily\nspecific to git-commit. It could apply to any command that understands\nverbosity levels and wants to be able to get them from both a config\nvariable and a command-line option.\n\nA much easier solution would be to update OPT_VERBOSE() to understand\nthat negative values are \"unspecified\", and then --verbose would\n(pseudocode):\n\n if (value < 0)\n value = 0\n value++;\n\nand --no-verbose would:\n\n value = 0\n\nThat should be compatible with existing clients of OPT__VERBOSE()\nwhich initialize the value to 0, and should satisfy Pranit's case; he\ncan initialize it to -1, and if it is still -1 when option parsing is\ndone, then he knows that neither --verbose nor --no-verbose was seen.\n"},{"id":"280972","messageId":"CAFZEwPOySiVwLJOw0seRu+c-NTK_vnsdi3Gsvv_tpJt6KY=Chg@mail.gmail.com","threadId":"41715","inReplyTo":"20160316212308.GA4538@sigill.intra.peff.net","subject":"Re: parse-options does not recognize \"unspecified\" behavior","fromName":"Pranit Bauva","fromEmail":"pranit.bauva@gmail.com","sentAt":"2016-03-16T21:37:36Z","receivedAt":"2016-03-16T21:37:36Z","isPatch":false,"sender":{"key":"pranit.bauva@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2959938?v=4"},"body":"On Thu, Mar 17, 2016 at 2:53 AM, Jeff King wrote:\n\n> I don't think that would produce the wrong behavior, but it seems like a\n> very complicated solution to a problem that can easily be solved by just\n> following the usual conventions (that verbose starts at 0, options make\n> it go up or down, and \"--no-\" resets it to zero).\n>\n> Perhaps it would make more sense if I understood what your goal was in\n> setting verbose to -1 in the first place.\n>\n> -Peff\n\nI am working on the micro project \"configuration for commonly used\ncommand line options like \"git commit -v\".\nI sent out a final patch without considering the multiple verbosity\nbehavior of `git commit`. To take consideration for that behavior,\nJunio suggested this approach [1]. I was trying to write code for this\nwhen I noticed this behavior. I did a `git grep \" unspecified \" just\nto find that using -1 is not that common. I think this problem can\nalso arise in future if some more options are included. The question\nstill remains whether its worth all this effort?\n\n[1] : http://thread.gmane.org/gmane.comp.version-control.git/288820/focus=288909\n"},{"id":"280974","messageId":"CAFZEwPMGBKCfEoo2ofCak25jDMRwWwq=1HmgqSyPEpJWhOL_6g@mail.gmail.com","threadId":"41715","inReplyTo":"CAPig+cRKyaUefz0qj6unkaiPg25=Xi2WorQE4Fm46CCf00UbHQ@mail.gmail.com","subject":"Re: parse-options does not recognize \"unspecified\" behavior","fromName":"Pranit Bauva","fromEmail":"pranit.bauva@gmail.com","sentAt":"2016-03-16T21:41:43Z","receivedAt":"2016-03-16T21:41:43Z","isPatch":false,"sender":{"key":"pranit.bauva@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2959938?v=4"},"body":"On Thu, Mar 17, 2016 at 3:07 AM, Eric Sunshine wrote:\n\n> The goal comes from his GSoC microproject. Specifically, Pranit wants\n> an \"unspecified\" value. The reason is that he is adding a\n> commit.verbose= config variable to back the existing git-commit\n> --verbose option. Any use of --verbose (one or more times) or\n> --no-verbose should override the config.verbose value altogether, so\n> he wants a way to know if --verbose or --no-verbose was used; hence\n> the \"unspecified\" value. And, really, this issue isn't necessarily\n> specific to git-commit. It could apply to any command that understands\n> verbosity levels and wants to be able to get them from both a config\n> variable and a command-line option.\n>\n> A much easier solution would be to update OPT_VERBOSE() to understand\n> that negative values are \"unspecified\", and then --verbose would\n> (pseudocode):\n>\n> if (value < 0)\n> value = 0\n> value++;\n>\n> and --no-verbose would:\n>\n> value = 0\n>\n> That should be compatible with existing clients of OPT__VERBOSE()\n> which initialize the value to 0, and should satisfy Pranit's case; he\n> can initialize it to -1, and if it is still -1 when option parsing is\n> done, then he knows that neither --verbose nor --no-verbose was seen.\n\nThis is a much easier solution. I didn't think of this. This type of\nproblem can only arise with only verbose, so it would be better to\nspecifically change that part of code.\n"},{"id":"280976","messageId":"20160316214442.GC4441@sigill.intra.peff.net","threadId":"41715","inReplyTo":"CAPig+cRKyaUefz0qj6unkaiPg25=Xi2WorQE4Fm46CCf00UbHQ@mail.gmail.com","subject":"Re: parse-options does not recognize \"unspecified\" behavior","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-03-16T21:44:42Z","receivedAt":"2016-03-16T21:44:42Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Mar 16, 2016 at 05:37:03PM -0400, Eric Sunshine wrote:\n\n> A much easier solution would be to update OPT_VERBOSE() to understand\n> that negative values are \"unspecified\", and then --verbose would\n> (pseudocode):\n> \n> if (value < 0)\n> value = 0\n> value++;\n> \n> and --no-verbose would:\n> \n> value = 0\n> \n> That should be compatible with existing clients of OPT__VERBOSE()\n> which initialize the value to 0, and should satisfy Pranit's case; he\n> can initialize it to -1, and if it is still -1 when option parsing is\n> done, then he knows that neither --verbose nor --no-verbose was seen.\n\nYes, that makes much more sense to me. Thanks for the back-story.\n\n-Peff\n"},{"id":"280977","messageId":"CAGZ79kbbAv=PukD+sftmoO8u3GX=S1YCYGV8zcNMxrZ+E41-UA@mail.gmail.com","threadId":"41715","inReplyTo":"20160316214442.GC4441@sigill.intra.peff.net","subject":"Re: parse-options does not recognize \"unspecified\" behavior","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2016-03-16T21:53:17Z","receivedAt":"2016-03-16T21:53:17Z","isPatch":false,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Wed, Mar 16, 2016 at 2:44 PM, Jeff King wrote:\n> On Wed, Mar 16, 2016 at 05:37:03PM -0400, Eric Sunshine wrote:\n>\n>> A much easier solution would be to update OPT_VERBOSE() to understand\n>> that negative values are \"unspecified\", and then --verbose would\n>> (pseudocode):\n>>\n>> if (value < 0)\n>> value = 0\n>> value++;\n>>\n>> and --no-verbose would:\n>>\n>> value = 0\n>>\n>> That should be compatible with existing clients of OPT__VERBOSE()\n>> which initialize the value to 0, and should satisfy Pranit's case; he\n>> can initialize it to -1, and if it is still -1 when option parsing is\n>> done, then he knows that neither --verbose nor --no-verbose was seen.\n>\n> Yes, that makes much more sense to me. Thanks for the back-story.\n>\n> -Peff\n\nIs there any command which needs more than one --no-verbose?\n(as an abuse to stacking --quiet multiple times)?\n"},{"id":"280985","messageId":"20160316231626.GA11808@sigill.intra.peff.net","threadId":"41715","inReplyTo":"CAGZ79kbbAv=PukD+sftmoO8u3GX=S1YCYGV8zcNMxrZ+E41-UA@mail.gmail.com","subject":"Re: parse-options does not recognize \"unspecified\" behavior","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-03-16T23:16:26Z","receivedAt":"2016-03-16T23:16:26Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Mar 16, 2016 at 02:53:17PM -0700, Stefan Beller wrote:\n\n> On Wed, Mar 16, 2016 at 2:44 PM, Jeff King wrote:\n> > On Wed, Mar 16, 2016 at 05:37:03PM -0400, Eric Sunshine wrote:\n> >\n> >> A much easier solution would be to update OPT_VERBOSE() to understand\n> >> that negative values are \"unspecified\", and then --verbose would\n> >> (pseudocode):\n> >>\n> >> if (value < 0)\n> >> value = 0\n> >> value++;\n> >>\n> >> and --no-verbose would:\n> >>\n> >> value = 0\n> >>\n> >> That should be compatible with existing clients of OPT__VERBOSE()\n> >> which initialize the value to 0, and should satisfy Pranit's case; he\n> >> can initialize it to -1, and if it is still -1 when option parsing is\n> >> done, then he knows that neither --verbose nor --no-verbose was seen.\n> >\n> > Yes, that makes much more sense to me. Thanks for the back-story.\n> \n> Is there any command which needs more than one --no-verbose?\n> (as an abuse to stacking --quiet multiple times)?\n\nI'm not sure I understand. \"--no-verbose\" is just about resetting the\nvalue. So you might get it multiple times in:\n\n git commit -v --no-verbose -v --no-verbose\n\nbut the caller would not care. Which makes me think I'm misunderstanding\nyour question.\n\nYou also mention \"--quiet\", but that is not handled by OPT__VERBOSE, but\nrather by OPT__QUIET. And there I do not think the \"-1 is undefined\"\ntrick works as well there, because presumably \"-1\" is the same as one\n\"--quiet\".\n\n-Peff\n"},{"id":"280987","messageId":"CAGZ79kYVT0cq8XgruZ+i_gGuSDySE+s3POU95PXyra9DwVGLkQ@mail.gmail.com","threadId":"41715","inReplyTo":"20160316231626.GA11808@sigill.intra.peff.net","subject":"Re: parse-options does not recognize \"unspecified\" behavior","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2016-03-16T23:33:03Z","receivedAt":"2016-03-16T23:33:03Z","isPatch":false,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Wed, Mar 16, 2016 at 4:16 PM, Jeff King wrote:\n> On Wed, Mar 16, 2016 at 02:53:17PM -0700, Stefan Beller wrote:\n>\n>> On Wed, Mar 16, 2016 at 2:44 PM, Jeff King wrote:\n>> > On Wed, Mar 16, 2016 at 05:37:03PM -0400, Eric Sunshine wrote:\n>> >\n>> >> A much easier solution would be to update OPT_VERBOSE() to understand\n>> >> that negative values are \"unspecified\", and then --verbose would\n>> >> (pseudocode):\n>> >>\n>> >> if (value < 0)\n>> >> value = 0\n>> >> value++;\n>> >>\n>> >> and --no-verbose would:\n>> >>\n>> >> value = 0\n>> >>\n>> >> That should be compatible with existing clients of OPT__VERBOSE()\n>> >> which initialize the value to 0, and should satisfy Pranit's case; he\n>> >> can initialize it to -1, and if it is still -1 when option parsing is\n>> >> done, then he knows that neither --verbose nor --no-verbose was seen.\n>> >\n>> > Yes, that makes much more sense to me. Thanks for the back-story.\n>>\n>> Is there any command which needs more than one --no-verbose?\n>> (as an abuse to stacking --quiet multiple times)?\n>\n> I'm not sure I understand. \"--no-verbose\" is just about resetting the\n> value. So you might get it multiple times in:\n>\n> git commit -v --no-verbose -v --no-verbose\n>\n> but the caller would not care. Which makes me think I'm misunderstanding\n> your question.\n>\n> You also mention \"--quiet\", but that is not handled by OPT__VERBOSE, but\n> rather by OPT__QUIET. And there I do not think the \"-1 is undefined\"\n> trick works as well there, because presumably \"-1\" is the same as one\n> \"--quiet\".\n\nThe way I understand verbosity is this:\n* You can instruct a command to be more verbose if it is supported by\nadding more -v\n* --no-verbose tells the command to be not verbose, i.e. no additional output.\n\nSo my question was, if there is any command, which is verbose by\ndefault, and --no-verbose\nwould make it \"more quiet\"? Such a case would be a UX bug, as a user\nwould rather expect\n--quiet instead of --no-verbose IMO. Would such a case ever happen,\nthat we'd want to give\n--no-verbose to decrease the amount said by the command?\n\nIIRC some commands use one integer variable to determine\nthe amount of output, i.e. --verbose increases that variable, --quiet\ndecreases it.\nWhat happens for example with\n\n git commit -v --no-verbose -v -q -q --no-quiet\n\nIn case of commit, the quietness and verbosity is done in 2 variables,\nso these are orthogonal to each other, there are even no internal checks for\n(verbosity > 0 && quietness > 0) at the same time, so it seems to be a valid\ncommand.\n\nIn case of a command where this is tracked in one variable,\n\n git -v --no-verbose -v -q -q --no-quiet\n\nyou'd expect:\n\n verbosity++ // because of first -v\n verbosity = 0 // because of the reset with --no-verbose\n verbosity++ // because of second -v\n verbosity-- // because of first -q\n verbosity-- // because of second -q\n verbosity = 0 // because of the reset with --no-quiet\n\nHaving typed that, I think my comment was not adding value to\nthe discussion, as --no-{verbose/quiet} would just reset it to 0 no matter\nif you track verbosity/quietness in one or two variables internally.\n\nStefan\n"},{"id":"280995","messageId":"20160317014310.GA12830@sigill.intra.peff.net","threadId":"41715","inReplyTo":"CAGZ79kYVT0cq8XgruZ+i_gGuSDySE+s3POU95PXyra9DwVGLkQ@mail.gmail.com","subject":"Re: parse-options does not recognize \"unspecified\" behavior","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-03-17T01:43:10Z","receivedAt":"2016-03-17T01:43:10Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Mar 16, 2016 at 04:33:03PM -0700, Stefan Beller wrote:\n\n> The way I understand verbosity is this:\n> * You can instruct a command to be more verbose if it is supported by\n> adding more -v\n> * --no-verbose tells the command to be not verbose, i.e. no additional output.\n> \n> So my question was, if there is any command, which is verbose by\n> default, and --no-verbose would make it \"more quiet\"? Such a case\n> would be a UX bug, as a user would rather expect --quiet instead of\n> --no-verbose IMO. Would such a case ever happen, that we'd want to\n> give --no-verbose to decrease the amount said by the command?\n\nAh, I see. I agree that would be a bug, because --no-verbose is not\n\"more quiet\". It is \"cancel all previous -v\". The right way to spell\nthat is \"--quiet\" (usually, see below).\n\n> IIRC some commands use one integer variable to determine\n> the amount of output, i.e. --verbose increases that variable, --quiet\n> decreases it.\n> What happens for example with\n> \n> git commit -v --no-verbose -v -q -q --no-quiet\n> \n> In case of commit, the quietness and verbosity is done in 2 variables,\n> so these are orthogonal to each other, there are even no internal checks for\n> (verbosity > 0 && quietness > 0) at the same time, so it seems to be a valid\n> command.\n\nYes, I think in general, \"-v\" and \"-q\" should work as opposites. But\nthat is not the case with commit, where \"-v\" and \"-q\" operate on totally\nseparate messages. I think that is a UX mistake, and we would not do\nit that way if designing from scratch. But we're stuck with it for\nhistorical reasons (I'd probably name \"--verbose\" as \"--show-diff\" or\nsomething if writing it today).\n\nArguably cmd_commit() should be using OPT_BOOL instead of OPT__VERBOSE,\nas there is no such thing as \"verbose > 1\" here. But I don't think there\nis any real user-facing consequence of that (however, given Eric's\nsuggestion, I suspect it would make Pranit's problem just go away, as it\nassigns rather than increments; IOW, it does the thing Eric was\nsuggestion OPT__VERBOSE to do).\n\n> In case of a command where this is tracked in one variable,\n> \n> git -v --no-verbose -v -q -q --no-quiet\n> \n> you'd expect:\n> \n> verbosity++ // because of first -v\n> verbosity = 0 // because of the reset with --no-verbose\n> verbosity++ // because of second -v\n> verbosity-- // because of first -q\n> verbosity-- // because of second -q\n> verbosity = 0 // because of the reset with --no-quiet\n> \n> Having typed that, I think my comment was not adding value to\n> the discussion, as --no-{verbose/quiet} would just reset it to 0 no matter\n> if you track verbosity/quietness in one or two variables internally.\n\nRight, in a command using OPT_VERBOSITY(), that is how it should (and\ndoes) work. I think \"commit\" is just funny for historical reasons.\n\n-Peff\n"},{"id":"281000","messageId":"CAPig+cSmQpt3u+1PPFD6CP9TF58R6NR=KCgC1dik9U=deaeCkA@mail.gmail.com","threadId":"41715","inReplyTo":"20160317014310.GA12830@sigill.intra.peff.net","subject":"Re: parse-options does not recognize \"unspecified\" behavior","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2016-03-17T05:32:41Z","receivedAt":"2016-03-17T05:32:41Z","isPatch":false,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Wed, Mar 16, 2016 at 9:43 PM, Jeff King wrote:\n> Arguably cmd_commit() should be using OPT_BOOL instead of OPT__VERBOSE,\n> as there is no such thing as \"verbose > 1\" here. But I don't think there\n> is any real user-facing consequence of that (however, given Eric's\n> suggestion, I suspect it would make Pranit's problem just go away, as it\n> assigns rather than increments; IOW, it does the thing Eric was\n> suggestion OPT__VERBOSE to do).\n\nActually, Pranit's previous version of the patch did treat verbosity\nas a boolean, but then SZEDER pointed out this bit from\ngit-commit.txt:\n\n --verbose::\n ...\n If specified twice, show in addition the unified diff between\n what would be committed and the worktree files, i.e. the unstaged\n changes to tracked files.\n\nwhich is what led us to the current discussion about wanting an\n\"unspecified\" value for OPT__VERBOSE.\n"},{"id":"281002","messageId":"20160317054047.GA3457@sigill.intra.peff.net","threadId":"41715","inReplyTo":"CAPig+cSmQpt3u+1PPFD6CP9TF58R6NR=KCgC1dik9U=deaeCkA@mail.gmail.com","subject":"Re: parse-options does not recognize \"unspecified\" behavior","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-03-17T05:40:47Z","receivedAt":"2016-03-17T05:40:47Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Mar 17, 2016 at 01:32:41AM -0400, Eric Sunshine wrote:\n\n> On Wed, Mar 16, 2016 at 9:43 PM, Jeff King wrote:\n> > Arguably cmd_commit() should be using OPT_BOOL instead of OPT__VERBOSE,\n> > as there is no such thing as \"verbose > 1\" here. But I don't think there\n> > is any real user-facing consequence of that (however, given Eric's\n> > suggestion, I suspect it would make Pranit's problem just go away, as it\n> > assigns rather than increments; IOW, it does the thing Eric was\n> > suggestion OPT__VERBOSE to do).\n> \n> Actually, Pranit's previous version of the patch did treat verbosity\n> as a boolean, but then SZEDER pointed out this bit from\n> git-commit.txt:\n> \n> --verbose::\n> ...\n> If specified twice, show in addition the unified diff between\n> what would be committed and the worktree files, i.e. the unstaged\n> changes to tracked files.\n> \n> which is what led us to the current discussion about wanting an\n> \"unspecified\" value for OPT__VERBOSE.\n\nAh, thanks. I looked for something like that in builtin/commit.c and\ndidn't see us using verbose as anything but a boolean. But we pass it\ninto wt_status, which does look at \"s->verbose > 1\".\n\nSorry for the noise (and probably I should stop participating in this\ndiscussion without having read all of the backstory!).\n\n-Peff\n"},{"id":"281227","messageId":"1458386735-8038-1-git-send-email-szeder@ira.uka.de","threadId":"41715","inReplyTo":"20160317014310.GA12830@sigill.intra.peff.net","subject":"Re: parse-options does not recognize \"unspecified\" behavior","fromName":"SZEDER Gábor","fromEmail":"szeder@ira.uka.de","sentAt":"2016-03-19T11:25:35Z","receivedAt":"2016-03-19T11:25:35Z","isPatch":false,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"> Yes, I think in general, \"-v\" and \"-q\" should work as opposites. But\n> that is not the case with commit, where \"-v\" and \"-q\" operate on totally\n> separate messages. I think that is a UX mistake, and we would not do\n> it that way if designing from scratch. But we're stuck with it for\n> historical reasons (I'd probably name \"--verbose\" as \"--show-diff\" or\n> something if writing it today).\n\nYeah, as a long-time 'git commit -v' user I never really thought about\nthe name of the option, but when I pointed out the multiple verbose\nlevels it struck me as a rather unfortunate name for this feature,\ntoo. Oh, well, we are stuck with it indeed.\n\nHowever, that doesn't mean that we have to spread this badly chosen\nname from options to config variables, does it? I think that if we\nare going to define a new config variable today, then it should be\nnamed properly, and it's better not to call it 'commit.verbose', but\n'commit.showDiff' or something. Perhaps we could even define two new\nconfig variables: 'commit.showDiff' for the diff of the changes to be\ncommitted (= '-v'), and 'commit.showUnstagedDiff' for the - wait for\nit! - unstaged changes (= '-v -v'). Not sure about the variable\nnames, though, because \"plain\" 'git diff' shows unstaged changes,\nwhile 'git diff --cached' shows staged changes.\n\nFurthermore, it doesn't mean that we can't add properly named command\nline option(s) and state that '-v|--verbose' is a synonym to\n'--show-diff' (maybe even deprecate '--verbose'), but I don't want to\nsqueeze even more into a GSOC micro project.\n\n\nGábor\n"},{"id":"281238","messageId":"CAFZEwPNK+XTjXzjn-Pv3iP4XiajXrdhUHWLz-8c0Ssy_Edkd7A@mail.gmail.com","threadId":"41715","inReplyTo":"1458386735-8038-1-git-send-email-szeder@ira.uka.de","subject":"Re: parse-options does not recognize \"unspecified\" behavior","fromName":"Pranit Bauva","fromEmail":"pranit.bauva@gmail.com","sentAt":"2016-03-19T16:55:35Z","receivedAt":"2016-03-19T16:55:35Z","isPatch":false,"sender":{"key":"pranit.bauva@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2959938?v=4"},"body":"On Sat, Mar 19, 2016 at 4:55 PM, SZEDER Gábor wrote:\n>> Yes, I think in general, \"-v\" and \"-q\" should work as opposites. But\n>> that is not the case with commit, where \"-v\" and \"-q\" operate on totally\n>> separate messages. I think that is a UX mistake, and we would not do\n>> it that way if designing from scratch. But we're stuck with it for\n>> historical reasons (I'd probably name \"--verbose\" as \"--show-diff\" or\n>> something if writing it today).\n>\n> Yeah, as a long-time 'git commit -v' user I never really thought about\n> the name of the option, but when I pointed out the multiple verbose\n> levels it struck me as a rather unfortunate name for this feature,\n> too. Oh, well, we are stuck with it indeed.\n>\n> However, that doesn't mean that we have to spread this badly chosen\n> name from options to config variables, does it? I think that if we\n> are going to define a new config variable today, then it should be\n> named properly, and it's better not to call it 'commit.verbose', but\n> 'commit.showDiff' or something. Perhaps we could even define two new\n> config variables: 'commit.showDiff' for the diff of the changes to be\n> committed (= '-v'), and 'commit.showUnstagedDiff' for the - wait for\n> it! - unstaged changes (= '-v -v'). Not sure about the variable\n> names, though, because \"plain\" 'git diff' shows unstaged changes,\n> while 'git diff --cached' shows staged changes.\n\nThis seems a better way to go about.\n\n>\n> Furthermore, it doesn't mean that we can't add properly named command\n> line option(s) and state that '-v|--verbose' is a synonym to\n> '--show-diff' (maybe even deprecate '--verbose'), but I don't want to\n> squeeze even more into a GSOC micro project.\n\nIts perfectly fine. It hardly a half an hour job. Though I like the\nidea that we should use both \"-v|--verbose\" and \"--show-diff\" instead\nof deprecating it. Plus, this edit would not be required. What are\nyour suggestions?\n\nRegards,\nPranit Bauva\n"},{"id":"281819","messageId":"20160325155805.Horde.IaS9Rdqv5fu6BmOZCj5ulLW@webmail.informatik.kit.edu","threadId":"41715","inReplyTo":"CAFZEwPNK+XTjXzjn-Pv3iP4XiajXrdhUHWLz-8c0Ssy_Edkd7A@mail.gmail.com","subject":"Re: parse-options does not recognize \"unspecified\" behavior","fromName":"SZEDER Gábor","fromEmail":"szeder@ira.uka.de","sentAt":"2016-03-25T14:58:05Z","receivedAt":"2016-03-25T14:58:05Z","isPatch":false,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"\nQuoting Pranit Bauva :\n\n> On Sat, Mar 19, 2016 at 4:55 PM, SZEDER Gábor wrote:\n>>> Yes, I think in general, \"-v\" and \"-q\" should work as opposites. But\n>>> that is not the case with commit, where \"-v\" and \"-q\" operate on totally\n>>> separate messages. I think that is a UX mistake, and we would not do\n>>> it that way if designing from scratch. But we're stuck with it for\n>>> historical reasons (I'd probably name \"--verbose\" as \"--show-diff\" or\n>>> something if writing it today).\n>>\n>> Yeah, as a long-time 'git commit -v' user I never really thought about\n>> the name of the option, but when I pointed out the multiple verbose\n>> levels it struck me as a rather unfortunate name for this feature,\n>> too. Oh, well, we are stuck with it indeed.\n>>\n>> However, that doesn't mean that we have to spread this badly chosen\n>> name from options to config variables, does it? I think that if we\n>> are going to define a new config variable today, then it should be\n>> named properly, and it's better not to call it 'commit.verbose', but\n>> 'commit.showDiff' or something. Perhaps we could even define two new\n>> config variables: 'commit.showDiff' for the diff of the changes to be\n>> committed (= '-v'), and 'commit.showUnstagedDiff' for the - wait for\n>> it! - unstaged changes (= '-v -v'). Not sure about the variable\n>> names, though, because \"plain\" 'git diff' shows unstaged changes,\n>> while 'git diff --cached' shows staged changes.\n>\n> This seems a better way to go about.\n>\n>>\n>> Furthermore, it doesn't mean that we can't add properly named command\n>> line option(s) and state that '-v|--verbose' is a synonym to\n>> '--show-diff' (maybe even deprecate '--verbose'), but I don't want to\n>> squeeze even more into a GSOC micro project.\n>\n> Its perfectly fine. It hardly a half an hour job. Though I like the\n> idea that we should use both \"-v|--verbose\" and \"--show-diff\" instead\n> of deprecating it.\n\nDeprecating doesn't mean \"removing\". It merely means that we add a note\nto the documentation stating that the option in question is deprecated,\nbut we will keep supporting it for several years and releases to come.\n\nThis means that '--verbose' and '--show-diff' must coexist for quite a\nwhile, and 'git commit' should do the right thing even if both old and\nnew options are combined. And, of course, in combination with the new\nconfig variable(s). I suspect that all this would be more than a half\nan hour job.\n"},{"id":"281820","messageId":"CAFZEwPO+iMCkMiz9ByzHm3XX7DkvSqbkpHLS+KBhyNHpE2NVJA@mail.gmail.com","threadId":"41715","inReplyTo":"20160325155805.Horde.IaS9Rdqv5fu6BmOZCj5ulLW@webmail.informatik.kit.edu","subject":"Re: parse-options does not recognize \"unspecified\" behavior","fromName":"Pranit Bauva","fromEmail":"pranit.bauva@gmail.com","sentAt":"2016-03-25T15:03:06Z","receivedAt":"2016-03-25T15:03:06Z","isPatch":false,"sender":{"key":"pranit.bauva@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2959938?v=4"},"body":"> Deprecating doesn't mean \"removing\". It merely means that we add a note\n> to the documentation stating that the option in question is deprecated,\n> but we will keep supporting it for several years and releases to come.\n\nOkay. Didn't know.\n\n> This means that '--verbose' and '--show-diff' must coexist for quite a\n> while, and 'git commit' should do the right thing even if both old and\n> new options are combined. And, of course, in combination with the new\n> config variable(s). I suspect that all this would be more than a half\n> an hour job.\n\nNow that I think of it again, I underestimated this. It will take a\nlot more time and create a lot of confusion on my side.\n"}]}