diff mbox series

[1/4] show-branch: show reflog message

Message ID fd2595d370a8a257c44693fdc98194cd8447e22a.1637590855.git.gitgitgadget@gmail.com (mailing list archive)
State Superseded
Headers show
Series Inspect reflog data programmatically in more tests | expand

Commit Message

Han-Wen Nienhuys Nov. 22, 2021, 2:20 p.m. UTC
From: Han-Wen Nienhuys <hanwen@google.com>

Before, --reflog option would look for '\t' in the reflog message. As refs.c
already parses the reflog line, the '\t' was never found, and show-branch
--reflog would always say "(none)" as reflog message

Add test.

Signed-off-by: Han-Wen Nienhuys <hanwen@google.com>
---
 builtin/show-branch.c  | 12 +++++++-----
 t/t3202-show-branch.sh | 15 +++++++++++++++
 2 files changed, 22 insertions(+), 5 deletions(-)

Comments

Junio C Hamano Nov. 22, 2021, 10:22 p.m. UTC | #1
"Han-Wen Nienhuys via GitGitGadget" <gitgitgadget@gmail.com> writes:

> From: Han-Wen Nienhuys <hanwen@google.com>
>
> Before, --reflog option would look for '\t' in the reflog message. As refs.c
> already parses the reflog line, the '\t' was never found, and show-branch
> --reflog would always say "(none)" as reflog message

Well spotted.  It may show that nobody pays attention to output from
"show-branch -g" (or nobody runs it in the first place), but it is
good to fix it anyway.

Thanks.

>
> Add test.
>
> Signed-off-by: Han-Wen Nienhuys <hanwen@google.com>
> ---
>  builtin/show-branch.c  | 12 +++++++-----
>  t/t3202-show-branch.sh | 15 +++++++++++++++
>  2 files changed, 22 insertions(+), 5 deletions(-)
>
> diff --git a/builtin/show-branch.c b/builtin/show-branch.c
> index 082449293b5..f1e8318592c 100644
> --- a/builtin/show-branch.c
> +++ b/builtin/show-branch.c
> @@ -761,6 +761,7 @@ int cmd_show_branch(int ac, const char **av, const char *prefix)
>  			char *logmsg;
>  			char *nth_desc;
>  			const char *msg;
> +			char *end;
>  			timestamp_t timestamp;
>  			int tz;
>  
> @@ -770,11 +771,12 @@ int cmd_show_branch(int ac, const char **av, const char *prefix)
>  				reflog = i;
>  				break;
>  			}
> -			msg = strchr(logmsg, '\t');
> -			if (!msg)
> -				msg = "(none)";
> -			else
> -				msg++;
> +
> +			end = strchr(logmsg, '\n');
> +			if (end)
> +				*end = '\0';
> +
> +			msg = (*logmsg == '\0') ? "(none)" : logmsg;
>  			reflog_msg[i] = xstrfmt("(%s) %s",
>  						show_date(timestamp, tz,
>  							  DATE_MODE(RELATIVE)),
> diff --git a/t/t3202-show-branch.sh b/t/t3202-show-branch.sh
> index ad9902a06b9..d4d64401e4b 100755
> --- a/t/t3202-show-branch.sh
> +++ b/t/t3202-show-branch.sh
> @@ -4,6 +4,9 @@ test_description='test show-branch'
>  
>  . ./test-lib.sh
>  
> +# arbitrary reference time: 2009-08-30 19:20:00
> +GIT_TEST_DATE_NOW=1251660000; export GIT_TEST_DATE_NOW
> +
>  test_expect_success 'setup' '
>  	test_commit initial &&
>  	for i in $(test_seq 1 10)
> @@ -146,4 +149,16 @@ test_expect_success 'show branch --merge-base with N arguments' '
>  	test_cmp expect actual
>  '
>  
> +test_expect_success 'show branch --reflog=2' '
> +	sed "s/^>	//" >expect <<-\EOF &&
> +	>	! [refs/heads/branch10@{0}] (4 years, 5 months ago) commit: branch10
> +	>	 ! [refs/heads/branch10@{1}] (4 years, 5 months ago) commit: branch10
> +	>	--
> +	>	+  [refs/heads/branch10@{0}] branch10
> +	>	++ [refs/heads/branch10@{1}] initial
> +	EOF
> +	git show-branch --reflog=2 >actual &&
> +	test_cmp actual expect
> +'
> +
>  test_done
Bagas Sanjaya Nov. 23, 2021, 7:40 a.m. UTC | #2
On 22/11/21 21.20, Han-Wen Nienhuys via GitGitGadget wrote:
> From: Han-Wen Nienhuys <hanwen@google.com>
> 
> Before, --reflog option would look for '\t' in the reflog message. As refs.c
> already parses the reflog line, the '\t' was never found, and show-branch
> --reflog would always say "(none)" as reflog message
> 
> Add test.
> 

Add what the test?

> +# arbitrary reference time: 2009-08-30 19:20:00
> +GIT_TEST_DATE_NOW=1251660000; export GIT_TEST_DATE_NOW
> +

Why don't you use oneliner `export GIT_TEST_DATE_NOW=1251660000`?
Elijah Newren Nov. 23, 2021, 8:03 a.m. UTC | #3
On Mon, Nov 22, 2021 at 11:44 PM Bagas Sanjaya <bagasdotme@gmail.com> wrote:
>
> On 22/11/21 21.20, Han-Wen Nienhuys via GitGitGadget wrote:
> > From: Han-Wen Nienhuys <hanwen@google.com>
> >
> > Before, --reflog option would look for '\t' in the reflog message. As refs.c
> > already parses the reflog line, the '\t' was never found, and show-branch
> > --reflog would always say "(none)" as reflog message
> >
> > Add test.
> >
>
> Add what the test?
>
> > +# arbitrary reference time: 2009-08-30 19:20:00
> > +GIT_TEST_DATE_NOW=1251660000; export GIT_TEST_DATE_NOW
> > +
>
> Why don't you use oneliner `export GIT_TEST_DATE_NOW=1251660000`?

t/check-non-portable-shell.pl will flag this construct and throw an
error if you put that in your test script.
diff mbox series

Patch

diff --git a/builtin/show-branch.c b/builtin/show-branch.c
index 082449293b5..f1e8318592c 100644
--- a/builtin/show-branch.c
+++ b/builtin/show-branch.c
@@ -761,6 +761,7 @@  int cmd_show_branch(int ac, const char **av, const char *prefix)
 			char *logmsg;
 			char *nth_desc;
 			const char *msg;
+			char *end;
 			timestamp_t timestamp;
 			int tz;
 
@@ -770,11 +771,12 @@  int cmd_show_branch(int ac, const char **av, const char *prefix)
 				reflog = i;
 				break;
 			}
