I think git show is broken

Subsystems: the rest

3 messages, 2 authors, 2016-06-15 · open the first message on its own page

I think git show is broken

From: Matthew Caron <hidden>
Date: 2016-06-15 22:54:35

(otherwise, there was a very strange change made to its functionality, 
which the documentation does not reflect)


Old, working git:

===
$ git --version
git version 1.7.0.4
$ git show --quiet --abbrev-commit --pretty=oneline 
47a7aee54553fb718c376cfa9d7de4389a391e33
47a7aee Fix hyperlinks for dependent tickets (#7139, #4976).
===

New, "broken" git:

===
$ git --version
git version 1.7.9.5

$ git show --quiet --abbrev-commit --pretty=oneline 
47a7aee54553fb718c376cfa9d7de4389a391e33
47a7aee Fix hyperlinks for dependent tickets (#7139, #4976).
diff --git a/mastertickets/web_ui.py b/mastertickets/web_ui.py
index a91b862..698ed98 100644
--- a/mastertickets/web_ui.py
+++ b/mastertickets/web_ui.py
@@ -32,7 +32,7 @@ class MasterTicketsModule(Component):
      use_gs = BoolOption('mastertickets', 'use_gs', default=False,
                          doc='If enabled, use ghostscript to produce 
nicer output.')

-    FIELD_XPATH = 
'div[@id="ticket"]/table[@class="properties"]/td[@headers="h_%s"]/text()'
+    FIELD_XPATH = 
'.//div[@id="ticket"]/table[@class="properties"]//td[@headers="h_%s"]/text()'
      fields = set(['blocking', 'blockedby'])

      # IRequestFilter methods
===

It appears as though the new functionality always puts out a "medium" 
verbosity diff. Though the manpage says that it honors pretty=oneline, 
it does not seem to.

I searched around the message logs, etc. and would have expected this 
change to have thrown everyone else into as much upheaval as it has in 
my organization, and found nothing. Am I missing something?

Thanks in advance.
-- 
Matthew Caron, Software Build Engineer
Sixnet, a Red Lion business | www.sixnet.com
+1 (518) 877-5173 x138 office

Re: I think git show is broken

From: Matthew Caron <hidden>
Date: 2016-06-15 22:54:35

On 08/28/2012 01:38 PM, Matthew Caron wrote:
(otherwise, there was a very strange change made to its functionality,
which the documentation does not reflect)
Never mind.

I was looking in the wrong spot. The issue is not with --pretty=oneline, 
it's with --quiet. In 1.7.0.4, --quiet worked like -s. It no longer does 
in 1.7.9.5. Switching to -s cures the problem.

-- 
Matthew Caron, Software Build Engineer
Sixnet, a Red Lion business | www.sixnet.com
+1 (518) 877-5173 x138 office

Re: I think git show is broken

From: Jeff King <hidden>
Date: 2016-06-15 22:54:35

On Tue, Aug 28, 2012 at 01:48:27PM -0400, Matthew Caron wrote:
On 08/28/2012 01:38 PM, Matthew Caron wrote:
quoted
(otherwise, there was a very strange change made to its functionality,
which the documentation does not reflect)
Never mind.

I was looking in the wrong spot. The issue is not with
--pretty=oneline, it's with --quiet. In 1.7.0.4, --quiet worked like
-s. It no longer does in 1.7.9.5. Switching to -s cures the problem.
Yes, that is what's going on. But it's still a regression. There was
some discussion of what --quiet should do here:

  http://thread.gmane.org/gmane.comp.version-control.git/171357

which resulted in a patch that took away --quiet. But then this thread:

  http://thread.gmane.org/gmane.comp.version-control.git/174665

resulted in restoring it as a synonym for "-s". Unfortunately there's a
bug in that fix, which you are seeing. Patch is below.

-- >8 --
Subject: [PATCH] log: fix --quiet synonym for -s

Originally the "--quiet" option was parsed by the
diff-option parser into the internal QUICK option. This had
the effect of silencing diff output from the log (which was
not intended, but happened to work and people started to
use it). But it also had other odd side effects at the diff
level (for example, it would suppress the second commit in
"git show A B").

To fix this, commit 1c40c36 converted log to parse-options
and handled the "quiet" option separately, not passing it
on to the diff code. However, it simply ignored the option,
which was a regression for people using it as a synonym for
"-s". Commit 01771a8 then fixed that by interpreting the
option to add DIFF_FORMAT_NO_OUTPUT to the list of output
formats.

However, that commit did not fix it in all cases. It sets
the flag after setup_revisions is called. Naively, this
makes sense because you would expect the setup_revisions
parser to overwrite our output format flag if "-p" or
another output format flag is seen.

However, that is not how the NO_OUTPUT flag works. We
actually store it in the bit-field as just another format.
At the end of setup_revisions, we call diff_setup_done,
which post-processes the bitfield and clears any other
formats if we have set NO_OUTPUT. By setting the flag after
setup_revisions is done, diff_setup_done does not have a
chance to make this tweak, and we end up with other format
options still set.

As a result, the flag would have no effect in "git log -p
--quiet" or "git show --quiet".  Fix it by setting the
format flag before the call to setup_revisions.

Signed-off-by: Jeff King <redacted>
---
 builtin/log.c   |  2 +-
 t/t7007-show.sh | 12 ++++++++++++
 2 files changed, 13 insertions(+), 1 deletion(-)
diff --git a/builtin/log.c b/builtin/log.c
index ecc2793..c22469c 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -109,9 +109,9 @@ static void cmd_log_init_finish(int argc, const char **argv, const char *prefix,
 			     PARSE_OPT_KEEP_ARGV0 | PARSE_OPT_KEEP_UNKNOWN |
 			     PARSE_OPT_KEEP_DASHDASH);
 
-	argc = setup_revisions(argc, argv, rev, opt);
 	if (quiet)
 		rev->diffopt.output_format |= DIFF_FORMAT_NO_OUTPUT;
+	argc = setup_revisions(argc, argv, rev, opt);
 
 	/* Any arguments at this point are not recognized */
 	if (argc > 1)
diff --git a/t/t7007-show.sh b/t/t7007-show.sh
index a40cd36..e41fa00 100755
--- a/t/t7007-show.sh
+++ b/t/t7007-show.sh
@@ -108,4 +108,16 @@ test_expect_success 'showing range' '
 	test_cmp expect actual.filtered
 '
 
+test_expect_success '-s suppresses diff' '
+	echo main3 >expect &&
+	git show -s --format=%s main3 >actual &&
+	test_cmp expect actual
+'
+
+test_expect_success '--quiet suppresses diff' '
+	echo main3 >expect &&
+	git show --quiet --format=%s main3 >actual &&
+	test_cmp expect actual
+'
+
 test_done
-- 
1.7.11.5.10.g3c8125b
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help