{"thread":{"id":"52029","subject":"[PATCH] column: use utf8_strnwidth() to strip out ANSI color escapes","startedAt":"2019-10-13T12:49:57Z","lastAt":"2019-10-14T19:33:39Z","messageCount":4,"participants":["René Scharfe","Johannes Schindelin"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"383993","messageId":"9b3f6960-ea75-c3a7-3a24-0554320bb359@web.de","threadId":"52029","inReplyTo":null,"subject":"[PATCH] column: use utf8_strnwidth() to strip out ANSI color escapes","fromName":"René Scharfe","fromEmail":"l.s.r@web.de","sentAt":"2019-10-13T12:49:50Z","receivedAt":"2019-10-13T12:49:57Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"Make use of utf8_strnwidth()'s feature to skip ANSI escape sequences\ninstead of open-coding it.  This shortens the code and makes it more\nconsistent.\n\nThis changes the behavior, though: The old code skips all kinds of\nControl Sequence Introducer sequences, while utf8_strnwidth() only skips\nthe Select Graphic Rendition kind, i.e. those ending with \"m\".  They are\nused for specifying color and font attributes like boldness.  The only\nother kind of escape sequence we print in Git is Erase in Line, ending\nwith \"K\".  That's not used for columnar output, so this difference\nactually doesn't matter here.\n\nSigned-off-by: René Scharfe <l.s.r@web.de>\n---\n column.c | 13 +------------\n 1 file changed, 1 insertion(+), 12 deletions(-)\n\ndiff --git a/column.c b/column.c\nindex 7a17c14b82..4a38eed322 100644\n--- a/column.c\n+++ b/column.c\n@@ -23,18 +23,7 @@ struct column_data {\n /* return length of 's' in letters, ANSI escapes stripped */\n static int item_length(const char *s)\n {\n-\tint len, i = 0;\n-\tstruct strbuf str = STRBUF_INIT;\n-\n-\tstrbuf_addstr(&str, s);\n-\twhile ((s = strstr(str.buf + i, \"\\033[\")) != NULL) {\n-\t\tint len = strspn(s + 2, \"0123456789;\");\n-\t\ti = s - str.buf;\n-\t\tstrbuf_remove(&str, i, len + 3); /* \\033[<len><func char> */\n-\t}\n-\tlen = utf8_strwidth(str.buf);\n-\tstrbuf_release(&str);\n-\treturn len;\n+\treturn utf8_strnwidth(s, -1, 1);\n }\n\n /*\n--\n2.23.0\n"},{"id":"384008","messageId":"nycvar.QRO.7.76.6.1910141312010.46@tvgsbejvaqbjf.bet","threadId":"52029","inReplyTo":"9b3f6960-ea75-c3a7-3a24-0554320bb359@web.de","subject":"Re: [PATCH] column: use utf8_strnwidth() to strip out ANSI color escapes","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2019-10-14T11:13:08Z","receivedAt":"2019-10-14T11:13:36Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi René,\n\nOn Sun, 13 Oct 2019, René Scharfe wrote:\n\n> Make use of utf8_strnwidth()'s feature to skip ANSI escape sequences\n> instead of open-coding it.  This shortens the code and makes it more\n> consistent.\n\nSounds good.\n\n> This changes the behavior, though: The old code skips all kinds of\n> Control Sequence Introducer sequences, while utf8_strnwidth() only skips\n> the Select Graphic Rendition kind, i.e. those ending with \"m\".  They are\n> used for specifying color and font attributes like boldness.  The only\n> other kind of escape sequence we print in Git is Erase in Line, ending\n> with \"K\".  That's not used for columnar output, so this difference\n> actually doesn't matter here.\n\nArguably, the \"Erase in Line\" thing should re-set the width to 0, no?\nBut as you say, this is not needed for this patch.\n\nThanks,\nDscho\n\n>\n> Signed-off-by: René Scharfe <l.s.r@web.de>\n> ---\n>  column.c | 13 +------------\n>  1 file changed, 1 insertion(+), 12 deletions(-)\n>\n> diff --git a/column.c b/column.c\n> index 7a17c14b82..4a38eed322 100644\n> --- a/column.c\n> +++ b/column.c\n> @@ -23,18 +23,7 @@ struct column_data {\n>  /* return length of 's' in letters, ANSI escapes stripped */\n>  static int item_length(const char *s)\n>  {\n> -\tint len, i = 0;\n> -\tstruct strbuf str = STRBUF_INIT;\n> -\n> -\tstrbuf_addstr(&str, s);\n> -\twhile ((s = strstr(str.buf + i, \"\\033[\")) != NULL) {\n> -\t\tint len = strspn(s + 2, \"0123456789;\");\n> -\t\ti = s - str.buf;\n> -\t\tstrbuf_remove(&str, i, len + 3); /* \\033[<len><func char> */\n> -\t}\n> -\tlen = utf8_strwidth(str.buf);\n> -\tstrbuf_release(&str);\n> -\treturn len;\n> +\treturn utf8_strnwidth(s, -1, 1);\n>  }\n>\n>  /*\n> --\n> 2.23.0\n>\n"},{"id":"384019","messageId":"a165b244-b18d-0434-42c5-bfe28b8bab51@web.de","threadId":"52029","inReplyTo":"nycvar.QRO.7.76.6.1910141312010.46@tvgsbejvaqbjf.bet","subject":"Re: [PATCH] column: use utf8_strnwidth() to strip out ANSI color escapes","fromName":"René Scharfe","fromEmail":"l.s.r@web.de","sentAt":"2019-10-14T14:16:15Z","receivedAt":"2019-10-14T14:16:43Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"Am 14.10.19 um 13:13 schrieb Johannes Schindelin:\n> Hi René,\n>\n> On Sun, 13 Oct 2019, René Scharfe wrote:\n>\n>> This changes the behavior, though: The old code skips all kinds of\n>> Control Sequence Introducer sequences, while utf8_strnwidth() only skips\n>> the Select Graphic Rendition kind, i.e. those ending with \"m\".  They are\n>> used for specifying color and font attributes like boldness.  The only\n>> other kind of escape sequence we print in Git is Erase in Line, ending\n>> with \"K\".  That's not used for columnar output, so this difference\n>> actually doesn't matter here.\n>\n> Arguably, the \"Erase in Line\" thing should re-set the width to 0, no?\n> But as you say, this is not needed for this patch.\n\nIt doesn't move the cursor, just clears the characters to the right, to\nthe left or both sides, depending on its parameter.  So ignoring it for\nwidth calculation like\tthe old code did would be appropriate -- if we'd\nencounter such an escape sequence in text to be shown in columns.\n\nRené\n"},{"id":"384028","messageId":"nycvar.QRO.7.76.6.1910142132360.46@tvgsbejvaqbjf.bet","threadId":"52029","inReplyTo":"a165b244-b18d-0434-42c5-bfe28b8bab51@web.de","subject":"Re: [PATCH] column: use utf8_strnwidth() to strip out ANSI color escapes","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2019-10-14T19:33:15Z","receivedAt":"2019-10-14T19:33:39Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi René,\n\nOn Mon, 14 Oct 2019, René Scharfe wrote:\n\n> Am 14.10.19 um 13:13 schrieb Johannes Schindelin:\n>\n> > On Sun, 13 Oct 2019, René Scharfe wrote:\n> >\n> >> This changes the behavior, though: The old code skips all kinds of\n> >> Control Sequence Introducer sequences, while utf8_strnwidth() only skips\n> >> the Select Graphic Rendition kind, i.e. those ending with \"m\".  They are\n> >> used for specifying color and font attributes like boldness.  The only\n> >> other kind of escape sequence we print in Git is Erase in Line, ending\n> >> with \"K\".  That's not used for columnar output, so this difference\n> >> actually doesn't matter here.\n> >\n> > Arguably, the \"Erase in Line\" thing should re-set the width to 0, no?\n> > But as you say, this is not needed for this patch.\n>\n> It doesn't move the cursor, just clears the characters to the right, to\n> the left or both sides, depending on its parameter.  So ignoring it for\n> width calculation like\tthe old code did would be appropriate -- if we'd\n> encounter such an escape sequence in text to be shown in columns.\n\nWhoops, you're right. I brainfarted, mistaking it for `\\r`... My bad!\n\nCiao,\nDscho\n"}]}