All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jacob Keller <jacob.e.keller@intel.com>
To: git@vger.kernel.org
Cc: Junio C Hamano <gitster@pobox.com>,
	Dennis Kaarsemaker <dennis@kaarsemaker.net>,
	Jacob Keller <jacob.keller@gmail.com>
Subject: [PATCH] rev-list: use hdr_termination instead of a always using a newline
Date: Thu, 20 Oct 2016 11:19:30 -0700	[thread overview]
Message-ID: <20161020181930.21084-1-jacob.e.keller@intel.com> (raw)

From: Jacob Keller <jacob.keller@gmail.com>

When adding support for prefixing output of log and other commands using
--line-prefix, commit 660e113ce118 ("graph: add support for
--line-prefix on all graph-aware output", 2016-08-31) accidentally
broke rev-list --header output.

In order to make the output appear with a line-prefix, the flow was
changed to always use the graph subsystem for display. Unfortunately
the graph flow in rev-list did not use info->hdr_termination as it was
assumed that graph output would never need to putput NULs.

Since we now always use the graph code in order to handle the case of
line-prefix, simply replace putchar('\n') with
putchar(info->hdr_termination) which will correct this issue.

Add a test for the --header case to make sure we don't break it in the
future. Implement a helper function test_ends_with_nul() to make it more
obvious what sort of check we are looking for.

Reported-by: Dennis Kaarsemaker <dennis@kaarsemaker.net>
Signed-off-by: Jacob Keller <jacob.keller@gmail.com>
---

Here's my solution, with an updated test using a helper function based
on using sed (which I think is more portable than tail -n1 ?). The
change actually is very simple. I ran the test suite and it appears to
be not breaking anyone else since the normal case is
hdr_termination="\n" except in the cases where it needs to be NUL.

Thanks for the bug report!

 builtin/rev-list.c       |  2 +-
 t/t6000-rev-list-misc.sh | 11 +++++++++++
 2 files changed, 12 insertions(+), 1 deletion(-)

diff --git a/builtin/rev-list.c b/builtin/rev-list.c
index 8479f6ed28aa..c43decda7011 100644
--- a/builtin/rev-list.c
+++ b/builtin/rev-list.c
@@ -145,7 +145,7 @@ static void show_commit(struct commit *commit, void *data)
 			 */
 			if (buf.len && buf.buf[buf.len - 1] == '\n')
 				graph_show_padding(revs->graph);
-			putchar('\n');
+			putchar(info->hdr_termination);
 		} else {
 			/*
 			 * If the message buffer is empty, just show
diff --git a/t/t6000-rev-list-misc.sh b/t/t6000-rev-list-misc.sh
index 3e752ce03280..e8c6979baf59 100755
--- a/t/t6000-rev-list-misc.sh
+++ b/t/t6000-rev-list-misc.sh
@@ -4,6 +4,12 @@ test_description='miscellaneous rev-list tests'
 
 . ./test-lib.sh
 
+test_ends_with_nul() {
+	printf "\0" >nul
+	sed '$!d' "$@" >contents
+	test_cmp_bin nul contents
+}
+
 test_expect_success setup '
 	echo content1 >wanted_file &&
 	echo content2 >unwanted_file &&
@@ -100,4 +106,9 @@ test_expect_success '--bisect and --first-parent can not be combined' '
 	test_must_fail git rev-list --bisect --first-parent HEAD
 '
 
+test_expect_success '--header shows a NUL after each commit' '
+	git rev-list --header --max-count=1 HEAD | sed \$!d >actual &&
+	test_ends_with_nul actual
+'
+
 test_done
-- 
2.10.0.560.g867c144


             reply	other threads:[~2016-10-20 18:19 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-10-20 18:19 Jacob Keller [this message]
2016-10-20 18:49 ` [PATCH] rev-list: use hdr_termination instead of a always using a newline Dennis Kaarsemaker
2016-10-20 18:54 ` Junio C Hamano
2016-10-20 20:05   ` Jacob Keller

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20161020181930.21084-1-jacob.e.keller@intel.com \
    --to=jacob.e.keller@intel.com \
    --cc=dennis@kaarsemaker.net \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=jacob.keller@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.