[PATCH 0/1] extend the truncating pretty formats

[PATCH 0/1] extend the truncating pretty formats

[Philip Oakley]

A recent enquiry on the Git-Users list asked for horizontal log graphs
similar to those used in the manual ASCII art graphs. These use single
or double character short strings for commits.

The existing pretty pretty-formats are unable to truncate to single or
double characters because of their use of ellipsis `..`. As a starter,
let's add left and right hard truncation options to the existing
options as a preparatory step for potential `--horizontal` graphs.

It is noted that the  truncation options do not have any tests yet.

Also cc'ing Nsengiyumva who has proposed to look at unifying the 
ref-filter and --pretty formats [1]

--
Philip

To: Git List <git@vger.kernel.org>
Cc: NSENGIYUMVA WILBERFORCE <nsengiyumvawilberforce@gmail.com>
Cc: Taylor Blau <me@ttaylorr.com>
Cc: Junio C Hamano <gitster@pobox.com>


[1] https://lore.kernel.org/git/CA+PPyiE=baAoVkrghE5GQMt984AcaL=XBAQRsVRbN8w7jQA+ig@mail.gmail.com/

Philip Oakley (1):
  pretty-formats: add hard truncation, without ellipsis, options

 Documentation/pretty-formats.txt |  7 ++++---
 pretty.c                         | 18 +++++++++++++++++-
 2 files changed, 21 insertions(+), 4 deletions(-)

-- 
2.38.1.281.g83ef3ded8d

[PATCH 1/1] pretty-formats: add hard truncation, without ellipsis, options

[Philip Oakley]

Instead of replacing with "..", replace with the empty string "",
having adjusted the padding length calculation.

Needswork:
There are no tests for these pretty formats, before or after
this change.

Signed-off-by: Philip Oakley <philipoakley@iee.email>
---
 Documentation/pretty-formats.txt |  7 ++++---
 pretty.c                         | 18 +++++++++++++++++-
 2 files changed, 21 insertions(+), 4 deletions(-)

diff --git a/Documentation/pretty-formats.txt b/Documentation/pretty-formats.txt
index 0b4c1c8d98..bd1657c032 100644
--- a/Documentation/pretty-formats.txt
+++ b/Documentation/pretty-formats.txt
@@ -146,14 +146,15 @@ The placeholders are:
 '%m':: left (`<`), right (`>`) or boundary (`-`) mark
 '%w([<w>[,<i1>[,<i2>]]])':: switch line wrapping, like the -w option of
 			    linkgit:git-shortlog[1].
-'%<(<N>[,trunc|ltrunc|mtrunc])':: make the next placeholder take at
+'%<(<N>[,trunc|ltrunc|mtrunc|Trunc|Ltrunc])':: make the next placeholder take at
 				  least N columns, padding spaces on
 				  the right if necessary.  Optionally
-				  truncate at the beginning (ltrunc),
+				  truncate (with ellipsis '..') at the beginning (ltrunc),
 				  the middle (mtrunc) or the end
 				  (trunc) if the output is longer than
-				  N columns.  Note that truncating
+				  N columns.  Note that truncating with ellipsis
 				  only works correctly with N >= 2.
+				  Use (Trunc|Ltrunc) for hard truncation without ellipsis.
 '%<|(<N>)':: make the next placeholder take at least until Nth
 	     columns, padding spaces on the right if necessary
 '%>(<N>)', '%>|(<N>)':: similar to '%<(<N>)', '%<|(<N>)' respectively,
diff --git a/pretty.c b/pretty.c
index 6cb363ae1c..f67844d638 100644
--- a/pretty.c
+++ b/pretty.c
@@ -857,7 +857,9 @@ enum trunc_type {
 	trunc_none,
 	trunc_left,
 	trunc_middle,
-	trunc_right
+	trunc_right,
+	trunc_left_hard,
+	trunc_right_hard
 };
 
 struct format_commit_context {
@@ -1145,6 +1147,10 @@ static size_t parse_padding_placeholder(const char *placeholder,
 				c->truncate = trunc_left;
 			else if (starts_with(start, "mtrunc)"))
 				c->truncate = trunc_middle;
+			else if (starts_with(start, "Ltrunc)"))
+				c->truncate = trunc_left_hard;
+			else if (starts_with(start, "Trunc)"))
+				c->truncate = trunc_right_hard;
 			else
 				return 0;
 		} else
@@ -1743,6 +1749,16 @@ static size_t format_and_pad_commit(struct strbuf *sb, /* in UTF-8 */
 					    padding - 2, len - (padding - 2),
 					    "..");
 			break;
+		case trunc_left_hard:
+			strbuf_utf8_replace(&local_sb,
+					    0, len - (padding),
+					    "");
+			break;
+		case trunc_right_hard:
+			strbuf_utf8_replace(&local_sb,
+					    padding, len - (padding),
+					    "");
+			break;
 		case trunc_none:
 			break;
 		}
-- 
2.38.1.281.g83ef3ded8d

Re: [PATCH 1/1] pretty-formats: add hard truncation, without ellipsis, options

[Taylor Blau]

On Sun, Oct 30, 2022 at 06:56:14PM +0000, Philip Oakley wrote:
> Instead of replacing with "..", replace with the empty string "",
> having adjusted the padding length calculation.
>
> Needswork:
> There are no tests for these pretty formats, before or after
> this change.

Hmmph. I see some when searching for l?trunc in
t4205-log-pretty-formats.sh. Is that coverage not sufficient for the
existing feature?

If so, it would be nice to see it bulked up to add coverage where we
are missing some. Either way, we should make sure the new code is
covered before continuing.

> @@ -1743,6 +1749,16 @@ static size_t format_and_pad_commit(struct strbuf *sb, /* in UTF-8 */
>  					    padding - 2, len - (padding - 2),
>  					    "..");
>  			break;
> +		case trunc_left_hard:
> +			strbuf_utf8_replace(&local_sb,
> +					    0, len - (padding),
> +					    "");
> +			break;
> +		case trunc_right_hard:
> +			strbuf_utf8_replace(&local_sb,
> +					    padding, len - (padding),
> +					    "");
> +			break;

If I remember correctly, strbuf_utf8_replace() supports taking NULL as
its last argument, though upon searching I couldn't find any callers
that behave that way. Can we use that instead of supplying the empty
string? If not, should we drop support for the NULL-as-last-argument?

Thanks,
Taylor

Re: [PATCH 1/1] pretty-formats: add hard truncation, without ellipsis, options

[Philip Oakley]

On 30/10/2022 19:23, Taylor Blau wrote:
> On Sun, Oct 30, 2022 at 06:56:14PM +0000, Philip Oakley wrote:
>> Instead of replacing with "..", replace with the empty string "",
>> having adjusted the padding length calculation.
>>
>> Needswork:
>> There are no tests for these pretty formats, before or after
>> this change.
> Hmmph. I see some when searching for l?trunc in
> t4205-log-pretty-formats.sh. Is that coverage not sufficient for the
> existing feature?
>
> If so, it would be nice to see it bulked up to add coverage where we
> are missing some. Either way, we should make sure the new code is
> covered before continuing.

No idea how I missed them. Maybe I'd filtered, by accident, the search
by a too narrow list of file types.

Thanks for that pointer.

>> @@ -1743,6 +1749,16 @@ static size_t format_and_pad_commit(struct strbuf *sb, /* in UTF-8 */
>>  					    padding - 2, len - (padding - 2),
>>  					    "..");
>>  			break;
>> +		case trunc_left_hard:
>> +			strbuf_utf8_replace(&local_sb,
>> +					    0, len - (padding),
>> +					    "");
>> +			break;
>> +		case trunc_right_hard:
>> +			strbuf_utf8_replace(&local_sb,
>> +					    padding, len - (padding),
>> +					    "");
>> +			break;
> If I remember correctly, strbuf_utf8_replace() supports taking NULL as
> its last argument, though upon searching I couldn't find any callers
> that behave that way. Can we use that instead of supplying the empty
> string? If not, should we drop support for the NULL-as-last-argument?

I wasalso  concerned about zero length strings but my brief look at the
code indicated it could cope with a zero length string.
The last param is `const char *subst`.

I've just relooked at the code and it does have a

     if (subst)
        subst_len = strlen(subst);

so it does look safe to pass NULL, though it would probably cause a
pause for thought for readers, and isn't likely to be that much faster
in these format scenarios.
>
> Thanks,
> Taylor
--
Philip

Re: [PATCH 1/1] pretty-formats: add hard truncation, without ellipsis, options

[Taylor Blau]

On Sun, Oct 30, 2022 at 10:01:47PM +0000, Philip Oakley wrote:
> > If I remember correctly, strbuf_utf8_replace() supports taking NULL as
> > its last argument, though upon searching I couldn't find any callers
> > that behave that way. Can we use that instead of supplying the empty
> > string? If not, should we drop support for the NULL-as-last-argument?
>
> I wasalso  concerned about zero length strings but my brief look at the
> code indicated it could cope with a zero length string.
> The last param is `const char *subst`.
>
> I've just relooked at the code and it does have a
>
>      if (subst)
>         subst_len = strlen(subst);
>
> so it does look safe to pass NULL, though it would probably cause a
> pause for thought for readers, and isn't likely to be that much faster
> in these format scenarios.

I'm not worried at all about speed, I was just wondering if there was a
more designated helper for "truncate these many UTF-8 characters from
the end of a string".

It appears that passing NULL to that argument behaves the same as
passing "", so I was wondering if it would be clearer to use that. But
I'm fine either way. If there are no callers that pass NULL, then we
should consider something like the following:

