pretty format string support for reflog times

 pretty.c                      | 36 ++++++++++++++++++++++++++++++++++++
 reflog-walk.c                 | 16 ++++++++++++++++
 reflog-walk.h                 |  3 +++
 t/t4205-log-pretty-formats.sh | 26 ++++++++++++++++++++++++++
 4 files changed, 81 insertions(+)

The reflog contains timestamp information, but these were not exposed to --pretty. Four of the six author/committer format string second-letters were still available and copied, but d/D are taken for reflog selector formatting. So use %gT for "time" instead of "date" mnemonic for using --date=... formatting.

Quoted 5 lines · Phil Pennock

The reflog contains timestamp information, but these were not exposed to --pretty. Four of the six author/committer format string second-letters were still available and copied, but d/D are taken for reflog selector formatting. So use %gT for "time" instead of "date" mnemonic for using --date=... formatting.

Hrm. Since Ted was not cc'd, it is not clear to me whether this is coincidental or in response to the thread over in

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

To summarize, I think the conclusion there was that we would go with at least the 't' and 'r' formatters in the short term.

to get "HEAD@{0} ...rfc-date...", because the presence of "--date" is the trigger to switch the meaning of "%gd".

Quoted 5 lines · Phil Pennock

The reflog contains timestamp information, but these were not exposed to --pretty. Four of the six author/committer format string second-letters were still available and copied, but d/D are taken for reflog selector formatting. So use %gT for "time" instead of "date" mnemonic for using --date=... formatting.

Hrm. Since Ted was not cc'd, it is not clear to me whether this is coincidental or in response to the thread over in

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

To summarize, I think the conclusion there was that we would go with at least the 't' and 'r' formatters in the short term. The 'i/I' ones were not something Ted cared about that much, I think, but they do make things orthogonal with the other ident dates.

Quoted 9 lines

Hrm. Since Ted was not cc'd, it is not clear to me whether this is coincidental or in response to the thread over in

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

To summarize, I think the conclusion there was that we would go with at least the 't' and 'r' formatters in the short term. The 'i/I' ones were not something Ted cared about that much, I think, but they do make things orthogonal with the other ident dates.

I forgot about that thread after it stalled without drawing conclusion, after Ted asked if anybody has a strong opinion and saw only one response to it at

https://public-inbox.org/git/20160711164317.GB3890%40thunk.org/

So, what is the next step? Apply

https://public-inbox.org/git/20160710055402.32684-1-tytso%40mit.edu/

but exclude %g[iI] bits out of that patch while doing so?

I actually think Phil's patch from today is a little cleaner for most of these, as it returns the values via out-parameters, and uses the return value for "did we get anything?".

So the final solution is more like:

- a formatter for just the reflog time, respecting date

- a formatter for just the reflog index (the "0" in HEAD@{0})

- a formatter for the ref name (just the "HEAD" in HEAD@{0})

I am still in favor of this suggestion you earlier made:

Quoted 2 lines

So the final solution is more like:

Quoted 5 lines

- a formatter for just the reflog time, respecting date

- a formatter for just the reflog index (the "0" in HEAD@{0})

- a formatter for the ref name (just the "HEAD" in HEAD@{0})

though. After all we only need three short ones while we migrate away to a longer %(reflog:<what>) format, right?

As to the unfortunate %gd that squats on the "date" other specifiers use, I do not see a good/quick approach to migrate it. If our ideal short-term endgame before the longer format were to use %gd, %g# and %gg for the above three, we first start warning people who use %gd for the historical mistaken "reflog selector", while telling people to use "%gg@{%g#}" instead if they truly want "reflog selector", and then switch its meaning to "reflog date". That would take a long time.

As %r prefix is not taken, we can immediately deprecate %g-anything format as a historical mistake and make sure we do not repeat the mistake of giving "d" to "reflog selector", perhaps?

Quoted 9 lines

Hrm. Since Ted was not cc'd, it is not clear to me whether this is coincidental or in response to the thread over in

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

To summarize, I think the conclusion there was that we would go with at least the 't' and 'r' formatters in the short term. The 'i/I' ones were not something Ted cared about that much, I think, but they do make things orthogonal with the other ident dates.

I forgot about that thread after it stalled without drawing conclusion, after Ted asked if anybody has a strong opinion and saw only one response to it at