-			msg = strchr(logmsg, '\t');
-			if (!msg)
-				msg = "(none)";
-			else
-				msg++;
+
+			end = strchr(logmsg, '\n');
+			if (end)
+				*end = '\0';
+
+			msg = (*logmsg == '\0') ? "(none)" : logmsg;
 			reflog_msg[i] = xstrfmt("(%s) %s",
 						show_date(timestamp, tz,
 							  DATE_MODE(RELATIVE)),
diff --git a/t/t3202-show-branch.sh b/t/t3202-show-branch.sh
index ad9902a06b9..d4d64401e4b 100755
--- a/t/t3202-show-branch.sh
+++ b/t/t3202-show-branch.sh
@@ -4,6 +4,9 @@  test_description='test show-branch'
 
 . ./test-lib.sh
 
+# arbitrary reference time: 2009-08-30 19:20:00
+GIT_TEST_DATE_NOW=1251660000; export GIT_TEST_DATE_NOW
+
 test_expect_success 'setup' '
 	test_commit initial &&
 	for i in $(test_seq 1 10)
@@ -146,4 +149,16 @@  test_expect_success 'show branch --merge-base with N arguments' '
 	test_cmp expect actual
 '
 
+test_expect_success 'show branch --reflog=2' '
+	sed "s/^>	//" >expect <<-\EOF &&
+	>	! [refs/heads/branch10@{0}] (4 years, 5 months ago) commit: branch10
+	>	 ! [refs/heads/branch10@{1}] (4 years, 5 months ago) commit: branch10
+	>	--
+	>	+  [refs/heads/branch10@{0}] branch10
+	>	++ [refs/heads/branch10@{1}] initial
+	EOF
+	git show-branch --reflog=2 >actual &&
+	test_cmp actual expect
+'
+
 test_done