{"thread":{"id":"46965","subject":"[PATCH] diff: alias -q to --quiet","startedAt":"2017-10-13T16:56:46Z","lastAt":"2017-10-16T21:45:19Z","messageCount":4,"participants":["Anthony Sottile","Jeff King","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"330369","messageId":"20171013164415.6632-1-asottile@umich.edu","threadId":"46965","inReplyTo":null,"subject":"[PATCH] diff: alias -q to --quiet","fromName":"Anthony Sottile","fromEmail":"asottile@umich.edu","sentAt":"2017-10-13T16:44:15Z","receivedAt":"2017-10-13T16:56:46Z","isPatch":true,"sender":{"key":"asottile@umich.edu","avatar":"https://avatars.githubusercontent.com/u/1810591?v=4"},"body":"Previously, `-q` was silently ignored:\n\nBefore:\n\n$ git diff -q -- Documentation/; echo $?\ndiff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt\nindex a88c767..aa6e724 100644\n--- a/Documentation/diff-options.txt\n+++ b/Documentation/diff-options.txt\n@@ -587,6 +587,7 @@ ifndef::git-log[]\n        That is, it exits with 1 if there were differences and\n        0 means no differences.\n\n+-q::\n --quiet::\n        Disable all output of the program. Implies `--exit-code`.\n endif::git-log[]\n0\n$\n\nAfter:\n$ ./git diff -q -- Documentation/; echo $?\n1\n$\n\nSigned-off-by: Anthony Sottile <asottile@umich.edu>\n---\n Documentation/diff-options.txt | 1 +\n diff.c                         | 2 +-\n 2 files changed, 2 insertions(+), 1 deletion(-)\n\ndiff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt\nindex a88c767..aa6e724 100644\n--- a/Documentation/diff-options.txt\n+++ b/Documentation/diff-options.txt\n@@ -587,6 +587,7 @@ ifndef::git-log[]\n \tThat is, it exits with 1 if there were differences and\n \t0 means no differences.\n \n+-q::\n --quiet::\n \tDisable all output of the program. Implies `--exit-code`.\n endif::git-log[]\ndiff --git a/diff.c b/diff.c\nindex 69f0357..13dfc3e 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -4751,7 +4751,7 @@ int diff_opt_parse(struct diff_options *options,\n \t}\n \telse if (!strcmp(arg, \"--exit-code\"))\n \t\tDIFF_OPT_SET(options, EXIT_WITH_STATUS);\n-\telse if (!strcmp(arg, \"--quiet\"))\n+\telse if (!strcmp(arg, \"-q\") || !strcmp(arg, \"--quiet\"))\n \t\tDIFF_OPT_SET(options, QUICK);\n \telse if (!strcmp(arg, \"--ext-diff\"))\n \t\tDIFF_OPT_SET(options, ALLOW_EXTERNAL);\n-- \n2.7.4\n\n"},{"id":"330377","messageId":"20171013175656.qitarz6lvrppyb5f@sigill.intra.peff.net","threadId":"46965","inReplyTo":"20171013164415.6632-1-asottile@umich.edu","subject":"Re: [PATCH] diff: alias -q to --quiet","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-10-13T17:56:56Z","receivedAt":"2017-10-13T17:57:03Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Oct 13, 2017 at 09:44:15AM -0700, Anthony Sottile wrote:\n\n> Previously, `-q` was silently ignored:\n\nI'm not sure if is totally ignored. Normally if we have an unknown\noptions we'd complain:\n\n  $ git diff -x\n  error: invalid option: -x\n\nbut we don't with \"-q\". Why?\n\nIn builtin/diff.c:471, we can see:\n\n  else if (!strcmp(argv[1], \"-q\"))\n          options |= DIFF_SILENT_ON_REMOVED;\n\nSo it _does_ do something, just not what you expected.\n\nBut wait, that's not the whole story. We convert \"-q\" into\nSILENT_ON_REMOVED in git-diff-files and in git-diff (when we're acting\nlike diff-files). But nobody ever seems to check it!\n\nRunning \"git log -p -SSILENT_ON_REMOVED\" turns up two interesting\ncommits:\n\n  - 95a7c546b0 (diff: deprecate -q option to diff-files, 2013-07-17)\n\n  - c48f6816f0 (diff: remove \"diff-files -q\" in a version of Git in a\n    distant future, 2013-07-18).\n\nSo we dropped \"-q\" a few years ago and added a deprecation notice. \"git\ntag --contains 95a7c546b0\" tells us that happened in v1.8.5, which\nshipped in Nov 2013.\n\nAnd then in v2.0.0 (May 2014) we tried to drop \"-q\" completely. Looking\nover c48f6816f0, I _think_ it's a mistake that \"-q\" became a silent noop\nthere. That commit should have ripped out the remaining bits that set\nthe SILENT_ON_REMOVED flag, and \"-q\" would have become an error.\n\nSo there are two separate questions/tasks:\n\n  1. Should we remove the special handling of \"-q\" leftover from this\n     deprecation? I think the answer is yes.\n\n  2. Should we teach the diff machinery as a whole to treat \"-q\" as a\n     synonym for \"--quiet\".\n\n     Probably yes, but it's less clear to me that this won't have funny\n     interactions. Are there other commands which use the diff-options\n     parser via setup_revisions(), but expect \"-q\" to be left in the\n     output so that they can handle it themselves?\n\n     It looks like git-log does so, but it pulls the \"-q\" out before\n     handing the remainder to setup_revisions(). And anyway, it just\n     converts the option into a quiet diff (though it does in a way\n     that's different than the rest of the diff code -- that might bear\n     investigating on its own).\n\n     Grepping for 'q' and OPT__QUIET, I don't see any others, but I\n     didn't spend very much time digging.\n\n-Peff\n"},{"id":"330389","messageId":"xmqq60biv4if.fsf@gitster.mtv.corp.google.com","threadId":"46965","inReplyTo":"20171013175656.qitarz6lvrppyb5f@sigill.intra.peff.net","subject":"Re: [PATCH] diff: alias -q to --quiet","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-10-14T02:37:28Z","receivedAt":"2017-10-14T02:37:36Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> So there are two separate questions/tasks:\n>\n>   1. Should we remove the special handling of \"-q\" leftover from this\n>      deprecation? I think the answer is yes.\n>\n>   2. Should we teach the diff machinery as a whole to treat \"-q\" as a\n>      synonym for \"--quiet\".\n\nGood questions.  And thanks for archaeology.\n\nThe topic #1 above is something that should have happened when \"-q\" stopped working\nas \"--diff-filter=d\", and we probably should have started to error\nout then, so that scripts that relied on the original behaviour\nwould have been forced to update.  That did not happen which was a\ngrave mistake.\n\nBy doing so, we would have made sure any script that uses \"-q\" died\nout, and after a while, we can talk about reusing it for other\npurposes, like the topic #2 above.\n\nIs it worth making \"-q\" error out while doing #1 and keep it error\nout for a few years?  I have a feeling that the answer might be\nunfortunately yes _if_ we want to also do #2.  Even though we broke\n\"-q\" for the scripts who wanted to see it ignore only the removals 4\nyears ago and left it broken since then.  Removals are much rarer\nthan modifications and additions, so it wouldn't be surprising if\nthe users of these scripts simply did not notice the old breakage,\nbut if we made \"-q\" to mean \"--quiet\" without doing #1, they will\nbreak, as all diffs these scripts work on will suddenly give an\nempty output.\n\nIf we aren't doing #2, then I do not think we need to make \"-q\"\nerror out when we do #1, though.\n\nIn any case, if we were to do both of the above two, they must\nhappen in that order, not the other way around.\n\nThanks.\n\n"},{"id":"330482","messageId":"20171016214512.xlp4ctzo37niaarx@sigill.intra.peff.net","threadId":"46965","inReplyTo":"xmqq60biv4if.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH] diff: alias -q to --quiet","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-10-16T21:45:13Z","receivedAt":"2017-10-16T21:45:19Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sat, Oct 14, 2017 at 11:37:28AM +0900, Junio C Hamano wrote:\n\n> Jeff King <peff@peff.net> writes:\n> \n> > So there are two separate questions/tasks:\n> >\n> >   1. Should we remove the special handling of \"-q\" leftover from this\n> >      deprecation? I think the answer is yes.\n> >\n> >   2. Should we teach the diff machinery as a whole to treat \"-q\" as a\n> >      synonym for \"--quiet\".\n> \n> Good questions.  And thanks for archaeology.\n> \n> The topic #1 above is something that should have happened when \"-q\" stopped working\n> as \"--diff-filter=d\", and we probably should have started to error\n> out then, so that scripts that relied on the original behaviour\n> would have been forced to update.  That did not happen which was a\n> grave mistake.\n> \n> By doing so, we would have made sure any script that uses \"-q\" died\n> out, and after a while, we can talk about reusing it for other\n> purposes, like the topic #2 above.\n> \n> Is it worth making \"-q\" error out while doing #1 and keep it error\n> out for a few years?  I have a feeling that the answer might be\n> unfortunately yes _if_ we want to also do #2.  Even though we broke\n> \"-q\" for the scripts who wanted to see it ignore only the removals 4\n> years ago and left it broken since then.  Removals are much rarer\n> than modifications and additions, so it wouldn't be surprising if\n> the users of these scripts simply did not notice the old breakage,\n> but if we made \"-q\" to mean \"--quiet\" without doing #1, they will\n> break, as all diffs these scripts work on will suddenly give an\n> empty output.\n\nYeah, after thinking about it, I do think we'd want to restart the\ndeprecation period. For some features it would be fine, but this one is\nsufficiently subtle that I agree there's a good chance scripts might\nhave been broken without anybody noticing them.\n\n> If we aren't doing #2, then I do not think we need to make \"-q\"\n> error out when we do #1, though.\n\nI don't think we'd add an explicit error-out. But by removing the\nleftover code, we would naturally say \"no such option: -q\", which\namounts to the same thing.\n\n> In any case, if we were to do both of the above two, they must\n> happen in that order, not the other way around.\n\nYep, agreed.\n\n-Peff\n"}]}