https://public-inbox.org/git/20160711164317.GB3890%40thunk.org/

So, what is the next step? Apply

https://public-inbox.org/git/20160710055402.32684-1-tytso%40mit.edu/

but exclude %g[iI] bits out of that patch while doing so?

The reflog contains timestamp information, but these were not exposed to --pretty. Four of the six author/committer format string second-letters were still available and copied, but d/D are taken for reflog selector formatting. So use %gT for "time" instead of "date" mnemonic for using --date=... formatting.

Implementation trade-off: could use format_person_part() if bounce information out of the reflog as a buffer-constructed string, then parse it back again, _and_ then use a hacky letter override for 'T' becoming 'd'. That would avoid reimplementing the specification of the five added timestamp rendering cases. Instead, we allow exporting the timestamp information from the reflog, and if we get it, we add the same data. No memory alloc and string construction/parsing, a small amount of duplication of meaning.

pretty.c +14 -0

+		/*
+		 * all the date options except those taken for other reflog handling
+		 * (so not 'd' or 'D').
+		 */
+		case 'r':
+			if (get_reflog_timeinfo(&timestamp, &tz, c->pretty_ctx->reflog_info)) {
+				strbuf_addstr(sb, show_date(timestamp, tz, DATE_MODE(RELATIVE)));
+			}
+			return 2;
+		case 't':
+			if (get_reflog_timeinfo(&timestamp, &tz, c->pretty_ctx->reflog_info)) {
+				strbuf_addf(sb, "%lu", timestamp);
+			}
+			return 2;

reflog-walk.c +5 -0