--- 8< ---
diff --git a/utf8.c b/utf8.c
index de4ce5c0e6..e8813f64fe 100644
--- a/utf8.c
+++ b/utf8.c
@@ -361,10 +361,9 @@ void strbuf_utf8_replace(struct strbuf *sb_src, int pos, int width,
 	char *src = sb_src->buf;
 	char *end = src + sb_src->len;
 	char *dst;
-	int w = 0, subst_len = 0;
+	int w = 0, subst_len;

-	if (subst)
-		subst_len = strlen(subst);
+	subst_len = strlen(subst);
 	strbuf_grow(&sb_dst, sb_src->len + subst_len);
 	dst = sb_dst.buf;
--- >8 ---

Below the context there is another "if (subst)", but that one needs to
stay since we only perform the replacement once.

Thanks,
Taylor

Re: [PATCH 0/1] extend the truncating pretty formats

[Philip Oakley]

On 30/10/2022 18:56, Philip Oakley wrote:
> A recent enquiry on the Git-Users list 

forgot the link
https://groups.google.com/g/git-users/c/ayo_4Sy65TI/m/wRkmrcVWAQAJ

> asked for horizontal log graphs
> similar to those used in the manual ASCII art graphs. These use single
> or double character short strings for commits.
>
> The existing pretty pretty-formats are unable to truncate to single or
> double characters because of their use of ellipsis `..`. As a starter,
> let's add left and right hard truncation options to the existing
> options as a preparatory step for potential `--horizontal` graphs.
>
> It is noted that the  truncation options do not have any tests yet.
>
> Also cc'ing Nsengiyumva who has proposed to look at unifying the 
> ref-filter and --pretty formats [1]
>
> --
> Philip
>
> To: Git List <git@vger.kernel.org>
> Cc: NSENGIYUMVA WILBERFORCE <nsengiyumvawilberforce@gmail.com>
> Cc: Taylor Blau <me@ttaylorr.com>
> Cc: Junio C Hamano <gitster@pobox.com>
>
>
> [1] https://lore.kernel.org/git/CA+PPyiE=baAoVkrghE5GQMt984AcaL=XBAQRsVRbN8w7jQA+ig@mail.gmail.com/
>
> Philip Oakley (1):
>   pretty-formats: add hard truncation, without ellipsis, options
>
>  Documentation/pretty-formats.txt |  7 ++++---
>  pretty.c                         | 18 +++++++++++++++++-
>  2 files changed, 21 insertions(+), 4 deletions(-)
>

[PATCH v2 0/1] extend the truncating pretty formats

[Philip Oakley]

Changes since V1.

Added tests for the [L]Trunc in both t/t4205-log-pretty-formats and
t/t6006-rev-list-format matching the existing tests, which I'd missed
before. Thanks to Taylor for point out my error.

Use NULL rather than the empty string "" const which strbuf_utf8_replace
accepts. Also remove unnecessary brackets.

Added suggestive examples for which end of the string are left and right
when truncating, along with middle.

Aside: no attempt at clarifying the potential man page confusion over
`<` in the '%<(<N>[,..` placeholder character sequence and subsequent
entries.

[V1] <20221030185614.3842-1-philipoakley@iee.email> 
A recent enquiry on the Git-Users list asked for horizontal log graphs
similar to those used in the manual ASCII art graphs. These use single
or double character short strings for commits.

The existing pretty pretty-formats are unable to truncate to single or
double characters because of their use of ellipsis `..`. As a starter,
let's add left and right hard truncation options to the existing
options as a preparatory step for potential `--horizontal` graphs.

It is noted that the  truncation options do not have any tests yet.

Also cc'ing Nsengiyumva who has proposed to look at unifying the 
ref-filter and --pretty formats [1]

--
Philip

To: Git List <git@vger.kernel.org>
Cc: NSENGIYUMVA WILBERFORCE <nsengiyumvawilberforce@gmail.com>
Cc: Taylor Blau <me@ttaylorr.com>
Cc: Junio C Hamano <gitster@pobox.com>


[1] https://lore.kernel.org/git/CA+PPyiE=baAoVkrghE5GQMt984AcaL=XBAQRsVRbN8w7jQA+ig@mail.gmail.com/

Philip Oakley (1):
  pretty-formats: add hard truncation, without ellipsis, options

 Documentation/pretty-formats.txt | 11 +++---
 pretty.c                         | 18 ++++++++-
 t/t4205-log-pretty-formats.sh    | 67 ++++++++++++++++++++++++++++++++
 t/t6006-rev-list-format.sh       | 45 +++++++++++++++++++++
 4 files changed, 135 insertions(+), 6 deletions(-)

1:  0f3a12c66c ! 1:  35f83cac94 pretty-formats: add hard truncation, without ellipsis, options
    @@ Documentation/pretty-formats.txt: The placeholders are:
      				  least N columns, padding spaces on
      				  the right if necessary.  Optionally
     -				  truncate at the beginning (ltrunc),
    -+				  truncate (with ellipsis '..') at the beginning (ltrunc),
    - 				  the middle (mtrunc) or the end
    - 				  (trunc) if the output is longer than
    +-				  the middle (mtrunc) or the end
    +-				  (trunc) if the output is longer than
     -				  N columns.  Note that truncating
    ++				  truncate (with ellipsis '..') at the beginning (ltrunc) `..ft`,
    ++				  the middle (mtrunc) `mi..le` or the end
    ++				  (trunc) `rig..` if the output is longer than
     +				  N columns.  Note that truncating with ellipsis
      				  only works correctly with N >= 2.
     +				  Use (Trunc|Ltrunc) for hard truncation without ellipsis.
    @@ pretty.c: static size_t format_and_pad_commit(struct strbuf *sb, /* in UTF-8 */
      			break;
     +		case trunc_left_hard:
     +			strbuf_utf8_replace(&local_sb,
    -+					    0, len - (padding),
    -+					    "");
    ++					    0, len - padding,
    ++					    NULL);
     +			break;
     +		case trunc_right_hard:
     +			strbuf_utf8_replace(&local_sb,
    -+					    padding, len - (padding),
    -+					    "");
    ++					    padding, len - padding,
    ++					    NULL);
     +			break;
      		case trunc_none:
      			break;
      		}
    +
    + ## t/t4205-log-pretty-formats.sh ##
    +@@ t/t4205-log-pretty-formats.sh: test_expect_success 'left alignment formatting with trunc' '
    + 	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 tw
    ++	message on
    ++	add bar  Z
    ++	initial. a
    ++	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 &&
    +@@ t/t4205-log-pretty-formats.sh: test_expect_success 'left alignment formatting with trunc. i18n.logOutputEncodin
    + 	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 tw
    ++	message on
    ++	add bar  Z
    ++	initial. a
    ++	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 &&
    +@@ t/t4205-log-pretty-formats.sh: test_expect_success 'left alignment formatting with ltrunc' '
    + 	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 &&
    ++	essage two
    ++	essage one
    ++	add bar  Z
    ++	an${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 &&
    +@@ t/t4205-log-pretty-formats.sh: test_expect_success 'left alignment formatting with ltrunc. i18n.logOutputEncodi
    + 	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 &&
    ++	essage two
    ++	essage one
    ++	add bar  Z
    ++	an${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 &&
    +@@ t/t4205-log-pretty-formats.sh: test_expect_success 'left/right alignment formatting with stealing' '
    + 	EOF
    + 	test_cmp expected actual
    + '
    ++
    ++test_expect_success 'left/right hard alignment formatting with stealing' '
    ++	git commit --amend -m short --author "long long long <long@me.com>" &&
    ++	git log --pretty="tformat:%<(10,Trunc)%s%>>(10,Ltrunc)% an" >actual &&
    ++	cat <<-\EOF >expected &&
    ++	short long  long long
    ++	message on   A U Thor
    ++	add bar      A U Thor
    ++	initial. a   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 &&
    +@@ t/t4205-log-pretty-formats.sh: test_expect_success 'left/right alignment formatting with stealing. i18n.logOutp
    + 	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 on   A U Thor
    ++	add bar      A U Thor
    ++	initial. a   A U Thor
    ++	EOF
    ++	test_cmp expected actual
    ++'
    + 
    + test_expect_success 'strbuf_utf8_replace() not producing NUL' '
    + 	git log --color --pretty="tformat:%<(10,trunc)%s%>>(10,ltrunc)%C(auto)%d" |
    +
    + ## t/t6006-rev-list-format.sh ##
    +@@ t/t6006-rev-list-format.sh: commit $head1
    + added (hinzugef${added_utf8_part}gt..
    + EOF
    + 
    ++# ZZZ for a space?
    ++test_format subject-truncated "%<($truncate_count,Trunc)%s" qz_to_tab_space <<EOF
    ++commit $head2
    ++changed (ge${changed_utf8_part}ndert) f
    ++commit $head1
    ++added (hinzugef${added_utf8_part}gt)Z
    ++EOF
    ++
    + test_format body %b <<EOF
    + commit $head2
    + commit $head1
    +@@ t/t6006-rev-list-format.sh: commit $head1
    + added (hinzugef${added_utf8_part_iso88591}gt..
    + EOF
    + 
    ++# need qz qz_to_tab_space
    ++test_format complex-subject-Trunc "%<($truncate_count,Trunc)%s" qz_to_tab_space <<EOF
    ++commit $head3
    ++Test printing of com
    ++commit $head2
    ++changed (ge${changed_utf8_part_iso88591}ndert) f
    ++commit $head1
    ++added (hinzugef${added_utf8_part_iso88591}gt)Z
    ++EOF
    ++
    + test_format complex-subject-mtrunc "%<($truncate_count,mtrunc)%s" <<EOF
    + commit $head3
    + Test prin..ex bodies
    +@@ t/t6006-rev-list-format.sh: commit $head1
    + .. (hinzugef${added_utf8_part_iso88591}gt) foo
    + EOF
    + 
    ++test_format complex-subject-Ltrunc "%<($truncate_count,Ltrunc)%s" <<EOF
    ++commit $head3
    ++ng of complex bodies
    ++commit $head2
    ++anged (ge${changed_utf8_part_iso88591}ndert) foo
    ++commit $head1
    ++ed (hinzugef${added_utf8_part_iso88591}gt) foo
    ++EOF
    + test_expect_success 'setup expected messages (for test %b)' '
    + 	cat <<-EOF >expected.utf-8 &&
    + 	commit $head3
    +@@ t/t6006-rev-list-format.sh: commit $head1
    + added (hinzugef${added_utf8_part}gt..
    + EOF
    + 
    ++# need qz_to_tab_space
    ++test_format complex-subject-commitencoding-unset-Trunc "%<($truncate_count,Trunc)%s" qz_to_tab_space <<EOF
    ++commit $head3
    ++Test printing of com
    ++commit $head2
    ++changed (ge${changed_utf8_part}ndert) f
    ++commit $head1
    ++added (hinzugef${added_utf8_part}gt)Z
    ++EOF
    ++
    + test_format complex-subject-commitencoding-unset-mtrunc "%<($truncate_count,mtrunc)%s" <<EOF
    + commit $head3
    + Test prin..ex bodies
    +@@ t/t6006-rev-list-format.sh: commit $head1
    + .. (hinzugef${added_utf8_part}gt) foo
    + EOF
    + 
    ++test_format complex-subject-commitencoding-unset-Ltrunc "%<($truncate_count,Ltrunc)%s" <<EOF
    ++commit $head3
    ++ng of complex bodies
    ++commit $head2
    ++anged (ge${changed_utf8_part}ndert) foo
    ++commit $head1
    ++ed (hinzugef${added_utf8_part}gt) foo
    ++EOF
    ++
    + test_format complex-body-commitencoding-unset %b <expected.utf-8
    + 
    + test_expect_success '%x00 shows NUL' '

-- 
2.38.1.windows.1

[PATCH v2 1/1] pretty-formats: add hard truncation, without ellipsis, options

[Philip Oakley]

Instead of replacing with "..", replace with the empty string "",
having adjusted the padding length calculation.

Needswork:
There are no tests for these pretty formats, before or after
this change.

Signed-off-by: Philip Oakley <philipoakley@iee.email>
---
 Documentation/pretty-formats.txt | 11 +++---
 pretty.c                         | 18 ++++++++-
 t/t4205-log-pretty-formats.sh    | 67 ++++++++++++++++++++++++++++++++
 t/t6006-rev-list-format.sh       | 45 +++++++++++++++++++++
 4 files changed, 135 insertions(+), 6 deletions(-)

diff --git a/Documentation/pretty-formats.txt b/Documentation/pretty-formats.txt
index 0b4c1c8d98..0bd339db33 100644
--- a/Documentation/pretty-formats.txt
+++ b/Documentation/pretty-formats.txt
@@ -146,14 +146,15 @@ The placeholders are:
 '%m':: left (`<`), right (`>`) or boundary (`-`) mark
 '%w([<w>[,<i1>[,<i2>]]])':: switch line wrapping, like the -w option of
 			    linkgit:git-shortlog[1].
-'%<(<N>[,trunc|ltrunc|mtrunc])':: make the next placeholder take at
+'%<(<N>[,trunc|ltrunc|mtrunc|Trunc|Ltrunc])':: make the next placeholder take at
 				  least N columns, padding spaces on
 				  the right if necessary.  Optionally
-				  truncate at the beginning (ltrunc),
-				  the middle (mtrunc) or the end
-				  (trunc) if the output is longer than
-				  N columns.  Note that truncating
+				  truncate (with ellipsis '..') at the beginning (ltrunc) `..ft`,
+				  the middle (mtrunc) `mi..le` or the end
+				  (trunc) `rig..` if the output is longer than
+				  N columns.  Note that truncating with ellipsis
 				  only works correctly with N >= 2.
+				  Use (Trunc|Ltrunc) for hard truncation without ellipsis.
 '%<|(<N>)':: make the next placeholder take at least until Nth
 	     columns, padding spaces on the right if necessary
 '%>(<N>)', '%>|(<N>)':: similar to '%<(<N>)', '%<|(<N>)' respectively,
diff --git a/pretty.c b/pretty.c
index 6cb363ae1c..1e873d3f41 100644
--- a/pretty.c
+++ b/pretty.c
@@ -857,7 +857,9 @@ enum trunc_type {
 	trunc_none,
 	trunc_left,
 	trunc_middle,
-	trunc_right
+	trunc_right,
+	trunc_left_hard,
+	trunc_right_hard
 };
 
 struct format_commit_context {
@@ -1145,6 +1147,10 @@ static size_t parse_padding_placeholder(const char *placeholder,
 				c->truncate = trunc_left;
 			else if (starts_with(start, "mtrunc)"))
 				c->truncate = trunc_middle;
+			else if (starts_with(start, "Ltrunc)"))
+				c->truncate = trunc_left_hard;
+			else if (starts_with(start, "Trunc)"))
+				c->truncate = trunc_right_hard;
 			else
 				return 0;
 		} else
@@ -1743,6 +1749,16 @@ static size_t format_and_pad_commit(struct strbuf *sb, /* in UTF-8 */
 					    padding - 2, len - (padding - 2),
 					    "..");
 			break;
+		case trunc_left_hard:
+			strbuf_utf8_replace(&local_sb,
+					    0, len - padding,
+					    NULL);
+			break;
+		case trunc_right_hard:
+			strbuf_utf8_replace(&local_sb,
+					    padding, len - padding,
+					    NULL);
+			break;
 		case trunc_none:
 			break;
 		}
diff --git a/t/t4205-log-pretty-formats.sh b/t/t4205-log-pretty-formats.sh
index e448ef2928..55dca30798 100755
--- a/t/t4205-log-pretty-formats.sh
+++ b/t/t4205-log-pretty-formats.sh
@@ -261,6 +261,17 @@ test_expect_success 'left alignment formatting with trunc' '
 	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 tw
+	message on
+	add bar  Z
+	initial. a
+	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 &&
@@ -272,6 +283,17 @@ test_expect_success 'left alignment formatting with trunc. i18n.logOutputEncodin
 	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 tw
+	message on
+	add bar  Z
+	initial. a
+	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 &&
@@ -283,6 +305,18 @@ test_expect_success 'left alignment formatting with ltrunc' '
 	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 &&
+	essage two
+	essage one
+	add bar  Z
+	an${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 &&
@@ -294,6 +328,16 @@ test_expect_success 'left alignment formatting with ltrunc. i18n.logOutputEncodi
 	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 &&
+	essage two
+	essage one
+	add bar  Z
+	an${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 &&
@@ -507,6 +551,19 @@ test_expect_success 'left/right alignment formatting with stealing' '
 	EOF
 	test_cmp expected actual
 '
+
+test_expect_success 'left/right hard alignment formatting with stealing' '
+	git commit --amend -m short --author "long long long <long@me.com>" &&
+	git log --pretty="tformat:%<(10,Trunc)%s%>>(10,Ltrunc)% an" >actual &&
+	cat <<-\EOF >expected &&
+	short long  long long
+	message on   A U Thor
+	add bar      A U Thor
+	initial. a   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 &&
@@ -517,6 +574,16 @@ test_expect_success 'left/right alignment formatting with stealing. i18n.logOutp
 	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 on   A U Thor
+	add bar      A U Thor
+	initial. a   A U Thor
+	EOF
+	test_cmp expected actual
+'
 
 test_expect_success 'strbuf_utf8_replace() not producing NUL' '
 	git log --color --pretty="tformat:%<(10,trunc)%s%>>(10,ltrunc)%C(auto)%d" |
diff --git a/t/t6006-rev-list-format.sh b/t/t6006-rev-list-format.sh
index 41d0ca00b1..8dcc8000d1 100755
--- a/t/t6006-rev-list-format.sh
+++ b/t/t6006-rev-list-format.sh
@@ -218,6 +218,14 @@ commit $head1
 added (hinzugef${added_utf8_part}gt..
 EOF
 
+# ZZZ for a space?
+test_format subject-truncated "%<($truncate_count,Trunc)%s" qz_to_tab_space <<EOF
+commit $head2
+changed (ge${changed_utf8_part}ndert) f
+commit $head1
+added (hinzugef${added_utf8_part}gt)Z
+EOF
+
 test_format body %b <<EOF
 commit $head2
 commit $head1
@@ -400,6 +408,16 @@ commit $head1
 added (hinzugef${added_utf8_part_iso88591}gt..
 EOF
 
+# need qz qz_to_tab_space
+test_format complex-subject-Trunc "%<($truncate_count,Trunc)%s" qz_to_tab_space <<EOF
+commit $head3
+Test printing of com
+commit $head2
+changed (ge${changed_utf8_part_iso88591}ndert) f
+commit $head1
+added (hinzugef${added_utf8_part_iso88591}gt)Z
+EOF
+
 test_format complex-subject-mtrunc "%<($truncate_count,mtrunc)%s" <<EOF
 commit $head3
 Test prin..ex bodies
@@ -418,6 +436,14 @@ commit $head1
 .. (hinzugef${added_utf8_part_iso88591}gt) foo
 EOF
 
+test_format complex-subject-Ltrunc "%<($truncate_count,Ltrunc)%s" <<EOF
+commit $head3
+ng of complex bodies
+commit $head2
+anged (ge${changed_utf8_part_iso88591}ndert) foo
+commit $head1
+ed (hinzugef${added_utf8_part_iso88591}gt) foo
+EOF
 test_expect_success 'setup expected messages (for test %b)' '
 	cat <<-EOF >expected.utf-8 &&
 	commit $head3
@@ -455,6 +481,16 @@ commit $head1
 added (hinzugef${added_utf8_part}gt..
 EOF
 
+# need qz_to_tab_space
+test_format complex-subject-commitencoding-unset-Trunc "%<($truncate_count,Trunc)%s" qz_to_tab_space <<EOF
+commit $head3
+Test printing of com
+commit $head2
+changed (ge${changed_utf8_part}ndert) f
+commit $head1
+added (hinzugef${added_utf8_part}gt)Z
+EOF
+
 test_format complex-subject-commitencoding-unset-mtrunc "%<($truncate_count,mtrunc)%s" <<EOF
 commit $head3
 Test prin..ex bodies
@@ -473,6 +509,15 @@ commit $head1
 .. (hinzugef${added_utf8_part}gt) foo
 EOF
 
+test_format complex-subject-commitencoding-unset-Ltrunc "%<($truncate_count,Ltrunc)%s" <<EOF
+commit $head3
+ng of complex bodies
+commit $head2
+anged (ge${changed_utf8_part}ndert) foo
+commit $head1
+ed (hinzugef${added_utf8_part}gt) foo
+EOF
+
 test_format complex-body-commitencoding-unset %b <expected.utf-8
 
 test_expect_success '%x00 shows NUL' '
-- 
2.38.1.windows.1

Re: [PATCH v2 1/1] pretty-formats: add hard truncation, without ellipsis, options

[Philip Oakley]

On 01/11/2022 22:57, Philip Oakley wrote:
> Instead of replacing with "..", replace with the empty string "",
> having adjusted the padding length calculation.
>
> Needswork:
> There are no tests for these pretty formats, before or after
> this change.
>
> Signed-off-by: Philip Oakley <philipoakley@iee.email>
Doh. Too busy doing code and text fixup!'s  to notice the commit message
needed  a tweak.

It's late already so that'll be tomorrow.

Philip

Re: [PATCH v2 1/1] pretty-formats: add hard truncation, without ellipsis, options

[Taylor Blau]

On Tue, Nov 01, 2022 at 11:05:50PM +0000, Philip Oakley wrote:
> On 01/11/2022 22:57, Philip Oakley wrote:
> > Instead of replacing with "..", replace with the empty string "",
> > having adjusted the padding length calculation.
> >
> > Needswork:
> > There are no tests for these pretty formats, before or after
> > this change.
> >
> > Signed-off-by: Philip Oakley <philipoakley@iee.email>
> Doh. Too busy doing code and text fixup!'s  to notice the commit message
> needed  a tweak.
>
> It's late already so that'll be tomorrow.

It happens. I'll take the new version in the meantime, and await a
reroll from you when you have a chance.

Thanks,
Taylor

[PATCH v3] pretty-formats: add hard truncation, without ellipsis, options

[Philip Oakley]

Instead of truncating with "..", add options to replace with the
empty string, implied by passing NULL, and adjust the padding length calculation.

Extend the existing tests for these pretty formats to include
`Trunc` and Ltrunc` options matching the `trunc` and `ltrunc`
tests.

Signed-off-by: Philip Oakley <philipoakley@iee.email>
---
With updated commit message. No other changes.
---
 Documentation/pretty-formats.txt | 11 +++---
 pretty.c                         | 18 ++++++++-
 t/t4205-log-pretty-formats.sh    | 67 ++++++++++++++++++++++++++++++++
 t/t6006-rev-list-format.sh       | 45 +++++++++++++++++++++
 4 files changed, 135 insertions(+), 6 deletions(-)

diff --git a/Documentation/pretty-formats.txt b/Documentation/pretty-formats.txt
index 0b4c1c8d98..0bd339db33 100644
--- a/Documentation/pretty-formats.txt
+++ b/Documentation/pretty-formats.txt
@@ -146,14 +146,15 @@ The placeholders are:
 '%m':: left (`<`), right (`>`) or boundary (`-`) mark
 '%w([<w>[,<i1>[,<i2>]]])':: switch line wrapping, like the -w option of
 			    linkgit:git-shortlog[1].
-'%<(<N>[,trunc|ltrunc|mtrunc])':: make the next placeholder take at
+'%<(<N>[,trunc|ltrunc|mtrunc|Trunc|Ltrunc])':: make the next placeholder take at
 				  least N columns, padding spaces on
 				  the right if necessary.  Optionally
-				  truncate at the beginning (ltrunc),
-				  the middle (mtrunc) or the end
-				  (trunc) if the output is longer than
-				  N columns.  Note that truncating
+				  truncate (with ellipsis '..') at the beginning (ltrunc) `..ft`,
+				  the middle (mtrunc) `mi..le` or the end
+				  (trunc) `rig..` if the output is longer than
+				  N columns.  Note that truncating with ellipsis
 				  only works correctly with N >= 2.
+				  Use (Trunc|Ltrunc) for hard truncation without ellipsis.
 '%<|(<N>)':: make the next placeholder take at least until Nth
 	     columns, padding spaces on the right if necessary
 '%>(<N>)', '%>|(<N>)':: similar to '%<(<N>)', '%<|(<N>)' respectively,
diff --git a/pretty.c b/pretty.c
index 6cb363ae1c..1e873d3f41 100644
--- a/pretty.c
+++ b/pretty.c
@@ -857,7 +857,9 @@ enum trunc_type {
 	trunc_none,
 	trunc_left,
 	trunc_middle,
-	trunc_right
+	trunc_right,
+	trunc_left_hard,
+	trunc_right_hard
 };
 
 struct format_commit_context {
@@ -1145,6 +1147,10 @@ static size_t parse_padding_placeholder(const char *placeholder,
 				c->truncate = trunc_left;
 			else if (starts_with(start, "mtrunc)"))
 				c->truncate = trunc_middle;
+			else if (starts_with(start, "Ltrunc)"))
+				c->truncate = trunc_left_hard;
+			else if (starts_with(start, "Trunc)"))
+				c->truncate = trunc_right_hard;
 			else
 				return 0;
 		} else
@@ -1743,6 +1749,16 @@ static size_t format_and_pad_commit(struct strbuf *sb, /* in UTF-8 */
 					    padding - 2, len - (padding - 2),
 					    "..");
 			break;
+		case trunc_left_hard:
+			strbuf_utf8_replace(&local_sb,
+					    0, len - padding,
+					    NULL);
+			break;
+		case trunc_right_hard:
+			strbuf_utf8_replace(&local_sb,
+					    padding, len - padding,
+					    NULL);
+			break;
 		case trunc_none:
 			break;
 		}
diff --git a/t/t4205-log-pretty-formats.sh b/t/t4205-log-pretty-formats.sh
index e448ef2928..55dca30798 100755
--- a/t/t4205-log-pretty-formats.sh
+++ b/t/t4205-log-pretty-formats.sh
@@ -261,6 +261,17 @@ test_expect_success 'left alignment formatting with trunc' '
 	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 tw
+	message on
+	add bar  Z
+	initial. a
+	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 &&
@@ -272,6 +283,17 @@ test_expect_success 'left alignment formatting with trunc. i18n.logOutputEncodin
 	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 tw
+	message on
+	add bar  Z
+	initial. a
+	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 &&
@@ -283,6 +305,18 @@ test_expect_success 'left alignment formatting with ltrunc' '
 	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 &&
+	essage two
+	essage one
+	add bar  Z
+	an${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 &&
@@ -294,6 +328,16 @@ test_expect_success 'left alignment formatting with ltrunc. i18n.logOutputEncodi
 	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 &&
+	essage two
+	essage one
+	add bar  Z
+	an${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 &&
@@ -507,6 +551,19 @@ test_expect_success 'left/right alignment formatting with stealing' '
 	EOF
 	test_cmp expected actual
 '
+
+test_expect_success 'left/right hard alignment formatting with stealing' '
+	git commit --amend -m short --author "long long long <long@me.com>" &&
+	git log --pretty="tformat:%<(10,Trunc)%s%>>(10,Ltrunc)% an" >actual &&
+	cat <<-\EOF >expected &&
+	short long  long long
+	message on   A U Thor
+	add bar      A U Thor
+	initial. a   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 &&
@@ -517,6 +574,16 @@ test_expect_success 'left/right alignment formatting with stealing. i18n.logOutp
 	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 on   A U Thor
+	add bar      A U Thor
+	initial. a   A U Thor
+	EOF
+	test_cmp expected actual
+'
 
 test_expect_success 'strbuf_utf8_replace() not producing NUL' '
 	git log --color --pretty="tformat:%<(10,trunc)%s%>>(10,ltrunc)%C(auto)%d" |
diff --git a/t/t6006-rev-list-format.sh b/t/t6006-rev-list-format.sh
index 41d0ca00b1..8dcc8000d1 100755
--- a/t/t6006-rev-list-format.sh
+++ b/t/t6006-rev-list-format.sh
@@ -218,6 +218,14 @@ commit $head1
 added (hinzugef${added_utf8_part}gt..
 EOF
 
+# ZZZ for a space?
+test_format subject-truncated "%<($truncate_count,Trunc)%s" qz_to_tab_space <<EOF
+commit $head2
+changed (ge${changed_utf8_part}ndert) f
+commit $head1
+added (hinzugef${added_utf8_part}gt)Z
+EOF
+
 test_format body %b <<EOF
 commit $head2
 commit $head1
@@ -400,6 +408,16 @@ commit $head1
 added (hinzugef${added_utf8_part_iso88591}gt..
 EOF
 
+# need qz qz_to_tab_space
+test_format complex-subject-Trunc "%<($truncate_count,Trunc)%s" qz_to_tab_space <<EOF
+commit $head3
+Test printing of com
+commit $head2
+changed (ge${changed_utf8_part_iso88591}ndert) f
+commit $head1
+added (hinzugef${added_utf8_part_iso88591}gt)Z
+EOF
+
 test_format complex-subject-mtrunc "%<($truncate_count,mtrunc)%s" <<EOF
 commit $head3
 Test prin..ex bodies
@@ -418,6 +436,14 @@ commit $head1
 .. (hinzugef${added_utf8_part_iso88591}gt) foo
 EOF
 
+test_format complex-subject-Ltrunc "%<($truncate_count,Ltrunc)%s" <<EOF
+commit $head3
+ng of complex bodies
+commit $head2
+anged (ge${changed_utf8_part_iso88591}ndert) foo
+commit $head1
+ed (hinzugef${added_utf8_part_iso88591}gt) foo
+EOF
 test_expect_success 'setup expected messages (for test %b)' '
 	cat <<-EOF >expected.utf-8 &&
 	commit $head3
@@ -455,6 +481,16 @@ commit $head1
 added (hinzugef${added_utf8_part}gt..
 EOF
 
+# need qz_to_tab_space
+test_format complex-subject-commitencoding-unset-Trunc "%<($truncate_count,Trunc)%s" qz_to_tab_space <<EOF
+commit $head3
+Test printing of com
+commit $head2
+changed (ge${changed_utf8_part}ndert) f
+commit $head1
+added (hinzugef${added_utf8_part}gt)Z
+EOF
+
 test_format complex-subject-commitencoding-unset-mtrunc "%<($truncate_count,mtrunc)%s" <<EOF
 commit $head3
 Test prin..ex bodies
@@ -473,6 +509,15 @@ commit $head1
 .. (hinzugef${added_utf8_part}gt) foo
 EOF
 
+test_format complex-subject-commitencoding-unset-Ltrunc "%<($truncate_count,Ltrunc)%s" <<EOF
+commit $head3
+ng of complex bodies
+commit $head2
+anged (ge${changed_utf8_part}ndert) foo
+commit $head1
+ed (hinzugef${added_utf8_part}gt) foo
+EOF
+
 test_format complex-body-commitencoding-unset %b <expected.utf-8
 
 test_expect_success '%x00 shows NUL' '
-- 
2.38.1.281.g83ef3ded8d

[PATCH v4] pretty-formats: add hard truncation, without ellipsis, options

[Philip Oakley]

Instead of replacing with "..", replace with the empty string,
implied by passing NULL, and adjust the padding length calculation.

Extend the existing tests for these pretty formats to include
`Trunc` and Ltrunc` options matching the `trunc` and `ltrunc`
tests.

Signed-off-by: Philip Oakley <philipoakley@iee.email>
---

Removed left over comments from locations that had needed QZ 'space'
conversion in the here-docs.

To: GitList <git@vger.kernel.org>
Cc: Taylor Blau <me@ttaylorr.com>
Cc: Junio C Hamano <gitster@pobox.com>
Cc: NSENGIYUMVA WILBERFORCE <nsengiyumvawilberforce@gmail.com>
In-reply-to: <20221102120853.2013-1-philipoakley@iee.email> 

This should complete the patch (cf. [1] <Y2rPAGp96IwrLS1T@nand.local>)
Note: latest What's Cooking <Y2riRSL+NprJt278@nand.local> hadn't been updated. Tests are included.

A final review would be welcomed.

Range-diff against v3:
1:  498439f0375 ! 1:  d26dabc5271 pretty-formats: add hard truncation, without ellipsis, options
    @@ t/t6006-rev-list-format.sh: commit $head1
      added (hinzugef${added_utf8_part}gt..
      EOF
      
    -+# ZZZ for a space?
     +test_format subject-truncated "%<($truncate_count,Trunc)%s" qz_to_tab_space <<EOF
     +commit $head2
     +changed (ge${changed_utf8_part}ndert) f
    @@ t/t6006-rev-list-format.sh: commit $head1
      added (hinzugef${added_utf8_part_iso88591}gt..
      EOF
      
    -+# need qz qz_to_tab_space
     +test_format complex-subject-Trunc "%<($truncate_count,Trunc)%s" qz_to_tab_space <<EOF
     +commit $head3
     +Test printing of com
    @@ t/t6006-rev-list-format.sh: commit $head1
      added (hinzugef${added_utf8_part}gt..
      EOF
      
    -+# need qz_to_tab_space
     +test_format complex-subject-commitencoding-unset-Trunc "%<($truncate_count,Trunc)%s" qz_to_tab_space <<EOF
     +commit $head3
     +Test printing of com

 Documentation/pretty-formats.txt | 11 +++---
 pretty.c                         | 18 ++++++++-
 t/t4205-log-pretty-formats.sh    | 67 ++++++++++++++++++++++++++++++++
 t/t6006-rev-list-format.sh       | 42 ++++++++++++++++++++
 4 files changed, 132 insertions(+), 6 deletions(-)

diff --git a/Documentation/pretty-formats.txt b/Documentation/pretty-formats.txt
index 0b4c1c8d98a..0bd339db333 100644
--- a/Documentation/pretty-formats.txt
+++ b/Documentation/pretty-formats.txt
@@ -146,14 +146,15 @@ The placeholders are:
 '%m':: left (`<`), right (`>`) or boundary (`-`) mark
 '%w([<w>[,<i1>[,<i2>]]])':: switch line wrapping, like the -w option of
 			    linkgit:git-shortlog[1].
-'%<(<N>[,trunc|ltrunc|mtrunc])':: make the next placeholder take at
+'%<(<N>[,trunc|ltrunc|mtrunc|Trunc|Ltrunc])':: make the next placeholder take at
 				  least N columns, padding spaces on
 				  the right if necessary.  Optionally
-				  truncate at the beginning (ltrunc),
-				  the middle (mtrunc) or the end
-				  (trunc) if the output is longer than
-				  N columns.  Note that truncating
+				  truncate (with ellipsis '..') at the beginning (ltrunc) `..ft`,
+				  the middle (mtrunc) `mi..le` or the end
+				  (trunc) `rig..` if the output is longer than
+				  N columns.  Note that truncating with ellipsis
 				  only works correctly with N >= 2.
+				  Use (Trunc|Ltrunc) for hard truncation without ellipsis.
 '%<|(<N>)':: make the next placeholder take at least until Nth
 	     columns, padding spaces on the right if necessary
 '%>(<N>)', '%>|(<N>)':: similar to '%<(<N>)', '%<|(<N>)' respectively,
diff --git a/pretty.c b/pretty.c
index 6cb363ae1c9..1e873d3f41a 100644
--- a/pretty.c
+++ b/pretty.c
@@ -857,7 +857,9 @@ enum trunc_type {
 	trunc_none,
 	trunc_left,
 	trunc_middle,
-	trunc_right
+	trunc_right,
+	trunc_left_hard,
+	trunc_right_hard
 };
 
 struct format_commit_context {
@@ -1145,6 +1147,10 @@ static size_t parse_padding_placeholder(const char *placeholder,
 				c->truncate = trunc_left;
 			else if (starts_with(start, "mtrunc)"))
 				c->truncate = trunc_middle;
+			else if (starts_with(start, "Ltrunc)"))
+				c->truncate = trunc_left_hard;
+			else if (starts_with(start, "Trunc)"))
+				c->truncate = trunc_right_hard;
 			else
 				return 0;
 		} else
@@ -1743,6 +1749,16 @@ static size_t format_and_pad_commit(struct strbuf *sb, /* in UTF-8 */
 					    padding - 2, len - (padding - 2),
 					    "..");
 			break;
+		case trunc_left_hard:
+			strbuf_utf8_replace(&local_sb,
+					    0, len - padding,
+					    NULL);
+			break;
+		case trunc_right_hard:
+			strbuf_utf8_replace(&local_sb,
+					    padding, len - padding,
+					    NULL);
+			break;
 		case trunc_none:
 			break;
 		}
diff --git a/t/t4205-log-pretty-formats.sh b/t/t4205-log-pretty-formats.sh
index e448ef2928a..55dca307983 100755
--- a/t/t4205-log-pretty-formats.sh
+++ b/t/t4205-log-pretty-formats.sh
@@ -261,6 +261,17 @@ test_expect_success 'left alignment formatting with trunc' '
 	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 tw
+	message on
+	add bar  Z
+	initial. a
+	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 &&
@@ -272,6 +283,17 @@ test_expect_success 'left alignment formatting with trunc. i18n.logOutputEncodin
 	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 tw
+	message on
+	add bar  Z
+	initial. a
+	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 &&
@@ -283,6 +305,18 @@ test_expect_success 'left alignment formatting with ltrunc' '
 	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 &&
+	essage two
+	essage one
+	add bar  Z
+	an${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 &&
@@ -294,6 +328,16 @@ test_expect_success 'left alignment formatting with ltrunc. i18n.logOutputEncodi
 	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 &&
+	essage two
+	essage one
+	add bar  Z
+	an${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 &&
@@ -507,6 +551,19 @@ test_expect_success 'left/right alignment formatting with stealing' '
 	EOF
 	test_cmp expected actual
 '
+
+test_expect_success 'left/right hard alignment formatting with stealing' '
+	git commit --amend -m short --author "long long long <long@me.com>" &&
+	git log --pretty="tformat:%<(10,Trunc)%s%>>(10,Ltrunc)% an" >actual &&
+	cat <<-\EOF >expected &&
+	short long  long long
+	message on   A U Thor
+	add bar      A U Thor
+	initial. a   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 &&
@@ -517,6 +574,16 @@ test_expect_success 'left/right alignment formatting with stealing. i18n.logOutp
 	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 on   A U Thor
+	add bar      A U Thor
+	initial. a   A U Thor
+	EOF
+	test_cmp expected actual
+'
 
 test_expect_success 'strbuf_utf8_replace() not producing NUL' '
 	git log --color --pretty="tformat:%<(10,trunc)%s%>>(10,ltrunc)%C(auto)%d" |
diff --git a/t/t6006-rev-list-format.sh b/t/t6006-rev-list-format.sh
index 41d0ca00b1c..c44935b0ab4 100755
--- a/t/t6006-rev-list-format.sh
+++ b/t/t6006-rev-list-format.sh
@@ -218,6 +218,13 @@ commit $head1
 added (hinzugef${added_utf8_part}gt..
 EOF
 
+test_format subject-truncated "%<($truncate_count,Trunc)%s" qz_to_tab_space <<EOF
+commit $head2
+changed (ge${changed_utf8_part}ndert) f
+commit $head1
+added (hinzugef${added_utf8_part}gt)Z
+EOF
+
 test_format body %b <<EOF
 commit $head2
 commit $head1
@@ -400,6 +407,15 @@ commit $head1
 added (hinzugef${added_utf8_part_iso88591}gt..
 EOF
 
+test_format complex-subject-Trunc "%<($truncate_count,Trunc)%s" qz_to_tab_space <<EOF
+commit $head3
+Test printing of com
+commit $head2
+changed (ge${changed_utf8_part_iso88591}ndert) f
+commit $head1
+added (hinzugef${added_utf8_part_iso88591}gt)Z
+EOF
+
 test_format complex-subject-mtrunc "%<($truncate_count,mtrunc)%s" <<EOF
 commit $head3
 Test prin..ex bodies
@@ -418,6 +434,14 @@ commit $head1
 .. (hinzugef${added_utf8_part_iso88591}gt) foo
 EOF
 
+test_format complex-subject-Ltrunc "%<($truncate_count,Ltrunc)%s" <<EOF
+commit $head3
+ng of complex bodies
+commit $head2
+anged (ge${changed_utf8_part_iso88591}ndert) foo
+commit $head1
+ed (hinzugef${added_utf8_part_iso88591}gt) foo
+EOF
 test_expect_success 'setup expected messages (for test %b)' '
 	cat <<-EOF >expected.utf-8 &&
 	commit $head3
@@ -455,6 +479,15 @@ commit $head1
 added (hinzugef${added_utf8_part}gt..
 EOF
 
+test_format complex-subject-commitencoding-unset-Trunc "%<($truncate_count,Trunc)%s" qz_to_tab_space <<EOF
+commit $head3
+Test printing of com
+commit $head2
+changed (ge${changed_utf8_part}ndert) f
+commit $head1
+added (hinzugef${added_utf8_part}gt)Z
+EOF
+
 test_format complex-subject-commitencoding-unset-mtrunc "%<($truncate_count,mtrunc)%s" <<EOF
 commit $head3
 Test prin..ex bodies
@@ -473,6 +506,15 @@ commit $head1
 .. (hinzugef${added_utf8_part}gt) foo
 EOF
 
+test_format complex-subject-commitencoding-unset-Ltrunc "%<($truncate_count,Ltrunc)%s" <<EOF
+commit $head3
+ng of complex bodies
+commit $head2
+anged (ge${changed_utf8_part}ndert) foo
+commit $head1
+ed (hinzugef${added_utf8_part}gt) foo
+EOF
+
 test_format complex-body-commitencoding-unset %b <expected.utf-8
 
 test_expect_success '%x00 shows NUL' '
-- 
2.38.1.windows.1

Re: [PATCH v4] pretty-formats: add hard truncation, without ellipsis, options

[Junio C Hamano]

Philip Oakley <philipoakley@iee.email> writes:

> Instead of replacing with "..", replace with the empty string,
> implied by passing NULL, and adjust the padding length calculation.

What's the point of saying "implied by passing NULL" here?  Is it an
excuse for passing NULL when passing "" would have sufficed and been
more natural, or something?  Also, it is unclear to whom you are
passing the NULL.  I think that it is sufficient that you said
"replace with the empty string" there.

> Extend the existing tests for these pretty formats to include
> `Trunc` and Ltrunc` options matching the `trunc` and `ltrunc`
> tests.

A more important thing to say is that we add Trunc and Ltrunc, than
we test for these new features ;-)

You may also want to explain why there is no matching Mtrunc added.

I also have another comment on the design.

Imagine there are series of wide characters, each occupying two
display columns, and you give 6 display columns to truncate such a
string into.  "trunc" would give you "[][].." (where [] denotes one
such wide letter that occupies two display columns), and "Trunc"
would give you "[][][]".  Now if you give only 5 display columns,
to fill instead of 6, what should happen?

I do not recall how ".."-stuffed truncation works in this case but
it should notice that it cannot stuff 3 wide letters and give you
"[][].".  The current code may be already buggy, but at least at the
design level, it is fairly clear what the feature _should_ do.

As a design question, what should "Trunc" do in such a case now?  I
do not think we can still call it "hard truncate" if the feature
gives "[][]" (i.e. fill only 4 display columns, resulting in a
string that is not wide enough) or "[][][]" (i.e. exceed 5 columns
that are given), but of course chomping a letter in the middle is
not acceptable behaviour, so ...

Re: [PATCH v4] pretty-formats: add hard truncation, without ellipsis, options

[Philip Oakley]

Hi Junio

On 21/11/2022 00:34, Junio C Hamano wrote:
> Philip Oakley <philipoakley@iee.email> writes:
>
>> Instead of replacing with "..", replace with the empty string,
>> implied by passing NULL, and adjust the padding length calculation.
> What's the point of saying "implied by passing NULL" here?  Is it an
> excuse for passing NULL when passing "" would have sufficed and been
> more natural, or something?  

Passing the empty string was my first approach, however Taylor had
commented "why pass the empty string, when NULL will do", hence I
checked, and yes, we can pass NULL, so I followed that guidance on the
re-roll.

> Also, it is unclear to whom you are
> passing the NULL.  I think that it is sufficient that you said
> "replace with the empty string" there.

I could drop the commit message comment, and keep the NULL being passed
tostrbuf_utf8_replace to indicate the empty string, though that may
create the same reviewer question that Taylor had.
>
>> Extend the existing tests for these pretty formats to include
>> `Trunc` and Ltrunc` options matching the `trunc` and `ltrunc`
>> tests.
> A more important thing to say is that we add Trunc and Ltrunc, than
> we test for these new features ;-)

That would be to swap the paragraphs about, yes?
>
> You may also want to explain why there is no matching Mtrunc added.

Can do, though it felt obvious that the original mtrunc ellipsis would
be necessary for the mid-case.
>
> I also have another comment on the design.
>
> Imagine there are series of wide characters, each occupying two
> display columns, and you give 6 display columns to truncate such a
> string into.  "trunc" would give you "[][].." (where [] denotes one
> such wide letter that occupies two display columns), and "Trunc"
> would give you "[][][]".  Now if you give only 5 display columns,
> to fill instead of 6, what should happen?

My reading of the existing code for ltrunc/mtrunc/trunc was that all
these padding conditions were already covered. It was simply a matter of
column counting.
>
> I do not recall how ".."-stuffed truncation works in this case but
> it should notice that it cannot stuff 3 wide letters and give you
> "[][].".  The current code may be already buggy, but at least at the
> design level, it is fairly clear what the feature _should_ do.
>
> As a design question, what should "Trunc" do in such a case now?  I
> do not think we can still call it "hard truncate" if the feature
> gives "[][]" (i.e. fill only 4 display columns, resulting in a
> string that is not wide enough) or "[][][]" (i.e. exceed 5 columns
> that are given), but of course chomping a letter in the middle is
> not acceptable behaviour, so ...
The design had already covered those cases. The author already had those
thoughts

--

Philip

Re: [PATCH v4] pretty-formats: add hard truncation, without ellipsis, options

[Junio C Hamano]

Philip Oakley <philipoakley@iee.email> writes:

>> As a design question, what should "Trunc" do in such a case now?  I
>> do not think we can still call it "hard truncate" if the feature
>> gives "[][]" (i.e. fill only 4 display columns, resulting in a
>> string that is not wide enough) or "[][][]" (i.e. exceed 5 columns
>> that are given), but of course chomping a letter in the middle is
>> not acceptable behaviour, so ...
> The design had already covered those cases. The author already had those
> thoughts

Sorry, I was saying that none of

 * giving only [][] to fill only 4 display columns, without filling
   the given 5 display columns,

 * giving [][][] to fill 6 display columns, exceeding the given 5
   display columns,

 * giving [][][ that chomps a letter in the middle, in a failed
   attempt to fill exactly 5 displya columns.

would be a sensible design of the behaviour for "Trunc", so I am not
sure what "had already covered" really mean...

Re: [PATCH v4] pretty-formats: add hard truncation, without ellipsis, options

[Philip Oakley]

On 22/11/2022 00:57, Junio C Hamano wrote:
> Philip Oakley <philipoakley@iee.email> writes:
>
>>> As a design question, what should "Trunc" do in such a case now?  I
>>> do not think we can still call it "hard truncate" if the feature
>>> gives "[][]" (i.e. fill only 4 display columns, resulting in a
>>> string that is not wide enough) or "[][][]" (i.e. exceed 5 columns
>>> that are given), but of course chomping a letter in the middle is
>>> not acceptable behaviour, so ...
>> The design had already covered those cases. The author already had those
>> thoughts
> Sorry, I was saying that none of
>
>  * giving only [][] to fill only 4 display columns, without filling
>    the given 5 display columns,
>
>  * giving [][][] to fill 6 display columns, exceeding the given 5
>    display columns,
>
>  * giving [][][ that chomps a letter in the middle, in a failed
>    attempt to fill exactly 5 displya columns.
>
> would be a sensible design of the behaviour for "Trunc", so I am not
> sure what "had already covered" really mean...
>
I'm still unsure what you are trying to say here.

Is this a question about the prior `trunc`, `mtrunc`, and `ltrunc`
design and tests?
e.g. how complete are their tests?

Is it looking for an extended explanation of the 'fit in N-columns'
design approaches that they all have?

I'm happy to add a paragraph saying that an `Mtrunc`, i.e. miss out the
middle characters to fit the set column width, without using the two dot
ellipsis (`..`), was considered a non starter because of potential
confusion when looking at such output when tabulated.

The existing code (and tests) already covers the need to hide the
characters those two dots (ellipsis) consumed win the N-column tabulated
output. The tests also include utf8-encoded characters.

All the previous tests for `trunc` and `'ltrunc` (i.e. with ellipsis)
have been repeated for the `Trunc` and `Ltrunc` (without ellipsis) hard
truncation commands, and their expected outputs updated, including the
use of the qz_to_tab_space  for those case where trailing spaces are now
present.

Does that cover your questions?

--
Philip

Re: [PATCH v4] pretty-formats: add hard truncation, without ellipsis, options

[Junio C Hamano]

Philip Oakley <philipoakley@iee.email> writes:

> On 22/11/2022 00:57, Junio C Hamano wrote:
>> Philip Oakley <philipoakley@iee.email> writes:
>>
>>>> As a design question, what should "Trunc" do in such a case now?  I
>>>> do not think we can still call it "hard truncate" if the feature
>>>> gives "[][]" (i.e. fill only 4 display columns, resulting in a
>>>> string that is not wide enough) or "[][][]" (i.e. exceed 5 columns
>>>> that are given), but of course chomping a letter in the middle is
>>>> not acceptable behaviour, so ...
>>> The design had already covered those cases. The author already had those
>>> thoughts
>> Sorry, I was saying that none of
>>
>>  * giving only [][] to fill only 4 display columns, without filling
>>    the given 5 display columns,
>>
>>  * giving [][][] to fill 6 display columns, exceeding the given 5
>>    display columns,
>>
>>  * giving [][][ that chomps a letter in the middle, in a failed
>>    attempt to fill exactly 5 displya columns.
>>
>> would be a sensible design of the behaviour for "Trunc", so I am not
>> sure what "had already covered" really mean...
>>
> I'm still unsure what you are trying to say here.
>
> Is this a question about the prior `trunc`, `mtrunc`, and `ltrunc`
> design and tests?
> e.g. how complete are their tests?

No.  As I said, the existing lowercase ones may already be buggy (I
didn't check), in that they may do "[][].." or "[][][]" when told to
"trunc" fill a string with four or more double-width letters into a
5 display space.  But the point is at least for these with ellipsis
it is fairly clear what the desired behaviour is.  For "trunc" in
the above example, I think the right thing for it to do would be to
do "[][].", i.e. consume exactly 5 display columns, and avoid
exceeding the given space by not giving two dots but just one.

But with "hard truncate", I do not think we can define any sensible
behaviour when we ask it to do the same.  Giving "[][]" leaves one
display space unconsumed and giving "[][][]" would exceed the given
space, so anything you would write after that would be unaligned on
the line.

As to the tests, the question was, whatever the designed behaviour
for the above case, if they record the design choice made by this
series (even though, as I said, I suspect no design choice for the
"hard-fill/trunc odd number of columns with a run of double-width
letters" problem is satisfactory).

Re: [PATCH v4] pretty-formats: add hard truncation, without ellipsis, options

[Philip Oakley]

On 25/11/2022 07:11, Junio C Hamano wrote:
> Philip Oakley <philipoakley@iee.email> writes:
>
>> On 22/11/2022 00:57, Junio C Hamano wrote:
>>> Philip Oakley <philipoakley@iee.email> writes:
>>>
>>>>> As a design question, what should "Trunc" do in such a case now?  I
>>>>> do not think we can still call it "hard truncate" if the feature
>>>>> gives "[][]" (i.e. fill only 4 display columns, resulting in a
>>>>> string that is not wide enough) or "[][][]" (i.e. exceed 5 columns
>>>>> that are given), but of course chomping a letter in the middle is
>>>>> not acceptable behaviour, so ...
>>>> The design had already covered those cases. The author already had those
>>>> thoughts
>>> Sorry, I was saying that none of
>>>
>>>  * giving only [][] to fill only 4 display columns, without filling
>>>    the given 5 display columns,
>>>
>>>  * giving [][][] to fill 6 display columns, exceeding the given 5
>>>    display columns,
>>>
>>>  * giving [][][ that chomps a letter in the middle, in a failed
>>>    attempt to fill exactly 5 displya columns.
>>>
>>> would be a sensible design of the behaviour for "Trunc", so I am not
>>> sure what "had already covered" really mean...
>>>
>> I'm still unsure what you are trying to say here.
>>
>> Is this a question about the prior `trunc`, `mtrunc`, and `ltrunc`
>> design and tests?
>> e.g. how complete are their tests?
> No.  As I said, the existing lowercase ones may already be buggy (I
> didn't check),

That question hadn't come across in the previous emails.

>  in that they may do "[][].." or "[][][]" when told to
> "trunc" fill a string with four or more double-width letters into a
> 5 display space.  But the point is at least for these with ellipsis
> it is fairly clear what the desired behaviour is.

That "is fairly clear" is probably the problem. In retrospect it's not
clear in the docs that the "%<(N" format is (would appear to be) about
defining the display width, in terminal character columns, that the
selected parameter is to be displayed within.

The code already pads the displayed parameter with spaces as required if
the parameter is shorter than the display width - the else condition in
pretty.c L1750

>   For "trunc" in
> the above example, I think the right thing for it to do would be to
> do "[][].", i.e. consume exactly 5 display columns, and avoid
> exceeding the given space by not giving two dots but just one.

The existing choice is padding "[][]" with a single space to reach 5
display chars.
For the 6-char "[][][]" truncation it is "[][..", i.e. 3 chars from
"[][][]", then the two ".." dots of the ellipsis.
>
> But with "hard truncate", I do not think we can define any sensible
> behaviour 

Given the existing code and the display column expectation, it felt
rather simple to apply the hard cut at the display width, or pad with
spaces if the chars to display were shorter than the allocated columns.

> when we ask it to do the same.  Giving "[][]" leaves one
> display space unconsumed and giving "[][][]" would exceed the given
> space, so anything you would write after that would be unaligned on
> the line.

A careful read (and testing) of the existing 'mtrunc' does show a
rounding error bug though. Its a confusion between the computed start
and end points of the cut that loses one display column (the string
displayed is short by one when the count is odd, IIUC).

>
> As to the tests, the question was, whatever the designed behaviour
> for the above case, if they record the design choice made by this
> series (even though, as I said, I suspect no design choice for the
> "hard-fill/trunc odd number of columns with a run of double-width
> letters" problem is satisfactory).

The existing tests already cover the trailing space padding issues.
However I'd agree that the documentation (and original commit messages
for the existing code) do not make clear the distinction between display
columns to be filled and characters in the displayed parameters (aka
'placeholders').

Within that, there is also the "%<(N" and "%<|(M" lack of clarity, where
the N count is for a single place holder's character count, while the M
value is the terminal's display column number, of the 24x80 column style.

Finally, the docs use of '<N>'  in the middle the place holders title
that also use `<` and `>` is a readability nightmare. Given that the
text then only talks about `N` it may be sensible to drop the
surrounding `<.>` to reduce confusion for these alignment placeholders.

--

Philip

Re: [PATCH v4] pretty-formats: add hard truncation, without ellipsis, options

[Philip Oakley]

On 26/11/2022 14:32, Philip Oakley wrote:
> A careful read (and testing) of the existing 'mtrunc' does show a
> rounding error bug though. Its a confusion between the computed start
> and end points of the cut that loses one display column (the string
> displayed is short by one when the count is odd, IIUC).

I had confused myself.

My test case `git log --graph --format="%<(19,mtrunc)%d=2" -6` had a
2-char step in the graph that I confused with the effects between
repeated runs with the 'N' value adjusted by +1 and -1.

I then jumped to conclusions about the integer division in the mid
string position of the mtrunc case win the code.

Sorry for that.

Re: [PATCH v4] pretty-formats: add hard truncation, without ellipsis, options

[Junio C Hamano]

Philip Oakley <philipoakley@iee.email> writes:

>>  in that they may do "[][].." or "[][][]" when told to
>> "trunc" fill a string with four or more double-width letters into a
>> 5 display space.  But the point is at least for these with ellipsis
>> it is fairly clear what the desired behaviour is.
>
> That "is fairly clear" is probably the problem. In retrospect it's not
> clear in the docs that the "%<(N" format is (would appear to be) about
> defining the display width, in terminal character columns, that the
> selected parameter is to be displayed within.
>
> The code already pads the displayed parameter with spaces as required if
> the parameter is shorter than the display width - the else condition in
> pretty.c L1750
>
>>   For "trunc" in
>> the above example, I think the right thing for it to do would be to
>> do "[][].", i.e. consume exactly 5 display columns, and avoid
>> exceeding the given space by not giving two dots but just one.
>
> The existing choice is padding "[][]" with a single space to reach 5
> display chars.
> For the 6-char "[][][]" truncation it is "[][..", i.e. 3 chars from
> "[][][]", then the two ".." dots of the ellipsis.

Here, I realize that I did not explain the scenario well.  The
message you are responding to was meant to be a clarification of my
earlier message and it should have done a better job but apparently
I failed.  Sorry, and let me try again.

The single example I meant to use to illustrate the scenario I worry
about is this.  There is a string, in which there are four (or more)
letters, each of which occupies two display columns.  And '[]' in my
earlier messages stood for a SINGLE such letter (I just wanted to
stick to ASCII, instead of using East Asian script, for
illustration).  So "[][.." is not possible (you are chomping the
second such letter in half).

I could use East Asian 一二三四 (there are four letters, denoting
one, two, three, and four, each occupying two display spaces when
typeset in a fixed width font), but to make it easier to see in
ASCII only text, let's pretend "[1]", "[2]", "[3]", "[4]" are such
letters.  You cannot chomp them in the middle (and please pretend
each of them occupy two, not three, display spaces).

When the given display space is 6 columns, we can fit 2 such letters
plus ".." in the space.  If the original string were [1][2][3][4],
it is clear trunk and ltrunk can do "[1][2].." (remember [n] stands
for a single letter whose width is 2 columns, so that takes 6
columns) and "..[3][4]", respectively.  It also is clear that Trunk
and Ltrunk can do "[1][2][3]" and "[2][3][4]", respectively.  We
truncate the given string so that we fill the alloted display
columns fully.

If the given display space is 5 columns, the desirable behaviour for
trunk and ltrunk is still clear.  Instead of consuming two dots, we
could use a single dot as the filler.  As I said, I suspect that the
implementation of trunk and ltrunc does this correctly, though.

My worry is it is not clear what Trunk and Ltrunk should do in that
case.  There is no way to fit a substring of [1][2][3][4] into 5
columns without any filler.

Re: [PATCH v4] pretty-formats: add hard truncation, without ellipsis, options

[Philip Oakley]

On 26/11/2022 23:19, Junio C Hamano wrote:
> Philip Oakley <philipoakley@iee.email> writes:
>
>>>  in that they may do "[][].." or "[][][]" when told to
>>> "trunc" fill a string with four or more double-width letters into a
>>> 5 display space.  But the point is at least for these with ellipsis
>>> it is fairly clear what the desired behaviour is.
>> That "is fairly clear" is probably the problem. In retrospect it's not
>> clear in the docs that the "%<(N" format is (would appear to be) about
>> defining the display width, in terminal character columns, that the
>> selected parameter is to be displayed within.
>>
>> The code already pads the displayed parameter with spaces as required if
>> the parameter is shorter than the display width - the else condition in
>> pretty.c L1750
>>
>>>   For "trunc" in
>>> the above example, I think the right thing for it to do would be to
>>> do "[][].", i.e. consume exactly 5 display columns, and avoid
>>> exceeding the given space by not giving two dots but just one.
>> The existing choice is padding "[][]" with a single space to reach 5
>> display chars.
>> For the 6-char "[][][]" truncation it is "[][..", i.e. 3 chars from
>> "[][][]", then the two ".." dots of the ellipsis.
> Here, I realize that I did not explain the scenario well.  The
> message you are responding to was meant to be a clarification of my
> earlier message and it should have done a better job but apparently
> I failed.  Sorry, and let me try again.
>
> The single example I meant to use to illustrate the scenario I worry
> about is this.  There is a string, in which there are four (or more)
> letters, each of which occupies two display columns.  And '[]' in my
> earlier messages stood for a SINGLE such letter (I just wanted to
> stick to ASCII, instead of using East Asian script, for
> illustration).  So "[][.." is not possible (you are chomping the
> second such letter in half).
>
> I could use East Asian 一二三四 (there are four letters, denoting
> one, two, three, and four, each occupying two display spaces when
> typeset in a fixed width font),

Thanks for that clarification, I'd been thinking it was about c char
(bytes) such as ASCII and multi-byte characters (code points), e.g.
European umlaut style distinctions.

I hadn't really picked up on the distinction between wide and narrow
'glyphs' (if that's the right term to use).
 
I see that the code does properly count the widths of narrow and wide
code points as 1 and 2 columns of the display, but then doesn't
explicitly try any adjustment for the wide code point problem you noted.
>  but to make it easier to see in
> ASCII only text, let's pretend "[1]", "[2]", "[3]", "[4]" are such
> letters.  You cannot chomp them in the middle (and please pretend
> each of them occupy two, not three, display spaces).
>
> When the given display space is 6 columns, we can fit 2 such letters
> plus ".." in the space.  If the original string were [1][2][3][4],
> it is clear trunk and ltrunk can do "[1][2].." (remember [n] stands
> for a single letter whose width is 2 columns, so that takes 6
> columns) and "..[3][4]", respectively.  It also is clear that Trunk
> and Ltrunk can do "[1][2][3]" and "[2][3][4]", respectively.  We
> truncate the given string so that we fill the alloted display
> columns fully.
>
> If the given display space is 5 columns, the desirable behaviour for
> trunk and ltrunk is still clear.  Instead of consuming two dots, we
> could use a single dot as the filler.  As I said, I suspect that the
> implementation of trunk and ltrunc does this correctly, though.

I believe there is a possible solution that, if we detect a column
over-run, then we can go back and replace the current two column double
dot with a narrow U+2026 Horizontal ellipsis, to regain the needed column.
>
> My worry is it is not clear what Trunk and Ltrunk should do in that
> case.  There is no way to fit a substring of [1][2][3][4] into 5
> columns without any filler.
For this case where the final code point overruns, my solution
could/would be to use the Vertical ellipsis U+22EE "⋮" to re-write that
final character (though the Unicode Replacement Character "�" could be
used, but that's ugly)

I suspect the code would need some close reading to ensure that the
column counting and replacement would correctly cope with the 'off by
one' wide width case inside the strbuf_utf8_replace().

I.e. given the same off-by-one position and replacement length, get back
to the same point to replace either the double dot or the final code
point in an idempotent manner.

The logic feels sound, as long as there are no three wide crocodile
code-points. Either we counted the right number of columns, or we
over-ran by one, so we go back and substitute with a one-for-two
replacement.

Philip

For watchers, https://github.com/microsoft/terminal/issues/4345 shows
some of the issues in the general case.

Re: [PATCH v4] pretty-formats: add hard truncation, without ellipsis, options

[Junio C Hamano]

Philip Oakley <philipoakley@iee.email> writes:

>> If the given display space is 5 columns, the desirable behaviour for
>> trunk and ltrunk is still clear.  Instead of consuming two dots, we
>> could use a single dot as the filler.  As I said, I suspect that the
>> implementation of trunk and ltrunc does this correctly, though.
>
> I believe there is a possible solution that, if we detect a column
> over-run, then we can go back and replace the current two column double
> dot with a narrow U+2026 Horizontal ellipsis, to regain the needed column.

It would work, too.  As "trunc" and "ltrunc" (and "mtrunc") are
about "truncate and show the filler to indicate some letters in the
original are not shown, and make the result exactly N columns", it
can use a narrower filler than the originally specified and use the
alloted space.

>> My worry is it is not clear what Trunk and Ltrunk should do in that
>> case.  There is no way to fit a substring of [1][2][3][4] into 5
>> columns without any filler.
> For this case where the final code point overruns, my solution
> could/would be to use the Vertical ellipsis U+22EE "⋮" to re-write that
> final character (though the Unicode Replacement Character "�" could be
> used, but that's ugly)

That would be "trunc" not "Trunc", wouldn't it?  If the capital
letter verions are "hard truncate" without filler, it fundamentally
cannot be done to fill 5 display spaces only with letters each of
which take 2 display spaces.

I am not saying that there is an impossible situation to handle and
"hard truncate" primitives should not be invented.  I just want us
to specify (and document) what happens in such a case, so that it no
longer is an "impossible" situation (we can say "odd/leftover
columns are filled with minimum number of SP to make the result N
display spaces", for example).  And not calling it "hard truncate"
might be a good place to start, as it won't be "hard truncate" with
such a padding.

Re: [PATCH v4] pretty-formats: add hard truncation, without ellipsis, options

[Philip Oakley]

Apologies for the delays.
On 26/11/2022 23:19, Junio C Hamano wrote:
> Philip Oakley <philipoakley@iee.email> writes:
>
>>>  in that they may do "[][].." or "[][][]" when told to
>>> "trunc" fill a string with four or more double-width letters into a
>>> 5 display space.  But the point is at least for these with ellipsis
>>> it is fairly clear what the desired behaviour is.
>> That "is fairly clear" is probably the problem. In retrospect it's not
>> clear in the docs that the "%<(N" format is (would appear to be) about
>> defining the display width, in terminal character columns, that the
>> selected parameter is to be displayed within.
>>
>> The code already pads the displayed parameter with spaces as required if
>> the parameter is shorter than the display width - the else condition in
>> pretty.c L1750
>>
>>>   For "trunc" in
>>> the above example, I think the right thing for it to do would be to
>>> do "[][].", i.e. consume exactly 5 display columns, and avoid
>>> exceeding the given space by not giving two dots but just one.
>> The existing choice is padding "[][]" with a single space to reach 5
>> display chars.
>> For the 6-char "[][][]" truncation it is "[][..", i.e. 3 chars from
>> "[][][]", then the two ".." dots of the ellipsis.
> Here, I realize that I did not explain the scenario well.  The
> message you are responding to was meant to be a clarification of my
> earlier message and it should have done a better job but apparently
> I failed.  Sorry, and let me try again.
>
> The single example I meant to use to illustrate the scenario I worry
> about is this.  There is a string, in which there are four (or more)
> letters, each of which occupies two display columns.

I hadn't appreciated that utf8 has 'wide' characters that are
effectively double width, i.e. use 2 display columns, such as emojis.
I had been well aware of the multi-byte nature of utf8, and vaguely
aware of the potential for combined characters.

>   And '[]' in my
> earlier messages stood for a SINGLE such letter (I just wanted to
> stick to ASCII, instead of using East Asian script, for
> illustration).  So "[][.." is not possible (you are chomping the
> second such letter in half).
>
> I could use East Asian 一二三四 (there are four letters, denoting
> one, two, three, and four, each occupying two display spaces when
> typeset in a fixed width font),

These 4 characters came through ok.
>  but to make it easier to see in
> ASCII only text, let's pretend "[1]", "[2]", "[3]", "[4]" are such
> letters.  You cannot chomp them in the middle (and please pretend
> each of them occupy two, not three, display spaces).
>
> When the given display space is 6 columns, we can fit 2 such letters
> plus ".." in the space.  If the original string were [1][2][3][4],
> it is clear trunk and ltrunk can do "[1][2].." 

> (remember [n] stands
> for a single letter whose width is 2 columns, so that takes 6
> columns) 

Aside: The man pages aren't that clear about the distinction between
display columns and characters, both for these width based formats
(allow this placeholder parameter a width of <N> columns), and the
terminal's column position formats (start this parameter at the
on-screen column <N>) .


> and "..[3][4]", respectively.  It also is clear that Trunk
> and Ltrunk can do "[1][2][3]" and "[2][3][4]", respectively.  We
> truncate the given string so that we fill the alloted display
> columns fully.

While this example is clear, it's not clear what should be done if we
have mixed width strings, e.g. with emojis, as the boundaries in random
text will also be randomly placed.
>
> If the given display space is 5 columns, the desirable behaviour for
> trunk and ltrunk is still clear.  Instead of consuming two dots, we
> could use a single dot as the filler.  As I said, I suspect that the
> implementation of trunk and ltrunc does this correctly, though.

The current implementation is buggy in the case of combining character
code points. We forget to add the (zero space) combining code points
once we have the base character when it is the character before the
split (where the double dot ".." is inserted). I.e. the zero space
characters are added after the ".." double dot.

This can cause problems with the existing code for `mtrunc` where the
'lost' combing code points then become attached to the preceding "two
dots".
>
> My worry is it is not clear what Trunk and Ltrunk should do in that
> case.  There is no way to fit a substring of [1][2][3][4] into 5
> columns without any filler.

I'm not sure that anyone has fully solved that issue of what to do when
a wide character overlaps the end of an available display space, such as
terminal word-wrap when resizing the window (in my mintty terminal
window it displays a 'space' then starts the wide character on a new line).

There's also a 'bug' reported for one of the microsoft terminals that
the user wants to position the cursor at a column that is the middle of
a wide emoji character and wants half of it over written!

For our case (no wordwrap) I'd be minded to to mark the end column with
a single width character to show that some (wide) thing should be here,
but we've had to cut it off, such as the vertical ellipsis. At least
it's explainable.

I'll at least work on the doc clarification regarding the column width,
column position and wide char (2-col) issue, and hopefully a few failing
tests for the combing code point and the wide char fitment issue.

--
Philip

Re: [PATCH v4] pretty-formats: add hard truncation, without ellipsis, options

[Junio C Hamano]

Philip Oakley <philipoakley@iee.email> writes:

>> and "..[3][4]", respectively.  It also is clear that Trunk
>> and Ltrunk can do "[1][2][3]" and "[2][3][4]", respectively.  We
>> truncate the given string so that we fill the alloted display
>> columns fully.
>
> While this example is clear, it's not clear what should be done if we
> have mixed width strings, e.g. with emojis, as the boundaries in random
> text will also be randomly placed.

As long as wider letters have widths that is integral of the
narrowest letters (ASCII?), "use N columns, padding with '.' if
needed" has a reasonable solution, no?  "[1]A[2]" occupies 2+1+2
columns, so trunc that is given only 3 (or 4) columns can drop the
last "[2]" and fit "[1]A" in the given columns with padding.

> I'll at least work on the doc clarification regarding the column width,
> column position and wide char (2-col) issue, and hopefully a few failing
> tests for the combing code point and the wide char fitment issue.

Thanks, that would give us a very good starting point.

[PATCH v5 0/5] Pretty formats: Clarify column alignment

[Philip Oakley]

This is a follow up to earlier versions of the series [1,2,3,4] that looked
to add left and right hard truncation of the column limited placeholders.

Taylor had provided an early review at [5].

Junio identified [6] that one problem would be that wide characters may
need to be cut in half and this wasn't properly covered.

I noted that there were other existing complications, and the wide
character problem could already be seen with particular placeholders.

I said I would at least update the docs [7] to cover features that weren't
fully documented and add a few simple tests to show the wide character
and other discovered problems.

The series now has 5 patches. The first four add documentation to match
their original code changes.

The final patch adds the tests for the special character cases, and adds a cautionary note to the docs.

The added tests make assumptions as to the desired solution in the 'expect'
result, and alternatives are discussed as to how to 'split' a wide
character to fit a single column when required to fit to column limits.

This series is unaffected by the security fix (CVE-2022-41903 size_t/int)
in Git 2.39.1 [8] within this formatting code.

Philip

[1] https://lore.kernel.org/git/20221030185614.3842-1-philipoakley@iee.email/ [PATCH 0/1] extend the truncating pretty formats
[2] https://lore.kernel.org/git/20221101225724.2165-1-philipoakley@iee.email/ [PATCH v2 0/1] extend the truncating pretty formats
[3] https://lore.kernel.org/git/20221102120853.2013-1-philipoakley@iee.email/ [PATCH v3] pretty-formats: add hard truncation, without ellipsis, options
[4] https://lore.kernel.org/git/20221112143616.1429-1-philipoakley@iee.email/ [PATCH v4] pretty-formats: add hard truncation, without ellipsis, options
[5] https://lore.kernel.org/git/Y17PS%2F2LgRIJKGoo@nand.local/ (Taylor: t4205-log-pretty-formats.sh. Is that coverage ..)
[6] https://lore.kernel.org/git/xmqqfsedywli.fsf@gitster.g/ (Junio: Imagine there are series of wide characters,...)
[7] https://lore.kernel.org/git/093e1dca-b9d4-f1f2-0845-ad6711622cf5@iee.email/ (po: I'll at least work on the doc clarification..)
[8] https://lore.kernel.org/git/xmqq7cxl9h0i.fsf@gitster.g/


Philip Oakley (5):
  doc: pretty-formats: separate parameters from placeholders
  doc: pretty-formats: delineate `%<|(` parameter values
  doc: pretty-formats document negative column alignments
  doc: pretty-formats describe use of ellipsis in truncation
  doc: pretty-formats note wide char limitations, and add tests

 Documentation/pretty-formats.txt | 32 +++++++++++++++++++++-----------
 t/t4205-log-pretty-formats.sh    | 27 +++++++++++++++++++++++++++
 2 files changed, 48 insertions(+), 11 deletions(-)

-- 
2.39.1.windows.1

[PATCH v5 1/5] doc: pretty-formats: separate parameters from placeholders

[Philip Oakley]

Commit a57523428b4 (pretty: support padding placeholders, %< %> and %><,
2013-04-19) introduced columnated place holders. These placeholders
can be confusing as they contain `<` and `>` characters as part
of their placeholders adjacent to the `<N>` parameters.

Add spaces either side of the `<N>` parameters in the title line.
The code (strtol) will consume any spaces around the number values
(assuming they are passed as a quoted string with spaces).
Note that the spaces are optional.

Subsequent commits will clarify other confusions.

Signed-off-by: Philip Oakley <philipoakley@iee.email>
---
 Documentation/pretty-formats.txt | 15 +++++++++------
 1 file changed, 9 insertions(+), 6 deletions(-)

diff --git a/Documentation/pretty-formats.txt b/Documentation/pretty-formats.txt
index 0b4c1c8d98..02bec23509 100644
--- a/Documentation/pretty-formats.txt
+++ b/Documentation/pretty-formats.txt
@@ -146,24 +146,27 @@ The placeholders are:
 '%m':: left (`<`), right (`>`) or boundary (`-`) mark
 '%w([<w>[,<i1>[,<i2>]]])':: switch line wrapping, like the -w option of
 			    linkgit:git-shortlog[1].
-'%<(<N>[,trunc|ltrunc|mtrunc])':: make the next placeholder take at
+'%<( <N> [,trunc|ltrunc|mtrunc])':: make the next placeholder take at
 				  least N columns, padding spaces on
 				  the right if necessary.  Optionally
 				  truncate at the beginning (ltrunc),
 				  the middle (mtrunc) or the end
 				  (trunc) if the output is longer than
-				  N columns.  Note that truncating
+				  N columns.
+				  Note 1: that truncating
 				  only works correctly with N >= 2.
-'%<|(<N>)':: make the next placeholder take at least until Nth
+				  Note 2: spaces around the N
+				  values are optional.
+'%<|( <N> )':: make the next placeholder take at least until Nth
 	     columns, padding spaces on the right if necessary
-'%>(<N>)', '%>|(<N>)':: similar to '%<(<N>)', '%<|(<N>)' respectively,
+'%>( <N> )', '%>|( <N> )':: similar to '%<( <N> )', '%<|( <N> )' respectively,
 			but padding spaces on the left
-'%>>(<N>)', '%>>|(<N>)':: similar to '%>(<N>)', '%>|(<N>)'
+'%>>( <N> )', '%>>|( <N> )':: similar to '%>( <N> )', '%>|( <N> )'
 			  respectively, except that if the next
 			  placeholder takes more spaces than given and
 			  there are spaces on its left, use those
 			  spaces
-'%><(<N>)', '%><|(<N>)':: similar to '%<(<N>)', '%<|(<N>)'
+'%><( <N> )', '%><|( <N> )':: similar to '%<( <N> )', '%<|( <N> )'
 			  respectively, but padding both sides
 			  (i.e. the text is centered)
 
-- 
2.39.1.windows.1

[PATCH v5 2/5] doc: pretty-formats: delineate `%<|(` parameter values

[Philip Oakley]

Commit a57523428b4 (pretty: support padding placeholders, %< %> and %><,
2013-04-19) introduced column width place holders. It also added
separate column position `%<|(` placeholders for display screen based
placement.

Change the display screen parameter reference from 'N' to 'M' and
corresponding descriptives to make the distinction clearer.

Signed-off-by: Philip Oakley <philipoakley@iee.email>
---
 Documentation/pretty-formats.txt | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/Documentation/pretty-formats.txt b/Documentation/pretty-formats.txt
index 02bec23509..8cc1072196 100644
--- a/Documentation/pretty-formats.txt
+++ b/Documentation/pretty-formats.txt
@@ -147,7 +147,7 @@ The placeholders are:
 '%w([<w>[,<i1>[,<i2>]]])':: switch line wrapping, like the -w option of
 			    linkgit:git-shortlog[1].
 '%<( <N> [,trunc|ltrunc|mtrunc])':: make the next placeholder take at
-				  least N columns, padding spaces on
+				  least N column widths, padding spaces on
 				  the right if necessary.  Optionally
 				  truncate at the beginning (ltrunc),
 				  the middle (mtrunc) or the end
@@ -155,18 +155,18 @@ The placeholders are:
 				  N columns.
 				  Note 1: that truncating
 				  only works correctly with N >= 2.
-				  Note 2: spaces around the N
+				  Note 2: spaces around the N and M (see below)
 				  values are optional.
-'%<|( <N> )':: make the next placeholder take at least until Nth
-	     columns, padding spaces on the right if necessary
-'%>( <N> )', '%>|( <N> )':: similar to '%<( <N> )', '%<|( <N> )' respectively,
+'%<|( <M> )':: make the next placeholder take at least until Mth
+	     display column, padding spaces on the right if necessary
+'%>( <N> )', '%>|( <M> )':: similar to '%<( <N> )', '%<|( <M> )' respectively,
 			but padding spaces on the left
-'%>>( <N> )', '%>>|( <N> )':: similar to '%>( <N> )', '%>|( <N> )'
+'%>>( <N> )', '%>>|( <M> )':: similar to '%>( <N> )', '%>|( <M> )'
 			  respectively, except that if the next
 			  placeholder takes more spaces than given and
 			  there are spaces on its left, use those
 			  spaces
-'%><( <N> )', '%><|( <N> )':: similar to '%<( <N> )', '%<|( <N> )'
+'%><( <N> )', '%><|( <M> )':: similar to '%<( <N> )', '%<|( <M> )'
 			  respectively, but padding both sides
 			  (i.e. the text is centered)
 
-- 
2.39.1.windows.1

[PATCH v5 3/5] doc: pretty-formats document negative column alignments

[Philip Oakley]

Commit 066790d7cb0 (pretty.c: support <direction>|(<negative number>) forms,
2016-06-16) added the option for right justified column alignment without
updating the documentation.

Add an explanation of its use of negative column values.

Signed-off-by: Philip Oakley <philipoakley@iee.email>
---
 Documentation/pretty-formats.txt | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/Documentation/pretty-formats.txt b/Documentation/pretty-formats.txt
index 8cc1072196..cbca60a196 100644
--- a/Documentation/pretty-formats.txt
+++ b/Documentation/pretty-formats.txt
@@ -158,7 +158,9 @@ The placeholders are:
 				  Note 2: spaces around the N and M (see below)
 				  values are optional.
 '%<|( <M> )':: make the next placeholder take at least until Mth
-	     display column, padding spaces on the right if necessary
+	     display column, padding spaces on the right if necessary.
+	     Use negative M values for column positions measured
+	     from the right hand edge of the terminal window.
 '%>( <N> )', '%>|( <M> )':: similar to '%<( <N> )', '%<|( <M> )' respectively,
 			but padding spaces on the left
 '%>>( <N> )', '%>>|( <M> )':: similar to '%>( <N> )', '%>|( <M> )'
-- 
2.39.1.windows.1

[PATCH v5 4/5] doc: pretty-formats describe use of ellipsis in truncation

[Philip Oakley]

Commit a7f01c6b4d (pretty: support truncating in %>, %< and %><,
2013-04-19) added the use of ellipsis when truncating placeholder
values.

Show our 'two dot' ellipsis, and examples for the left, middle and
right truncation to avoid any confusion as to which end of the string
is adjusted. (cf justification and sub-string).

Signed-off-by: Philip Oakley <philipoakley@iee.email>
---
 Documentation/pretty-formats.txt | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/Documentation/pretty-formats.txt b/Documentation/pretty-formats.txt
index cbca60a196..e51f1e54e1 100644
--- a/Documentation/pretty-formats.txt
+++ b/Documentation/pretty-formats.txt
@@ -149,9 +149,9 @@ The placeholders are:
 '%<( <N> [,trunc|ltrunc|mtrunc])':: make the next placeholder take at
 				  least N column widths, padding spaces on
 				  the right if necessary.  Optionally
-				  truncate at the beginning (ltrunc),
-				  the middle (mtrunc) or the end
-				  (trunc) if the output is longer than
+				  truncate (with ellipsis '..') at the left (ltrunc) `..ft`,
+				  the middle (mtrunc) `mi..le`, or the end
+				  (trunc) `rig..`, if the output is longer than
 				  N columns.
 				  Note 1: that truncating
 				  only works correctly with N >= 2.
-- 
2.39.1.windows.1

[PATCH v5 5/5] doc: pretty-formats note wide char limitations, and add tests

[Philip Oakley]

The previous commits added clarifications to the column alignment
placeholders, note that the spaces are optional around the parameters.

Also, a proposed extension [1] to allow hard truncation (without
ellipsis '..') highlighted that the existing code does not play well
with wide characters, such as Asian fonts and emojis.

For example, N wide characters take 2N columns so won't fit an odd number
column width, causing misalignment somewhere.

Further analysis also showed that decomposed characters, e.g. separate
`a` + `umlaut` Unicode code-points may also be mis-counted, in some cases
leaving multiple loose `umlauts` all combined together.

Add some notes about these limitations, and add basic tests to demonstrate
them.

The chosen solution for the tests is to substitute any wide character
that overlaps a splitting boundary for the unicode vertical ellipsis
code point as a rare but 'obvious' substitution.

An alternative could be the substitution with a single dot '.' which
matches regular expression usage, and our two dot ellipsis, and further
in scenarios where the bulk of the text is wide characters, would be
obvious. In mainly 'ascii' scenarios a singleton emoji being substituted
by a dot could be confusing.

It is enough that the tests fail cleanly. The final choice for the
substitute character can be deferred.

[1]
https://lore.kernel.org/git/20221030185614.3842-1-philipoakley@iee.email/

Signed-off-by: Philip Oakley <philipoakley@iee.email>
---
 Documentation/pretty-formats.txt |  5 +++++
 t/t4205-log-pretty-formats.sh    | 27 +++++++++++++++++++++++++++
 2 files changed, 32 insertions(+)

diff --git a/Documentation/pretty-formats.txt b/Documentation/pretty-formats.txt
index e51f1e54e1..3b71334459 100644
--- a/Documentation/pretty-formats.txt
+++ b/Documentation/pretty-formats.txt
@@ -157,6 +157,11 @@ The placeholders are:
 				  only works correctly with N >= 2.
 				  Note 2: spaces around the N and M (see below)
 				  values are optional.
+				  Note 3: Emojis and other wide characters
+				  will take two display columns, which may
+				  over-run column boundaries.
+				  Note 4: decomposed character combining marks
+				  may be misplaced at padding boundaries.
 '%<|( <M> )':: make the next placeholder take at least until Mth
 	     display column, padding spaces on the right if necessary.
 	     Use negative M values for column positions measured
diff --git a/t/t4205-log-pretty-formats.sh b/t/t4205-log-pretty-formats.sh
index 0404491d6e..2cba0e0c56 100755
--- a/t/t4205-log-pretty-formats.sh
+++ b/t/t4205-log-pretty-formats.sh
@@ -1018,4 +1018,31 @@ test_expect_success '%(describe:abbrev=...) vs git describe --abbrev=...' '
 	test_cmp expect actual
 '
 
+# pretty-formats note wide char limitations, and add tests
+test_expect_failure 'wide and decomposed characters column counting' '
+
+# from t/lib-unicode-nfc-nfd.sh hex values converted to octal
+	utf8_nfc=$(printf "\303\251") && # e acute combined.
+	utf8_nfd=$(printf "\145\314\201") && # e with a combining acute (i.e. decomposed)
+	utf8_emoji=$(printf "\360\237\221\250") &&
+
+# replacement character when requesting a wide char fits in a single display colum.
+# "half wide" alternative could be a plain ASCII dot `.`
+	utf8_vert_ell=$(printf "\342\213\256") &&
+
+# use ${xxx} here!
+	nfc10="${utf8_nfc}${utf8_nfc}${utf8_nfc}${utf8_nfc}${utf8_nfc}${utf8_nfc}${utf8_nfc}${utf8_nfc}${utf8_nfc}${utf8_nfc}" &&
+	nfd10="${utf8_nfd}${utf8_nfd}${utf8_nfd}${utf8_nfd}${utf8_nfd}${utf8_nfd}${utf8_nfd}${utf8_nfd}${utf8_nfd}${utf8_nfd}" &&
+	emoji5="${utf8_emoji}${utf8_emoji}${utf8_emoji}${utf8_emoji}${utf8_emoji}" &&
+# emoji5 uses 10 display columns
+
+	test_commit "abcdefghij" &&
+	test_commit --no-tag "${nfc10}" &&
+	test_commit --no-tag "${nfd10}" &&
+	test_commit --no-tag "${emoji5}" &&
+	printf "${utf8_emoji}..${utf8_emoji}${utf8_vert_ell}\n${utf8_nfd}..${utf8_nfd}${utf8_nfd}\n${utf8_nfc}..${utf8_nfc}${utf8_nfc}\na..ij\n" >expected &&
+	git log --format="%<(5,mtrunc)%s" -4 >actual &&
+	test_cmp expected actual
+'
+
 test_done
-- 
2.39.1.windows.1