{"thread":{"id":"27108","subject":"git status -s -v: no override","startedAt":"2011-04-15T21:34:55Z","lastAt":"2011-05-15T04:05:01Z","messageCount":10,"participants":["Jacek Masiulaniec","Jeff King","Jonathan Nieder","Junio C Hamano"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"165933","messageId":"BANLkTimKO7ihPpJ80Ad1kbYaMv1ycu0y9A@mail.gmail.com","threadId":"27108","inReplyTo":null,"subject":"git status -s -v: no override","fromName":"Jacek Masiulaniec","fromEmail":"jacekm@dobremiasto.net","sentAt":"2011-04-15T21:34:55Z","receivedAt":"2011-04-15T21:34:55Z","isPatch":false,"sender":{"key":"jacekm@dobremiasto.net","avatar":null},"body":"Hello git@,\n\nSmall git-status argument processing issue:\n\n[jacekm@localhost test]$ git init\nInitialized empty Git repository in /private/tmp/test/.git/\n[jacekm@localhost test]$ git status -v\n# On branch master\n#\n# Initial commit\n#\nnothing to commit (create/copy files and use \"git add\" to track)\n[jacekm@localhost test]$ git status -s\n[jacekm@localhost test]$ git status -v -s\n[jacekm@localhost test]$ git status -s -v\n[jacekm@localhost test]$\n\nThings look consistent until the last command: -v does not override -s,\nwhich is unexpected given that -s does override -v.\n\nJacek\n"},{"id":"165940","messageId":"20110416000918.GB9334@sigill.intra.peff.net","threadId":"27108","inReplyTo":"BANLkTimKO7ihPpJ80Ad1kbYaMv1ycu0y9A@mail.gmail.com","subject":"Re: git status -s -v: no override","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2011-04-16T00:09:18Z","receivedAt":"2011-04-16T00:09:18Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Apr 15, 2011 at 10:34:55PM +0100, Jacek Masiulaniec wrote:\n\n> Small git-status argument processing issue:\n> \n> [jacekm@localhost test]$ git init\n> Initialized empty Git repository in /private/tmp/test/.git/\n> [jacekm@localhost test]$ git status -v\n> # On branch master\n> #\n> # Initial commit\n> #\n> nothing to commit (create/copy files and use \"git add\" to track)\n> [jacekm@localhost test]$ git status -s\n> [jacekm@localhost test]$ git status -v -s\n> [jacekm@localhost test]$ git status -s -v\n> [jacekm@localhost test]$\n> \n> Things look consistent until the last command: -v does not override -s,\n> which is unexpected given that -s does override -v.\n\nSort of. I think you are expecting \"-v\" to mean \"use the long output\nformat\", but it doesn't. Instead, \"-v\" actually indicates that we should\nshow a diff along with the usual output (in your case, the diff is\nempty).\n\nThere is no option that means \"counteract -s or --porcelain seen earlier\non the command line and use the default long format\", which I think is\nwhat you want.\n\n-Peff\n"},{"id":"165941","messageId":"20110416004544.GA5628@elie","threadId":"27108","inReplyTo":"20110416000918.GB9334@sigill.intra.peff.net","subject":"Re: git status -s -v: no override","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2011-04-16T00:45:45Z","receivedAt":"2011-04-16T00:45:45Z","isPatch":false,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Jeff King wrote:\n\n> There is no option that means \"counteract -s or --porcelain seen earlier\n> on the command line and use the default long format\", which I think is\n> what you want.\n\nDoesn't \"git status $opts --no-short --no-porcelain\" work?\n"},{"id":"165943","messageId":"20110416013723.GA23105@sigill.intra.peff.net","threadId":"27108","inReplyTo":"20110416004544.GA5628@elie","subject":"Re: git status -s -v: no override","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2011-04-16T01:37:23Z","receivedAt":"2011-04-16T01:37:23Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Apr 15, 2011 at 07:45:45PM -0500, Jonathan Nieder wrote:\n\n> Jeff King wrote:\n> \n> > There is no option that means \"counteract -s or --porcelain seen earlier\n> > on the command line and use the default long format\", which I think is\n> > what you want.\n> \n> Doesn't \"git status $opts --no-short --no-porcelain\" work?\n\nHmm. That does work (with either option, or both), but it is somewhat of\nan accident. There is an enum specifying the format the user wants. We\nhand it to parse-options for those options, telling it that the value is\nan int. Parse-options will treat --no-foo on an int as setting it to 0.\n\nThe enumeration list does not have explicit integer values, but does\nhappen to have the constant for the long format first, which in ANSI C\nguarantees it to be 0.\n\nSo yeah, it works, but I will admit to being surprised by it. And\ncertainly as a user, I wouldn't have thought of that.\n\nI think it probably should have been a tristate like:\n\n --format={long,short,porcelain}\n\nall along, though I don't know that it is a particularly big deal. We\ncan still do that if we want, and just keep --short and --porcelain as\naliases.\n\n-Peff\n"},{"id":"165947","messageId":"20110416052704.GA10807@elie","threadId":"27108","inReplyTo":"20110416013723.GA23105@sigill.intra.peff.net","subject":"[PATCH] status: store format option as an int","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2011-04-16T05:27:04Z","receivedAt":"2011-04-16T05:27:04Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"It is unsafe to pass a pointer to a value of enumerated type to\nOPT_SET_INT (as v1.7.0-rc0~137^2~14, 2009-0905) does, since it might\nhave the wrong alignment or width (C99 only says \"Each enumerated type\nshall be compatible with char, a signed integer type, or an unsigned\ninteger type. The choice of type is implementation-defined, but shall\nbe capable of representing the values of all the members of the\nenumeration.)\n\nProbably this didn't come up in practice because by default GCC uses\nan 'int' to represent small enums unless passed -fshort-enums (except\non certain architectures where -fshort-enums is the default).\n\nNoticed-by: Jeff King \nSigned-off-by: Jonathan Nieder \n---\nJeff King wrote:\n\n> Hmm. That does work (with either option, or both), but it is somewhat of\n> an accident. There is an enum specifying the format the user wants. We\n> hand it to parse-options for those options, telling it that the value is\n> an int.\n\nYikes. That's an accident waiting to happen. How about this, to start?\n\n builtin/commit.c | 7 ++++---\n 1 files changed, 4 insertions(+), 3 deletions(-)\n\ndiff --git a/builtin/commit.c b/builtin/commit.c\nindex 6e32166..b28848d 100644\n--- a/builtin/commit.c\n+++ b/builtin/commit.c\n@@ -94,11 +94,12 @@ static const char *only_include_assumed;\n static struct strbuf message;\n \n static int null_termination;\n-static enum {\n-\tSTATUS_FORMAT_LONG,\n+enum status_format {\n+\tSTATUS_FORMAT_LONG = 0,\n \tSTATUS_FORMAT_SHORT,\n \tSTATUS_FORMAT_PORCELAIN\n-} status_format = STATUS_FORMAT_LONG;\n+};\n+static int status_format;\n static int status_show_branch;\n \n static int opt_parse_m(const struct option *opt, const char *arg, int unset)\n-- \n1.7.5.rc2\n"},{"id":"165948","messageId":"20110416052959.GB10807@elie","threadId":"27108","inReplyTo":"20110416052704.GA10807@elie","subject":"Re: [PATCH] status: store format option as an int","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2011-04-16T05:29:59Z","receivedAt":"2011-04-16T05:29:59Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Jonathan Nieder wrote:\n\n> It is unsafe to pass a pointer to a value of enumerated type to\n> OPT_SET_INT (as v1.7.0-rc0~137^2~14, 2009-0905) does, since it might\n\nAgh, proofreading fail. For the confused, this is supposed to read as\n\"(as v1.7.0-rc0~137^2~14, status: refactor format option parsing,\n2009-09-05) does\".\n"},{"id":"165950","messageId":"7vy63abu3i.fsf@alter.siamese.dyndns.org","threadId":"27108","inReplyTo":"20110416052959.GB10807@elie","subject":"Re: [PATCH] status: store format option as an int","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-04-16T06:14:57Z","receivedAt":"2011-04-16T06:14:57Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jonathan Nieder writes:\n\n> Jonathan Nieder wrote:\n>\n>> It is unsafe to pass a pointer to a value of enumerated type to\n>> OPT_SET_INT (as v1.7.0-rc0~137^2~14, 2009-0905) does, since it might\n>\n> Agh, proofreading fail. For the confused, this is supposed to read as\n> \"(as v1.7.0-rc0~137^2~14, status: refactor format option parsing,\n> 2009-09-05) does\".\n\nStill ECANTPARSE. Perhaps the \"does\" at the end should be inside the same\nparentheses pair as \"as v1.7.0-...\"?\n"},{"id":"165951","messageId":"20110416062233.GA28853@sigill.intra.peff.net","threadId":"27108","inReplyTo":"20110416052704.GA10807@elie","subject":"Re: [PATCH] status: store format option as an int","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2011-04-16T06:22:33Z","receivedAt":"2011-04-16T06:22:33Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sat, Apr 16, 2011 at 12:27:04AM -0500, Jonathan Nieder wrote:\n\n> It is unsafe to pass a pointer to a value of enumerated type to\n> OPT_SET_INT (as v1.7.0-rc0~137^2~14, 2009-0905) does, since it might\n> have the wrong alignment or width (C99 only says \"Each enumerated type\n> shall be compatible with char, a signed integer type, or an unsigned\n> integer type. The choice of type is implementation-defined, but shall\n> be capable of representing the values of all the members of the\n> enumeration.)\n> \n> Probably this didn't come up in practice because by default GCC uses\n> an 'int' to represent small enums unless passed -fshort-enums (except\n> on certain architectures where -fshort-enums is the default).\n> \n> Noticed-by: Jeff King \n\nIf by \"noticed by\" you mean \"mentioned but was completely unaware of the\nsignificance of what he was saying\", then yes. :)\n\nNow that you mention it, though, I was reminded that we had run across\nsomething similar before. And I think it was this:\n\n http://article.gmane.org/gmane.comp.version-control.git/144858\n\nYour fix looks sane. I don't think we can do anything more clever on the\nparse-options side.\n\n-Peff\n"},{"id":"165953","messageId":"20110416062727.GA15933@elie","threadId":"27108","inReplyTo":"7vy63abu3i.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] status: store format option as an int","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2011-04-16T06:28:20Z","receivedAt":"2011-04-16T06:28:20Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Junio C Hamano wrote:\n> Jonathan Nieder writes:\n>> Jonathan Nieder wrote:\n\n>> Agh, proofreading fail. For the confused, this is supposed to read as\n>> \"(as v1.7.0-rc0~137^2~14, status: refactor format option parsing,\n>> 2009-09-05) does\".\n>\n> Still ECANTPARSE. Perhaps the \"does\" at the end should be inside the same\n> parentheses pair as \"as v1.7.0-...\"?\n\nYes. Sorry for the noise.\n"},{"id":"167878","messageId":"20110515040501.GA28948@elie","threadId":"27108","inReplyTo":"20110416052704.GA10807@elie","subject":"[PATCH resend] status: store format option as an int","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2011-05-15T04:05:01Z","receivedAt":"2011-05-15T04:05:01Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"It is unsafe to pass a pointer to a value of enumerated type to\nOPT_SET_INT (as v1.7.0-rc0~137^2~14, 2009-09-05, does), since it might\nhave the wrong alignment or width. C99 only says \"Each enumerated\ntype shall be compatible with char, a signed integer type, or an\nunsigned integer type. The choice of type is implementation-defined,\nbut shall be capable of representing the values of all the members of\nthe enumeration.\"\n\nProbably this hasn't come up in practice because GCC uses an 'int' to\nrepresent small enums unless passed -fshort-enums, except on certain\narchitectures where -fshort-enums is the default.\n\nNoticed-by: Jeff King \nSigned-off-by: Jonathan Nieder \nAcked-by: Jeff King \n---\nI last sent this about a month ago and it seemed ok. The changes\nsince last time are to the commit message:\n\n - hopefully it parses as English now\n - adding Jeff's ack\n\nThanks again, both.\n\n builtin/commit.c | 7 ++++---\n 1 files changed, 4 insertions(+), 3 deletions(-)\n\ndiff --git a/builtin/commit.c b/builtin/commit.c\nindex 411d5e4..64808aa 100644\n--- a/builtin/commit.c\n+++ b/builtin/commit.c\n@@ -108,11 +108,12 @@ static const char *only_include_assumed;\n static struct strbuf message;\n \n static int null_termination;\n-static enum {\n-\tSTATUS_FORMAT_LONG,\n+enum status_format {\n+\tSTATUS_FORMAT_LONG = 0,\n \tSTATUS_FORMAT_SHORT,\n \tSTATUS_FORMAT_PORCELAIN\n-} status_format = STATUS_FORMAT_LONG;\n+};\n+static int status_format;\n static int status_show_branch;\n \n static int opt_parse_m(const struct option *opt, const char *arg, int unset)\n-- \n1.7.5.1\n"}]}