+int get_reflog_timeinfo(unsigned long *timestamp_out,
+			 int *tz_out,
+			 struct reflog_walk_info *reflog_info)
+{
+	struct commit_reflog *commit_reflog = reflog_info->last_commit_reflog;

pretty.c static size_t format_commit_one(struct strbuf *sb, /* in UTF-8 */ · line 1251

Quoted 7 lines · Phil Pennock

pretty.c +6 -0

+		 */
+		case 'T':
+			if (get_reflog_timeinfo(&timestamp, &tz, c->pretty_ctx->reflog_info)) {
+				strbuf_addstr(sb, show_date(timestamp, tz, &c->pretty_ctx->date_mode));
+			}
+			return 2;
 		}

I think we can drop this one. It's a nice direction to go eventually, but without the extra placeholders I mentioned elsewhere, it doesn't buy us much.

Our usual error-return is "0 is success", "-1 is error".

Though we don't _always_ adhere to that, and I won't be surprised if you simply copied this from other nearby code.

t/t4205-log-pretty-formats.sh EOF · line 537

Quoted 2 lines

t/t4205-log-pretty-formats.sh +2 -0

+refhead1_short=$(git rev-parse --short --verify HEAD@{0}) &&
+refhead2_short=$(git rev-parse --short --verify HEAD@{1}) &&

We try to push as much as possible into a test_expect_success block, since it handles things like unexpected output to stderr. I guess you put these outside because they are used in multiple tests. You don't have to do that, because tests can affect the greater environment. But if you do keep something outside of a test, you _don't_ want to use &&-chaining, as it means that the lines below it (i.e., the next test!) would simply not be run at all.

Quoted 4 lines

t/t4205-log-pretty-formats.sh +4 -0

+test_expect_success 'can access the reflog' '
+	git reflog -n 2 >actual &&
+	cat >expected <<EOF &&
+$refhead1_short HEAD@{0}: commit (amend): shorter

t/t4205-log-pretty-formats.sh EOF · line 546

Quoted 4 lines

+$refhead2_short HEAD@{1}: commit (amend): short

t/t4205-log-pretty-formats.sh +3 -0

+EOF
+	test_cmp expected actual
+'

I'm not sure what this is testing. Just that we have reflogs turned on at all? I think we can skip this, as it's implicit in the reflog-formatting test below.

You can use "<<-" to ask the shell to strip leading tabs from the here-doc. And then you can indent the contents to match the rest of the test.

I kind of wonder if it would be better to drop "%h" from your format, too. You're just testing the timestamps, and handling the shortening is cluttering up the test. So maybe just:

(I did a few more tweaks to the format to hopefully make it easier to read). It would probably be a more interesting test if the two reflogs actually had different timestamps, though.

Also, come to think of it, that "%gr" test is going to break in about year. :-/

Quoted 1 line

Our usual error-return is "0 is success", "-1 is error".

The idea was "boolean function" and adding more negations elsewhere just made things ugly. I can change if you really want, as consistency wins, but I'll be holding my nose as the invoker flow becomes:

    if (! function_call(out_params)) {
      use(out_params);
    }

which is counter-intuitive (but then, I do much less C these days and have been corrupted). So I'll hold off for now, until told otherwise.

Quoted 10 lines

t/t4205-log-pretty-formats.sh EOF · line 537

Quoted 2 lines

t/t4205-log-pretty-formats.sh +2 -0

+refhead1_short=$(git rev-parse --short --verify HEAD@{0}) &&
+refhead2_short=$(git rev-parse --short --verify HEAD@{1}) &&

We try to push as much as possible into a test_expect_success block, since it handles things like unexpected output to stderr. I guess you put these outside because they are used in multiple tests. You don't have to do that, because tests can affect the greater environment. But if you do keep something outside of a test, you _don't_ want to use &&-chaining, as it means that the lines below it (i.e., the next test!) would simply not be run at all.

This however was matching existing style for head1 and head2 a little above. I was somewhat surprised.

Quoted 4 lines

Quoted 4 lines

t/t4205-log-pretty-formats.sh +4 -0

+test_expect_success 'can access the reflog' '
+	git reflog -n 2 >actual &&
+	cat >expected <<EOF &&
+$refhead1_short HEAD@{0}: commit (amend): shorter

Quoted 8 lines

t/t4205-log-pretty-formats.sh EOF · line 546

Quoted 4 lines

+$refhead2_short HEAD@{1}: commit (amend): short

t/t4205-log-pretty-formats.sh +3 -0

+EOF
+	test_cmp expected actual
+'

I'm not sure what this is testing. Just that we have reflogs turned on at all? I think we can skip this, as it's implicit in the reflog-formatting test below.

Disagree: I could see no existing tests for reflog content matching an expected layout (but could have missed one; I see some _using_ reflog).

If adding a test for minutiae of how tuning options adjust the output, and something changes which breaks the output more widely, the person investigating can spend a lot of time investigating a red herring, looking to see what they broke in the --pretty handling.

First test the basics, then test the specifics, so that if the basics break too then the developer is naturally led to the correct thing to investigate instead of their only clue being that specifics broke.

Quoted 3 lines

You can use "<<-" to ask the shell to strip leading tabs from the here-doc. And then you can indent the contents to match the rest of the test.

You can, but it's fragile if tabs become spaces and it isn't consistent with the existing tests above.

Quoted 3 lines

(I did a few more tweaks to the format to hopefully make it easier to read). It would probably be a more interesting test if the two reflogs actually had different timestamps, though.

Is there a way to force that, through the normalizations?

Quoted 2 lines

Also, come to think of it, that "%gr" test is going to break in about year. :-/

Oh crap, I saw the normalization and thought everything was being set to specifics, but of course the relative is against now. Good catch, thanks. How about:

    | sed "s/[1-9][0-9]* years/N years/"

and then test against "N years" in expected?

Quoted 2 lines

This however was matching existing style for head1 and head2 a little above. I was somewhat surprised.

Ah. Yeah, those are wrong and bad. We should fix that.

Our usual style in the test scripts is to indent here documents with tabs, and use "<<-" to strip the tabs. The result is easier to read.

This old test script did not do so in its inception, and further tests added onto it followed the local style. Let's bring it in line with our usual style.

Some of the tests actually care quite a bit about whitespace, but none of them do so at the beginning of the line (because they things like qz_to_tab_space to avoid

depending on the literal whitespace), so we can do a fairly mechanical conversion.

Most of the here-docs also use interpolation, so they have been left as "<<-EOF". In a few cases, though, where interpolation was not in use, I've converted them to "<<-\EOF" to match our usual "don't interpolate unless you need to" style.

Signed-off-by Jeff King <[email protected]>

---
 t/t4205-log-pretty-formats.sh | 392 +++++++++++++++++++++---------------------
 1 file changed, 196 insertions(+), 196 deletions(-)

t/t4205-log-pretty-formats.sh +196 -196

index d6518fa..f5435fd 100755
@@ -145,199 +145,199 @@ test_expect_success 'setup more commits' '
 
 test_expect_success 'left alignment formatting' '
 	git log --pretty="tformat:%<(40)%s" >actual &&
-	qz_to_tab_space <<EOF >expected &&
-message two                            Z
-message one                            Z
-add bar                                Z
-$(commit_msg)                    Z
-EOF
+	qz_to_tab_space <<-EOF >expected &&
+	message two                            Z
+	message one                            Z
+	add bar                                Z
+	$(commit_msg)                    Z
+	EOF
 	test_cmp expected actual
 '
 
 test_expect_success 'left alignment formatting. i18n.logOutputEncoding' '
 	git -c i18n.logOutputEncoding=$test_encoding log --pretty="tformat:%<(40)%s" >actual &&
-	qz_to_tab_space <<EOF | iconv -f utf-8 -t $test_encoding >expected &&
-message two                            Z
-message one                            Z
-add bar                                Z
-$(commit_msg)                    Z
-EOF
+	qz_to_tab_space <<-EOF | iconv -f utf-8 -t $test_encoding >expected &&
+	message two                            Z
+	message one                            Z
+	add bar                                Z
+	$(commit_msg)                    Z
+	EOF
 	test_cmp expected actual
 '
 
 test_expect_success 'left alignment formatting at the nth column' '
 	git log --pretty="tformat:%h %<|(40)%s" >actual &&
-	qz_to_tab_space <<EOF >expected &&
-$head1 message two                    Z
-$head2 message one                    Z
-$head3 add bar                        Z
-$head4 $(commit_msg)            Z
-EOF
+	qz_to_tab_space <<-EOF >expected &&
+	$head1 message two                    Z
+	$head2 message one                    Z
+	$head3 add bar                        Z
+	$head4 $(commit_msg)            Z
+	EOF
 	test_cmp expected actual
 '
 
 test_expect_success 'left alignment formatting at the nth column' '
 	COLUMNS=50 git log --pretty="tformat:%h %<|(-10)%s" >actual &&
-	qz_to_tab_space <<EOF >expected &&
-$head1 message two                    Z
-$head2 message one                    Z
-$head3 add bar                        Z
-$head4 $(commit_msg)            Z
-EOF
+	qz_to_tab_space <<-EOF >expected &&
+	$head1 message two                    Z
+	$head2 message one                    Z
+	$head3 add bar                        Z
+	$head4 $(commit_msg)            Z
+	EOF
 	test_cmp expected actual
 '
 
 test_expect_success 'left alignment formatting at the nth column. i18n.logOutputEncoding' '
 	git -c i18n.logOutputEncoding=$test_encoding log --pretty="tformat:%h %<|(40)%s" >actual &&
-	qz_to_tab_space <<EOF | iconv -f utf-8 -t $test_encoding >expected &&
-$head1 message two                    Z
-$head2 message one                    Z
-$head3 add bar                        Z
-$head4 $(commit_msg)            Z
-EOF
+	qz_to_tab_space <<-EOF | iconv -f utf-8 -t $test_encoding >expected &&
+	$head1 message two                    Z
+	$head2 message one                    Z
+	$head3 add bar                        Z
+	$head4 $(commit_msg)            Z
+	EOF
 	test_cmp expected actual
 '
 
 test_expect_success 'left alignment formatting with no padding' '
 	git log --pretty="tformat:%<(1)%s" >actual &&
-	cat <<EOF >expected &&
-message two
-message one
-add bar
-$(commit_msg)
-EOF
+	cat <<-EOF >expected &&
+	message two
+	message one
+	add bar
+	$(commit_msg)
+	EOF
 	test_cmp expected actual
 '
 
 test_expect_success 'left alignment formatting with no padding. i18n.logOutputEncoding' '
 	git -c i18n.logOutputEncoding=$test_encoding log --pretty="tformat:%<(1)%s" >actual &&
-	cat <<EOF | iconv -f utf-8 -t $test_encoding >expected &&
-message two
-message one
-add bar
-$(commit_msg)
-EOF
+	cat <<-EOF | iconv -f utf-8 -t $test_encoding >expected &&
+	message two
+	message one
+	add bar
+	$(commit_msg)
+	EOF
 	test_cmp expected actual
 '
 
 test_expect_success 'left alignment formatting with trunc' '
 	git log --pretty="tformat:%<(10,trunc)%s" >actual &&
-	qz_to_tab_space <<EOF >expected &&
-message ..
-message ..
-add bar  Z
-initial...
-EOF
+	qz_to_tab_space <<-\EOF >expected &&
+	message ..
+	message ..
+	add bar  Z
+	initial...
+	EOF
 	test_cmp expected actual
 '
 
 test_expect_success 'left alignment formatting with trunc. i18n.logOutputEncoding' '
 	git -c i18n.logOutputEncoding=$test_encoding log --pretty="tformat:%<(10,trunc)%s" >actual &&
-	qz_to_tab_space <<EOF | iconv -f utf-8 -t $test_encoding >expected &&
-message ..
-message ..
-add bar  Z
-initial...
-EOF
+	qz_to_tab_space <<-\EOF | iconv -f utf-8 -t $test_encoding >expected &&
+	message ..
+	message ..
+	add bar  Z
+	initial...
+	EOF
 	test_cmp expected actual
 '
 
 test_expect_success 'left alignment formatting with ltrunc' '
 	git log --pretty="tformat:%<(10,ltrunc)%s" >actual &&
-	qz_to_tab_space <<EOF >expected &&
-..sage two
-..sage one
-add bar  Z
-..${sample_utf8_part}lich
-EOF
+	qz_to_tab_space <<-EOF >expected &&
+	..sage two
+	..sage one
+	add bar  Z
+	..${sample_utf8_part}lich
+	EOF
 	test_cmp expected actual
 '
 
 test_expect_success 'left alignment formatting with ltrunc. i18n.logOutputEncoding' '
 	git -c i18n.logOutputEncoding=$test_encoding log --pretty="tformat:%<(10,ltrunc)%s" >actual &&
-	qz_to_tab_space <<EOF | iconv -f utf-8 -t $test_encoding >expected &&
-..sage two
-..sage one
-add bar  Z
-..${sample_utf8_part}lich
-EOF
+	qz_to_tab_space <<-EOF | iconv -f utf-8 -t $test_encoding >expected &&
+	..sage two
+	..sage one
+	add bar  Z
+	..${sample_utf8_part}lich
+	EOF
 	test_cmp expected actual
 '
 
 test_expect_success 'left alignment formatting with mtrunc' '
 	git log --pretty="tformat:%<(10,mtrunc)%s" >actual &&
-	qz_to_tab_space <<EOF >expected &&
-mess.. two
-mess.. one
-add bar  Z
-init..lich
-EOF
+	qz_to_tab_space <<-\EOF >expected &&
+	mess.. two
+	mess.. one
+	add bar  Z
+	init..lich
+	EOF
 	test_cmp expected actual
 '
 
 test_expect_success 'left alignment formatting with mtrunc. i18n.logOutputEncoding' '
 	git -c i18n.logOutputEncoding=$test_encoding log --pretty="tformat:%<(10,mtrunc)%s" >actual &&
-	qz_to_tab_space <<EOF | iconv -f utf-8 -t $test_encoding >expected &&
-mess.. two
-mess.. one
-add bar  Z
-init..lich
-EOF
+	qz_to_tab_space <<-\EOF | iconv -f utf-8 -t $test_encoding >expected &&
+	mess.. two
+	mess.. one
+	add bar  Z
+	init..lich
+	EOF
 	test_cmp expected actual
 '
 
 test_expect_success 'right alignment formatting' '
 	git log --pretty="tformat:%>(40)%s" >actual &&
-	qz_to_tab_space <<EOF >expected &&
-Z                            message two
-Z                            message one
-Z                                add bar
-Z                    $(commit_msg)
-EOF
+	qz_to_tab_space <<-EOF >expected &&
+	Z                            message two
+	Z                            message one
+	Z                                add bar
+	Z                    $(commit_msg)
+	EOF
 	test_cmp expected actual
 '
 
 test_expect_success 'right alignment formatting. i18n.logOutputEncoding' '
 	git -c i18n.logOutputEncoding=$test_encoding log --pretty="tformat:%>(40)%s" >actual &&
-	qz_to_tab_space <<EOF | iconv -f utf-8 -t $test_encoding >expected &&
-Z                            message two
-Z                            message one
-Z                                add bar
-Z                    $(commit_msg)
-EOF
+	qz_to_tab_space <<-EOF | iconv -f utf-8 -t $test_encoding >expected &&
+	Z                            message two
+	Z                            message one
+	Z                                add bar
+	Z                    $(commit_msg)
+	EOF
 	test_cmp expected actual
 '
 
 test_expect_success 'right alignment formatting at the nth column' '
 	git log --pretty="tformat:%h %>|(40)%s" >actual &&
-	qz_to_tab_space <<EOF >expected &&
-$head1                      message two
-$head2                      message one
-$head3                          add bar
-$head4              $(commit_msg)
-EOF
+	qz_to_tab_space <<-EOF >expected &&
+	$head1                      message two
+	$head2                      message one
+	$head3                          add bar
+	$head4              $(commit_msg)
+	EOF
 	test_cmp expected actual
 '
 
 test_expect_success 'right alignment formatting at the nth column' '
 	COLUMNS=50 git log --pretty="tformat:%h %>|(-10)%s" >actual &&
-	qz_to_tab_space <<EOF >expected &&
-$head1                      message two
-$head2                      message one
-$head3                          add bar
-$head4              $(commit_msg)
-EOF
+	qz_to_tab_space <<-EOF >expected &&
+	$head1                      message two
+	$head2                      message one
+	$head3                          add bar
+	$head4              $(commit_msg)
+	EOF
 	test_cmp expected actual
 '
 
 test_expect_success 'right alignment formatting at the nth column. i18n.logOutputEncoding' '
 	git -c i18n.logOutputEncoding=$test_encoding log --pretty="tformat:%h %>|(40)%s" >actual &&
-	qz_to_tab_space <<EOF | iconv -f utf-8 -t $test_encoding >expected &&
-$head1                      message two
-$head2                      message one
-$head3                          add bar
-$head4              $(commit_msg)
-EOF
+	qz_to_tab_space <<-EOF | iconv -f utf-8 -t $test_encoding >expected &&
+	$head1                      message two
+	$head2                      message one
+	$head3                          add bar
+	$head4              $(commit_msg)
+	EOF
 	test_cmp expected actual
 '
 

@@ -345,110 +345,110 @@ EOF
 # as in previous test.
 test_expect_success 'right alignment formatting at the nth column with --graph. i18n.logOutputEncoding' '
 	git -c i18n.logOutputEncoding=$test_encoding log --graph --pretty="tformat:%h %>|(40)%s" >actual &&
-	iconv -f utf-8 -t $test_encoding >expected <<EOF&&
-* $head1                    message two
-* $head2                    message one
-* $head3                        add bar
-* $head4            $(commit_msg)
-EOF
+	iconv -f utf-8 -t $test_encoding >expected <<-EOF &&
+	* $head1                    message two
+	* $head2                    message one
+	* $head3                        add bar
+	* $head4            $(commit_msg)
+	EOF
 	test_cmp expected actual
 '
 
 test_expect_success 'right alignment formatting with no padding' '
 	git log --pretty="tformat:%>(1)%s" >actual &&
-	cat <<EOF >expected &&
-message two
-message one
-add bar
-$(commit_msg)
-EOF
+	cat <<-EOF >expected &&
+	message two
+	message one
+	add bar
+	$(commit_msg)
+	EOF
 	test_cmp expected actual
 '
 
 test_expect_success 'right alignment formatting with no padding and with --graph' '
 	git log --graph --pretty="tformat:%>(1)%s" >actual &&
-	cat <<EOF >expected &&
-* message two
-* message one
-* add bar
-* $(commit_msg)
-EOF
+	cat <<-EOF >expected &&
+	* message two
+	* message one
+	* add bar
+	* $(commit_msg)
+	EOF
 	test_cmp expected actual
 '
 
 test_expect_success 'right alignment formatting with no padding. i18n.logOutputEncoding' '
 	git -c i18n.logOutputEncoding=$test_encoding log --pretty="tformat:%>(1)%s" >actual &&
-	cat <<EOF | iconv -f utf-8 -t $test_encoding >expected &&
-message two
-message one
-add bar
-$(commit_msg)
-EOF
+	cat <<-EOF | iconv -f utf-8 -t $test_encoding >expected &&
+	message two
+	message one
+	add bar
+	$(commit_msg)
+	EOF
 	test_cmp expected actual
 '
 
 test_expect_success 'center alignment formatting' '
 	git log --pretty="tformat:%><(40)%s" >actual &&
-	qz_to_tab_space <<EOF >expected &&
-Z             message two              Z
-Z             message one              Z
-Z               add bar                Z
-Z         $(commit_msg)          Z
-EOF
+	qz_to_tab_space <<-EOF >expected &&
+	Z             message two              Z
+	Z             message one              Z
+	Z               add bar                Z
+	Z         $(commit_msg)          Z
+	EOF
 	test_cmp expected actual
 '
 
 test_expect_success 'center alignment formatting. i18n.logOutputEncoding' '
 	git -c i18n.logOutputEncoding=$test_encoding log --pretty="tformat:%><(40)%s" >actual &&
-	qz_to_tab_space <<EOF | iconv -f utf-8 -t $test_encoding >expected &&
-Z             message two              Z
-Z             message one              Z
-Z               add bar                Z
-Z         $(commit_msg)          Z
-EOF
+	qz_to_tab_space <<-EOF | iconv -f utf-8 -t $test_encoding >expected &&
+	Z             message two              Z
+	Z             message one              Z
+	Z               add bar                Z
+	Z         $(commit_msg)          Z
+	EOF
 	test_cmp expected actual
 '
 test_expect_success 'center alignment formatting at the nth column' '
 	git log --pretty="tformat:%h %><|(40)%s" >actual &&
-	qz_to_tab_space <<EOF >expected &&
-$head1           message two          Z
-$head2           message one          Z
-$head3             add bar            Z
-$head4       $(commit_msg)      Z
-EOF
+	qz_to_tab_space <<-EOF >expected &&
+	$head1           message two          Z
+	$head2           message one          Z
+	$head3             add bar            Z
+	$head4       $(commit_msg)      Z
+	EOF
 	test_cmp expected actual
 '
 
 test_expect_success 'center alignment formatting at the nth column' '
 	COLUMNS=70 git log --pretty="tformat:%h %><|(-30)%s" >actual &&
-	qz_to_tab_space <<EOF >expected &&
-$head1           message two          Z
-$head2           message one          Z
-$head3             add bar            Z
-$head4       $(commit_msg)      Z
-EOF
+	qz_to_tab_space <<-EOF >expected &&
+	$head1           message two          Z
+	$head2           message one          Z
+	$head3             add bar            Z
+	$head4       $(commit_msg)      Z
+	EOF
 	test_cmp expected actual
 '
 
 test_expect_success 'center alignment formatting at the nth column. i18n.logOutputEncoding' '
 	git -c i18n.logOutputEncoding=$test_encoding log --pretty="tformat:%h %><|(40)%s" >actual &&
-	qz_to_tab_space <<EOF | iconv -f utf-8 -t $test_encoding >expected &&
-$head1           message two          Z
-$head2           message one          Z
-$head3             add bar            Z
-$head4       $(commit_msg)      Z
-EOF
+	qz_to_tab_space <<-EOF | iconv -f utf-8 -t $test_encoding >expected &&
+	$head1           message two          Z
+	$head2           message one          Z
+	$head3             add bar            Z
+	$head4       $(commit_msg)      Z
+	EOF
 	test_cmp expected actual
 '
 
 test_expect_success 'center alignment formatting with no padding' '
 	git log --pretty="tformat:%><(1)%s" >actual &&
-	cat <<EOF >expected &&
-message two
-message one
-add bar
-$(commit_msg)
-EOF
+	cat <<-EOF >expected &&
+	message two
+	message one
+	add bar
+	$(commit_msg)
+	EOF
 	test_cmp expected actual
 '
 

@@ -457,34 +457,34 @@ EOF
 old_head1=$(git rev-parse --verify HEAD~0)
 test_expect_success 'center alignment formatting with no padding. i18n.logOutputEncoding' '
 	git -c i18n.logOutputEncoding=$test_encoding log --pretty="tformat:%><(1)%s" >actual &&
-	cat <<EOF | iconv -f utf-8 -t $test_encoding >expected &&
-message two
-message one
-add bar
-$(commit_msg)
-EOF
+	cat <<-EOF | iconv -f utf-8 -t $test_encoding >expected &&
+	message two
+	message one
+	add bar
+	$(commit_msg)
+	EOF
 	test_cmp expected actual
 '
 
 test_expect_success 'left/right alignment formatting with stealing' '
 	git commit --amend -m short --author "long long long <[email protected]>" &&
 	git log --pretty="tformat:%<(10,trunc)%s%>>(10,ltrunc)% an" >actual &&
-	cat <<EOF >expected &&
-short long  long long
-message ..   A U Thor
-add bar      A U Thor
-initial...   A U Thor
-EOF
+	cat <<-\EOF >expected &&
+	short long  long long
+	message ..   A U Thor
+	add bar      A U Thor
+	initial...   A U Thor
+	EOF
 	test_cmp expected actual
 '
 test_expect_success 'left/right alignment formatting with stealing. i18n.logOutputEncoding' '
 	git -c i18n.logOutputEncoding=$test_encoding log --pretty="tformat:%<(10,trunc)%s%>>(10,ltrunc)% an" >actual &&
-	cat <<EOF | iconv -f utf-8 -t $test_encoding >expected &&
-short long  long long
-message ..   A U Thor
-add bar      A U Thor
-initial...   A U Thor
-EOF
+	cat <<-\EOF | iconv -f utf-8 -t $test_encoding >expected &&
+	short long  long long
+	message ..   A U Thor
+	add bar      A U Thor
+	initial...   A U Thor
+	EOF
 	test_cmp expected actual
 '
 

@@ -515,22 +515,22 @@ test_expect_success 'log decoration properly follows tag chain' '
 	git tag -d tag1 &&
 	git commit --amend -m shorter &&
 	git log --no-walk --tags --pretty="%H %d" --decorate=full >actual &&
-	cat <<EOF >expected &&
-$head1  (tag: refs/tags/tag2)
-$head2  (tag: refs/tags/message-one)
-$old_head1  (tag: refs/tags/message-two)
-EOF
+	cat <<-EOF >expected &&
+	$head1  (tag: refs/tags/tag2)
+	$head2  (tag: refs/tags/message-one)
+	$old_head1  (tag: refs/tags/message-two)
+	EOF
 	sort actual >actual1 &&
 	test_cmp expected actual1
 '
 
 test_expect_success 'clean log decoration' '
 	git log --no-walk --tags --pretty="%H %D" --decorate=full >actual &&
-	cat >expected <<EOF &&
-$head1 tag: refs/tags/tag2
-$head2 tag: refs/tags/message-one
-$old_head1 tag: refs/tags/message-two
-EOF
+	cat >expected <<-EOF &&
+	$head1 tag: refs/tags/tag2
+	$head2 tag: refs/tags/message-one
+	$old_head1 tag: refs/tags/message-two
+	EOF
 	sort actual >actual1 &&
 	test_cmp expected actual1
 '

Quoted 3 lines

Some of the tests actually care quite a bit about whitespace, but none of them do so at the beginning of the line (because they things like qz_to_tab_space to avoid

I'll do s/they things/they use things/ here while queuing.

Yeah, I agree the "!" test for "did it work" is counter-intuitive if you're coming from other languages, but it's pretty normal for C code bases (especially ours).

Ah. Yeah, those are wrong and bad. We should fix that.

I don't buy the tabs-become-spaces argument. We use tabs for indentation in Git, and that's extremely unlikely to change. If your patch gets munged in transit or by your editor, then the maintainer is going to complain when applying your patch.

The magic function you are looking for is "test_tick()", which advances

Quoted 3 lines · Jeff King

Yeah, I agree the "!" test for "did it work" is counter-intuitive if you're coming from other languages, but it's pretty normal for C code bases (especially ours).

For stuff returning pointers, sure.

Quoted 4 lines

I don't buy the tabs-become-spaces argument. We use tabs for indentation in Git, and that's extremely unlikely to change. If your patch gets munged in transit or by your editor, then the maintainer is going to complain when applying your patch.

Okay. (I happen to think that robustness against a cycle of developers discussing why whitespace broke patches in transit is good, but I'll change this).

I'm not going to write any more code today, for reasons of "weak human needs sleep and I'll make more stupid mistakes if I continue". So I'm going silent on this thread for the rest of today. Not ignoring. If folks want fast progress (I haven't looked at release cycle status) I won't mind cutting me out of the loop :-D but otherwise I'll look Thu/Fri this week for any remedial work and offer a fresh patch with fixes from this thread, and documentation.

I'm tempted to just steal the docs from Ted's patch, unless that's considered bad form?