* [PATCH 0/1] extend the truncating pretty formats @ 2022-10-30 18:56 Philip Oakley 2022-10-30 18:56 ` [PATCH 1/1] pretty-formats: add hard truncation, without ellipsis, options Philip Oakley ` (2 more replies) 0 siblings, 3 replies; 30+ messages in thread From: Philip Oakley @ 2022-10-30 18:56 UTC (permalink / raw) To: GitList; +Cc: Self, Junio C Hamano, Taylor Blau, NSENGIYUMVA WILBERFORCE 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 ^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH 1/1] pretty-formats: add hard truncation, without ellipsis, options 2022-10-30 18:56 [PATCH 0/1] extend the truncating pretty formats Philip Oakley @ 2022-10-30 18:56 ` Philip Oakley 2022-10-30 19:23 ` Taylor Blau 2022-10-30 21:41 ` [PATCH 0/1] extend the truncating pretty formats Philip Oakley 2022-11-01 22:57 ` [PATCH v2 " Philip Oakley 2 siblings, 1 reply; 30+ messages in thread From: Philip Oakley @ 2022-10-30 18:56 UTC (permalink / raw) To: GitList; +Cc: Self, Junio C Hamano, Taylor Blau, NSENGIYUMVA WILBERFORCE 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 ^ permalink raw reply related [flat|nested] 30+ messages in thread
* Re: [PATCH 1/1] pretty-formats: add hard truncation, without ellipsis, options 2022-10-30 18:56 ` [PATCH 1/1] pretty-formats: add hard truncation, without ellipsis, options Philip Oakley @ 2022-10-30 19:23 ` Taylor Blau 2022-10-30 22:01 ` Philip Oakley 0 siblings, 1 reply; 30+ messages in thread From: Taylor Blau @ 2022-10-30 19:23 UTC (permalink / raw) To: Philip Oakley Cc: GitList, Junio C Hamano, Taylor Blau, NSENGIYUMVA WILBERFORCE 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 ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH 1/1] pretty-formats: add hard truncation, without ellipsis, options 2022-10-30 19:23 ` Taylor Blau @ 2022-10-30 22:01 ` Philip Oakley 2022-10-30 23:42 ` Taylor Blau 0 siblings, 1 reply; 30+ messages in thread From: Philip Oakley @ 2022-10-30 22:01 UTC (permalink / raw) To: Taylor Blau; +Cc: GitList, Junio C Hamano, NSENGIYUMVA WILBERFORCE 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 ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH 1/1] pretty-formats: add hard truncation, without ellipsis, options 2022-10-30 22:01 ` Philip Oakley @ 2022-10-30 23:42 ` Taylor Blau 0 siblings, 0 replies; 30+ messages in thread From: Taylor Blau @ 2022-10-30 23:42 UTC (permalink / raw) To: Philip Oakley Cc: Taylor Blau, GitList, Junio C Hamano, NSENGIYUMVA WILBERFORCE 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 ^ permalink raw reply related [flat|nested] 30+ messages in thread
* Re: [PATCH 0/1] extend the truncating pretty formats 2022-10-30 18:56 [PATCH 0/1] extend the truncating pretty formats Philip Oakley 2022-10-30 18:56 ` [PATCH 1/1] pretty-formats: add hard truncation, without ellipsis, options Philip Oakley @ 2022-10-30 21:41 ` Philip Oakley 2022-11-01 22:57 ` [PATCH v2 " Philip Oakley 2 siblings, 0 replies; 30+ messages in thread From: Philip Oakley @ 2022-10-30 21:41 UTC (permalink / raw) To: GitList; +Cc: Junio C Hamano, Taylor Blau, NSENGIYUMVA WILBERFORCE 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(-) > ^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH v2 0/1] extend the truncating pretty formats 2022-10-30 18:56 [PATCH 0/1] extend the truncating pretty formats Philip Oakley 2022-10-30 18:56 ` [PATCH 1/1] pretty-formats: add hard truncation, without ellipsis, options Philip Oakley 2022-10-30 21:41 ` [PATCH 0/1] extend the truncating pretty formats Philip Oakley @ 2022-11-01 22:57 ` Philip Oakley 2022-11-01 22:57 ` [PATCH v2 1/1] pretty-formats: add hard truncation, without ellipsis, options Philip Oakley 2 siblings, 1 reply; 30+ messages in thread From: Philip Oakley @ 2022-11-01 22:57 UTC (permalink / raw) To: GitList; +Cc: Self, Junio C Hamano, Taylor Blau, NSENGIYUMVA WILBERFORCE 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 ^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH v2 1/1] pretty-formats: add hard truncation, without ellipsis, options 2022-11-01 22:57 ` [PATCH v2 " Philip Oakley @ 2022-11-01 22:57 ` Philip Oakley 2022-11-01 23:05 ` Philip Oakley 0 siblings, 1 reply; 30+ messages in thread From: Philip Oakley @ 2022-11-01 22:57 UTC (permalink / raw) To: GitList; +Cc: Self, Junio C Hamano, Taylor Blau, NSENGIYUMVA WILBERFORCE 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 ^ permalink raw reply related [flat|nested] 30+ messages in thread
* Re: [PATCH v2 1/1] pretty-formats: add hard truncation, without ellipsis, options 2022-11-01 22:57 ` [PATCH v2 1/1] pretty-formats: add hard truncation, without ellipsis, options Philip Oakley @ 2022-11-01 23:05 ` Philip Oakley 2022-11-02 0:45 ` Taylor Blau 0 siblings, 1 reply; 30+ messages in thread From: Philip Oakley @ 2022-11-01 23:05 UTC (permalink / raw) To: GitList; +Cc: Junio C Hamano, Taylor Blau, NSENGIYUMVA WILBERFORCE 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 ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH v2 1/1] pretty-formats: add hard truncation, without ellipsis, options 2022-11-01 23:05 ` Philip Oakley @ 2022-11-02 0:45 ` Taylor Blau 2022-11-02 12:08 ` [PATCH v3] " Philip Oakley 0 siblings, 1 reply; 30+ messages in thread From: Taylor Blau @ 2022-11-02 0:45 UTC (permalink / raw) To: Philip Oakley Cc: GitList, Junio C Hamano, Taylor Blau, NSENGIYUMVA WILBERFORCE 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 ^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH v3] pretty-formats: add hard truncation, without ellipsis, options 2022-11-02 0:45 ` Taylor Blau @ 2022-11-02 12:08 ` Philip Oakley 2022-11-12 14:36 ` [PATCH v4] " Philip Oakley 0 siblings, 1 reply; 30+ messages in thread From: Philip Oakley @ 2022-11-02 12:08 UTC (permalink / raw) To: GitList; +Cc: Self, Junio C Hamano, Taylor Blau, NSENGIYUMVA WILBERFORCE 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 ^ permalink raw reply related [flat|nested] 30+ messages in thread
* [PATCH v4] pretty-formats: add hard truncation, without ellipsis, options 2022-11-02 12:08 ` [PATCH v3] " Philip Oakley @ 2022-11-12 14:36 ` Philip Oakley 2022-11-21 0:34 ` Junio C Hamano 2023-01-19 18:18 ` [PATCH v5 0/5] Pretty formats: Clarify column alignment Philip Oakley 0 siblings, 2 replies; 30+ messages in thread From: Philip Oakley @ 2022-11-12 14:36 UTC (permalink / raw) To: GitList; +Cc: Self, Junio C Hamano, Taylor Blau, NSENGIYUMVA WILBERFORCE 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 ^ permalink raw reply related [flat|nested] 30+ messages in thread
* Re: [PATCH v4] pretty-formats: add hard truncation, without ellipsis, options 2022-11-12 14:36 ` [PATCH v4] " Philip Oakley @ 2022-11-21 0:34 ` Junio C Hamano 2022-11-21 18:10 ` Philip Oakley 2023-01-19 18:18 ` [PATCH v5 0/5] Pretty formats: Clarify column alignment Philip Oakley 1 sibling, 1 reply; 30+ messages in thread From: Junio C Hamano @ 2022-11-21 0:34 UTC (permalink / raw) To: Philip Oakley; +Cc: GitList, Taylor Blau, NSENGIYUMVA WILBERFORCE 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 ... ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH v4] pretty-formats: add hard truncation, without ellipsis, options 2022-11-21 0:34 ` Junio C Hamano @ 2022-11-21 18:10 ` Philip Oakley 2022-11-22 0:57 ` Junio C Hamano 0 siblings, 1 reply; 30+ messages in thread From: Philip Oakley @ 2022-11-21 18:10 UTC (permalink / raw) To: Junio C Hamano; +Cc: GitList, Taylor Blau, NSENGIYUMVA WILBERFORCE 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 ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH v4] pretty-formats: add hard truncation, without ellipsis, options 2022-11-21 18:10 ` Philip Oakley @ 2022-11-22 0:57 ` Junio C Hamano 2022-11-23 14:26 ` Philip Oakley 0 siblings, 1 reply; 30+ messages in thread From: Junio C Hamano @ 2022-11-22 0:57 UTC (permalink / raw) To: Philip Oakley; +Cc: GitList, Taylor Blau, NSENGIYUMVA WILBERFORCE 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... ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH v4] pretty-formats: add hard truncation, without ellipsis, options 2022-11-22 0:57 ` Junio C Hamano @ 2022-11-23 14:26 ` Philip Oakley 2022-11-25 7:11 ` Junio C Hamano 0 siblings, 1 reply; 30+ messages in thread From: Philip Oakley @ 2022-11-23 14:26 UTC (permalink / raw) To: Junio C Hamano; +Cc: GitList, Taylor Blau, NSENGIYUMVA WILBERFORCE 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 ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH v4] pretty-formats: add hard truncation, without ellipsis, options 2022-11-23 14:26 ` Philip Oakley @ 2022-11-25 7:11 ` Junio C Hamano 2022-11-26 14:32 ` Philip Oakley 0 siblings, 1 reply; 30+ messages in thread From: Junio C Hamano @ 2022-11-25 7:11 UTC (permalink / raw) To: Philip Oakley; +Cc: GitList, Taylor Blau, NSENGIYUMVA WILBERFORCE 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). ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH v4] pretty-formats: add hard truncation, without ellipsis, options 2022-11-25 7:11 ` Junio C Hamano @ 2022-11-26 14:32 ` Philip Oakley 2022-11-26 22:44 ` Philip Oakley 2022-11-26 23:19 ` Junio C Hamano 0 siblings, 2 replies; 30+ messages in thread From: Philip Oakley @ 2022-11-26 14:32 UTC (permalink / raw) To: Junio C Hamano; +Cc: GitList, Taylor Blau, NSENGIYUMVA WILBERFORCE 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 ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH v4] pretty-formats: add hard truncation, without ellipsis, options 2022-11-26 14:32 ` Philip Oakley @ 2022-11-26 22:44 ` Philip Oakley 2022-11-26 23:19 ` Junio C Hamano 1 sibling, 0 replies; 30+ messages in thread From: Philip Oakley @ 2022-11-26 22:44 UTC (permalink / raw) To: Junio C Hamano; +Cc: GitList, Taylor Blau, NSENGIYUMVA WILBERFORCE 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. ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH v4] pretty-formats: add hard truncation, without ellipsis, options 2022-11-26 14:32 ` Philip Oakley 2022-11-26 22:44 ` Philip Oakley @ 2022-11-26 23:19 ` Junio C Hamano 2022-11-28 13:39 ` Philip Oakley 2022-12-07 0:24 ` Philip Oakley 1 sibling, 2 replies; 30+ messages in thread From: Junio C Hamano @ 2022-11-26 23:19 UTC (permalink / raw) To: Philip Oakley; +Cc: GitList, Taylor Blau, NSENGIYUMVA WILBERFORCE 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. ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH v4] pretty-formats: add hard truncation, without ellipsis, options 2022-11-26 23:19 ` Junio C Hamano @ 2022-11-28 13:39 ` Philip Oakley 2022-11-29 0:18 ` Junio C Hamano 2022-12-07 0:24 ` Philip Oakley 1 sibling, 1 reply; 30+ messages in thread From: Philip Oakley @ 2022-11-28 13:39 UTC (permalink / raw) To: Junio C Hamano; +Cc: GitList, Taylor Blau, NSENGIYUMVA WILBERFORCE 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. ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH v4] pretty-formats: add hard truncation, without ellipsis, options 2022-11-28 13:39 ` Philip Oakley @ 2022-11-29 0:18 ` Junio C Hamano 0 siblings, 0 replies; 30+ messages in thread From: Junio C Hamano @ 2022-11-29 0:18 UTC (permalink / raw) To: Philip Oakley; +Cc: GitList, Taylor Blau, NSENGIYUMVA WILBERFORCE 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. ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH v4] pretty-formats: add hard truncation, without ellipsis, options 2022-11-26 23:19 ` Junio C Hamano 2022-11-28 13:39 ` Philip Oakley @ 2022-12-07 0:24 ` Philip Oakley 2022-12-07 0:54 ` Junio C Hamano 1 sibling, 1 reply; 30+ messages in thread From: Philip Oakley @ 2022-12-07 0:24 UTC (permalink / raw) To: Junio C Hamano; +Cc: GitList, Taylor Blau, NSENGIYUMVA WILBERFORCE 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 ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH v4] pretty-formats: add hard truncation, without ellipsis, options 2022-12-07 0:24 ` Philip Oakley @ 2022-12-07 0:54 ` Junio C Hamano 0 siblings, 0 replies; 30+ messages in thread From: Junio C Hamano @ 2022-12-07 0:54 UTC (permalink / raw) To: Philip Oakley; +Cc: GitList, Taylor Blau, NSENGIYUMVA WILBERFORCE 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. ^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH v5 0/5] Pretty formats: Clarify column alignment 2022-11-12 14:36 ` [PATCH v4] " Philip Oakley 2022-11-21 0:34 ` Junio C Hamano @ 2023-01-19 18:18 ` Philip Oakley 2023-01-19 18:18 ` [PATCH v5 1/5] doc: pretty-formats: separate parameters from placeholders Philip Oakley ` (4 more replies) 1 sibling, 5 replies; 30+ messages in thread From: Philip Oakley @ 2023-01-19 18:18 UTC (permalink / raw) To: GitList, Junio C Hamano; +Cc: Taylor Blau, NSENGIYUMVA WILBERFORCE, self 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 ^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH v5 1/5] doc: pretty-formats: separate parameters from placeholders 2023-01-19 18:18 ` [PATCH v5 0/5] Pretty formats: Clarify column alignment Philip Oakley @ 2023-01-19 18:18 ` Philip Oakley 2023-01-19 18:18 ` [PATCH v5 2/5] doc: pretty-formats: delineate `%<|(` parameter values Philip Oakley ` (3 subsequent siblings) 4 siblings, 0 replies; 30+ messages in thread From: Philip Oakley @ 2023-01-19 18:18 UTC (permalink / raw) To: GitList, Junio C Hamano; +Cc: Taylor Blau, NSENGIYUMVA WILBERFORCE, self 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 ^ permalink raw reply related [flat|nested] 30+ messages in thread
* [PATCH v5 2/5] doc: pretty-formats: delineate `%<|(` parameter values 2023-01-19 18:18 ` [PATCH v5 0/5] Pretty formats: Clarify column alignment Philip Oakley 2023-01-19 18:18 ` [PATCH v5 1/5] doc: pretty-formats: separate parameters from placeholders Philip Oakley @ 2023-01-19 18:18 ` Philip Oakley 2023-01-19 18:18 ` [PATCH v5 3/5] doc: pretty-formats document negative column alignments Philip Oakley ` (2 subsequent siblings) 4 siblings, 0 replies; 30+ messages in thread From: Philip Oakley @ 2023-01-19 18:18 UTC (permalink / raw) To: GitList, Junio C Hamano; +Cc: Taylor Blau, NSENGIYUMVA WILBERFORCE, self 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 ^ permalink raw reply related [flat|nested] 30+ messages in thread
* [PATCH v5 3/5] doc: pretty-formats document negative column alignments 2023-01-19 18:18 ` [PATCH v5 0/5] Pretty formats: Clarify column alignment Philip Oakley 2023-01-19 18:18 ` [PATCH v5 1/5] doc: pretty-formats: separate parameters from placeholders Philip Oakley 2023-01-19 18:18 ` [PATCH v5 2/5] doc: pretty-formats: delineate `%<|(` parameter values Philip Oakley @ 2023-01-19 18:18 ` Philip Oakley 2023-01-19 18:18 ` [PATCH v5 4/5] doc: pretty-formats describe use of ellipsis in truncation Philip Oakley 2023-01-19 18:18 ` [PATCH v5 5/5] doc: pretty-formats note wide char limitations, and add tests Philip Oakley 4 siblings, 0 replies; 30+ messages in thread From: Philip Oakley @ 2023-01-19 18:18 UTC (permalink / raw) To: GitList, Junio C Hamano; +Cc: Taylor Blau, NSENGIYUMVA WILBERFORCE, self 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 ^ permalink raw reply related [flat|nested] 30+ messages in thread
* [PATCH v5 4/5] doc: pretty-formats describe use of ellipsis in truncation 2023-01-19 18:18 ` [PATCH v5 0/5] Pretty formats: Clarify column alignment Philip Oakley ` (2 preceding siblings ...) 2023-01-19 18:18 ` [PATCH v5 3/5] doc: pretty-formats document negative column alignments Philip Oakley @ 2023-01-19 18:18 ` Philip Oakley 2023-01-19 18:18 ` [PATCH v5 5/5] doc: pretty-formats note wide char limitations, and add tests Philip Oakley 4 siblings, 0 replies; 30+ messages in thread From: Philip Oakley @ 2023-01-19 18:18 UTC (permalink / raw) To: GitList, Junio C Hamano; +Cc: Taylor Blau, NSENGIYUMVA WILBERFORCE, self 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 ^ permalink raw reply related [flat|nested] 30+ messages in thread
* [PATCH v5 5/5] doc: pretty-formats note wide char limitations, and add tests 2023-01-19 18:18 ` [PATCH v5 0/5] Pretty formats: Clarify column alignment Philip Oakley ` (3 preceding siblings ...) 2023-01-19 18:18 ` [PATCH v5 4/5] doc: pretty-formats describe use of ellipsis in truncation Philip Oakley @ 2023-01-19 18:18 ` Philip Oakley 4 siblings, 0 replies; 30+ messages in thread From: Philip Oakley @ 2023-01-19 18:18 UTC (permalink / raw) To: GitList, Junio C Hamano; +Cc: Taylor Blau, NSENGIYUMVA WILBERFORCE, self 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 ^ permalink raw reply related [flat|nested] 30+ messages in thread
end of thread, other threads:[~2023-01-19 18:20 UTC | newest] Thread overview: 30+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2022-10-30 18:56 [PATCH 0/1] extend the truncating pretty formats Philip Oakley 2022-10-30 18:56 ` [PATCH 1/1] pretty-formats: add hard truncation, without ellipsis, options Philip Oakley 2022-10-30 19:23 ` Taylor Blau 2022-10-30 22:01 ` Philip Oakley 2022-10-30 23:42 ` Taylor Blau 2022-10-30 21:41 ` [PATCH 0/1] extend the truncating pretty formats Philip Oakley 2022-11-01 22:57 ` [PATCH v2 " Philip Oakley 2022-11-01 22:57 ` [PATCH v2 1/1] pretty-formats: add hard truncation, without ellipsis, options Philip Oakley 2022-11-01 23:05 ` Philip Oakley 2022-11-02 0:45 ` Taylor Blau 2022-11-02 12:08 ` [PATCH v3] " Philip Oakley 2022-11-12 14:36 ` [PATCH v4] " Philip Oakley 2022-11-21 0:34 ` Junio C Hamano 2022-11-21 18:10 ` Philip Oakley 2022-11-22 0:57 ` Junio C Hamano 2022-11-23 14:26 ` Philip Oakley 2022-11-25 7:11 ` Junio C Hamano 2022-11-26 14:32 ` Philip Oakley 2022-11-26 22:44 ` Philip Oakley 2022-11-26 23:19 ` Junio C Hamano 2022-11-28 13:39 ` Philip Oakley 2022-11-29 0:18 ` Junio C Hamano 2022-12-07 0:24 ` Philip Oakley 2022-12-07 0:54 ` Junio C Hamano 2023-01-19 18:18 ` [PATCH v5 0/5] Pretty formats: Clarify column alignment Philip Oakley 2023-01-19 18:18 ` [PATCH v5 1/5] doc: pretty-formats: separate parameters from placeholders Philip Oakley 2023-01-19 18:18 ` [PATCH v5 2/5] doc: pretty-formats: delineate `%<|(` parameter values Philip Oakley 2023-01-19 18:18 ` [PATCH v5 3/5] doc: pretty-formats document negative column alignments Philip Oakley 2023-01-19 18:18 ` [PATCH v5 4/5] doc: pretty-formats describe use of ellipsis in truncation Philip Oakley 2023-01-19 18:18 ` [PATCH v5 5/5] doc: pretty-formats note wide char limitations, and add tests Philip Oakley
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.