{"thread":{"id":"65628","subject":"[PATCH] sideband: allow ANSI SGR with colon-separated subfields","startedAt":"2026-05-13T07:15:47Z","lastAt":"2026-07-07T19:11:18Z","messageCount":4,"participants":["grawity@nullroute.lt","Johannes Schindelin","Junio C Hamano","Mantas"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"543230","messageId":"20260513070803.163546-1-grawity@nullroute.lt","threadId":"65628","inReplyTo":null,"subject":"[PATCH] sideband: allow ANSI SGR with colon-separated subfields","fromName":"","fromEmail":"grawity@nullroute.lt","sentAt":"2026-05-13T07:08:03Z","receivedAt":"2026-05-13T07:15:47Z","isPatch":true,"body":"From: Mantas Mikulėnas <grawity@gmail.com>\n\nThe SGR values used for 256-color formatting are officially defined to\nbe a single field with :-separated subfields (e.g. \"\\e[1;38:5:XX;40m\")\ndespite the more common but kludgy use of separate values (which then\nbecome context-dependent and lead to misinterpretation by incompatible\nterminals).\n\nSee also: https://github.com/ThomasDickey/xterm-snapshots/blob/6380a3eaed857c182ea6cfa78cd706966b2628d0/charproc.c#L2047-L2118\n\nSigned-off-by: Mantas Mikulėnas <grawity@gmail.com>\n---\n sideband.c | 6 +++++-\n 1 file changed, 5 insertions(+), 1 deletion(-)\n\ndiff --git a/sideband.c b/sideband.c\nindex 04282a568e..6cf70ef6f6 100644\n--- a/sideband.c\n+++ b/sideband.c\n@@ -163,6 +163,10 @@ static int handle_ansi_sequence(struct strbuf *dest, const char *src, int n)\n \t *\n \t * ESC [ [<n> [; <n>]*] m\n \t *\n+\t * where <n> can be either zero-length, or a decimal number, or a\n+\t * series of decimal numbers separated by a colon (for 256-color or\n+\t * true-color codes).\n+\t *\n \t * These are part of the Select Graphic Rendition sequences which\n \t * contain more than just color sequences, for more details see\n \t * https://en.wikipedia.org/wiki/ANSI_escape_code#SGR.\n@@ -210,7 +214,7 @@ static int handle_ansi_sequence(struct strbuf *dest, const char *src, int n)\n \t\t\tstrbuf_add(dest, src, i + 1);\n \t\t\treturn i;\n \t\t}\n-\t\tif (!isdigit(src[i]) && src[i] != ';')\n+\t\tif (!isdigit(src[i]) && src[i] != ':' && src[i] != ';')\n \t\t\tbreak;\n \t}\n \n-- \n2.54.0\n\n"},{"id":"547322","messageId":"8addf7c0-ae39-f1c0-20ab-52114702aaf6@gmx.de","threadId":"65628","inReplyTo":"20260513070803.163546-1-grawity@nullroute.lt","subject":"Re: [PATCH] sideband: allow ANSI SGR with colon-separated subfields","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2026-07-07T11:45:44Z","receivedAt":"2026-07-07T11:45:51Z","isPatch":true,"body":"Hi Mantas,\n\nOn Wed, 13 May 2026, grawity@nullroute.lt wrote:\n\n> From: Mantas Mikulėnas <grawity@gmail.com>\n> \n> The SGR values used for 256-color formatting are officially defined to\n> be a single field with :-separated subfields (e.g. \"\\e[1;38:5:XX;40m\")\n> despite the more common but kludgy use of separate values (which then\n> become context-dependent and lead to misinterpretation by incompatible\n> terminals).\n> \n> See also: https://github.com/ThomasDickey/xterm-snapshots/blob/6380a3eaed857c182ea6cfa78cd706966b2628d0/charproc.c#L2047-L2118\n\nThis change seems well-motivated and well-executed to me. Just in case\nanybody was waiting for my objections, there ain't any coming ;-)\n\nCiao,\nJohannes\n\n> \n> Signed-off-by: Mantas Mikulėnas <grawity@gmail.com>\n> ---\n>  sideband.c | 6 +++++-\n>  1 file changed, 5 insertions(+), 1 deletion(-)\n> \n> diff --git a/sideband.c b/sideband.c\n> index 04282a568e..6cf70ef6f6 100644\n> --- a/sideband.c\n> +++ b/sideband.c\n> @@ -163,6 +163,10 @@ static int handle_ansi_sequence(struct strbuf *dest, const char *src, int n)\n>  \t *\n>  \t * ESC [ [<n> [; <n>]*] m\n>  \t *\n> +\t * where <n> can be either zero-length, or a decimal number, or a\n> +\t * series of decimal numbers separated by a colon (for 256-color or\n> +\t * true-color codes).\n> +\t *\n>  \t * These are part of the Select Graphic Rendition sequences which\n>  \t * contain more than just color sequences, for more details see\n>  \t * https://en.wikipedia.org/wiki/ANSI_escape_code#SGR.\n> @@ -210,7 +214,7 @@ static int handle_ansi_sequence(struct strbuf *dest, const char *src, int n)\n>  \t\t\tstrbuf_add(dest, src, i + 1);\n>  \t\t\treturn i;\n>  \t\t}\n> -\t\tif (!isdigit(src[i]) && src[i] != ';')\n> +\t\tif (!isdigit(src[i]) && src[i] != ':' && src[i] != ';')\n>  \t\t\tbreak;\n>  \t}\n>  \n> -- \n> 2.54.0\n> \n> \n"},{"id":"547380","messageId":"xmqq4iia4q8t.fsf@gitster.g","threadId":"65628","inReplyTo":"8addf7c0-ae39-f1c0-20ab-52114702aaf6@gmx.de","subject":"Re: [PATCH] sideband: allow ANSI SGR with colon-separated subfields","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-07-07T18:19:14Z","receivedAt":"2026-07-07T18:19:17Z","isPatch":true,"body":"Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:\n\n> Hi Mantas,\n>\n> On Wed, 13 May 2026, grawity@nullroute.lt wrote:\n>\n>> From: Mantas Mikulėnas <grawity@gmail.com>\n>> \n>> The SGR values used for 256-color formatting are officially defined to\n>> be a single field with :-separated subfields (e.g. \"\\e[1;38:5:XX;40m\")\n>> despite the more common but kludgy use of separate values (which then\n>> become context-dependent and lead to misinterpretation by incompatible\n>> terminals).\n>> \n>> See also: https://github.com/ThomasDickey/xterm-snapshots/blob/6380a3eaed857c182ea6cfa78cd706966b2628d0/charproc.c#L2047-L2118\n>\n> This change seems well-motivated and well-executed to me. Just in case\n> anybody was waiting for my objections, there ain't any coming ;-)\n\nShould I take it as an Ack?\n\nFWIW, this patch literally flew below my radar coverage.  Thanks for\nnoticing.\n\nA need for fix-up like this does makes me doubt out decision to go\nwith whitelisting very narrow cases that are known to be OK (and\nfinding that the cases were too narrow and we need to extend),\ninstead of rejecting known-bad cases, by the way.\n\nThanks.\n\n> Ciao,\n> Johannes\n>\n>> \n>> Signed-off-by: Mantas Mikulėnas <grawity@gmail.com>\n>> ---\n>>  sideband.c | 6 +++++-\n>>  1 file changed, 5 insertions(+), 1 deletion(-)\n>> \n>> diff --git a/sideband.c b/sideband.c\n>> index 04282a568e..6cf70ef6f6 100644\n>> --- a/sideband.c\n>> +++ b/sideband.c\n>> @@ -163,6 +163,10 @@ static int handle_ansi_sequence(struct strbuf *dest, const char *src, int n)\n>>  \t *\n>>  \t * ESC [ [<n> [; <n>]*] m\n>>  \t *\n>> +\t * where <n> can be either zero-length, or a decimal number, or a\n>> +\t * series of decimal numbers separated by a colon (for 256-color or\n>> +\t * true-color codes).\n>> +\t *\n>>  \t * These are part of the Select Graphic Rendition sequences which\n>>  \t * contain more than just color sequences, for more details see\n>>  \t * https://en.wikipedia.org/wiki/ANSI_escape_code#SGR.\n>> @@ -210,7 +214,7 @@ static int handle_ansi_sequence(struct strbuf *dest, const char *src, int n)\n>>  \t\t\tstrbuf_add(dest, src, i + 1);\n>>  \t\t\treturn i;\n>>  \t\t}\n>> -\t\tif (!isdigit(src[i]) && src[i] != ';')\n>> +\t\tif (!isdigit(src[i]) && src[i] != ':' && src[i] != ';')\n>>  \t\t\tbreak;\n>>  \t}\n>>  \n>> -- \n>> 2.54.0\n>> \n>> \n"},{"id":"547385","messageId":"897cefe6-18f1-4c53-bf2a-dd91052e70e5@nullroute.lt","threadId":"65628","inReplyTo":"xmqq4iia4q8t.fsf@gitster.g","subject":"Re: [PATCH] sideband: allow ANSI SGR with colon-separated subfields","fromName":"Mantas","fromEmail":"grawity@nullroute.lt","sentAt":"2026-07-07T19:01:50Z","receivedAt":"2026-07-07T19:11:18Z","isPatch":true,"body":"On 2026-07-07 21:19, Junio C Hamano wrote:\n> A need for fix-up like this does makes me doubt out decision to go\n> with whitelisting very narrow cases that are known to be OK (and\n> finding that the cases were too narrow and we need to extend),\n> instead of rejecting known-bad cases, by the way.\n\nFor the SGR sequence (CSI ... m) AFAIK there are no other value types \nbesides what this patch adds (i.e. colon-separated decimal \nsubparameters), so the filter will now include all possible cases. It \nalready isn't picky about individual values.\n\nAs for everything else (that is escape sequences in general), I'd say \nthere are way too many \"bad\" cases – which can toggle terminal modes, \ndisplay images, send notifications, copy to clipboard, move windows \naround... which a remote \"status text\" output has no reason to include – \nand basically zero \"good\" cases. Though the code already disallows the \nmajority of harmful cases by only accepting those starting with CSI \n(\"ESC [\").\n\nIn fact I might personally go further and disallow even all of the \ncursor movement sequences and leave only SGR and EL (clear line). I can \nimagine a use-case for free cursor movement (parallel progress bars like \nin Arch's pacman?) but it practically requires the sender to know \nterminal dimensions to be useful, so IMO is entirely out of scope for \nsideband status output.\n\n(Progress bar for example (OSC 9;4;n ST) might be useful, but IMO it \nshould be generated client-side, with the server instead having some way \nto report an integer percentage through the protocol. In theory the \n\"hyperlink\" sequence (OSC 8 ; ... ST) could also be useful to send as \npart of status text, but I'd personally disallow that as well, given all \nthe reporting last time someone discovered it was possible to link to a \nfile:// URL.)\n\n\n> Thanks.\n>\n>> Ciao,\n>> Johannes\n>>\n>>> Signed-off-by: Mantas Mikulėnas <grawity@gmail.com>\n>>> ---\n>>>   sideband.c | 6 +++++-\n>>>   1 file changed, 5 insertions(+), 1 deletion(-)\n>>>\n>>> diff --git a/sideband.c b/sideband.c\n>>> index 04282a568e..6cf70ef6f6 100644\n>>> --- a/sideband.c\n>>> +++ b/sideband.c\n>>> @@ -163,6 +163,10 @@ static int handle_ansi_sequence(struct strbuf *dest, const char *src, int n)\n>>>   \t *\n>>>   \t * ESC [ [<n> [; <n>]*] m\n>>>   \t *\n>>> +\t * where <n> can be either zero-length, or a decimal number, or a\n>>> +\t * series of decimal numbers separated by a colon (for 256-color or\n>>> +\t * true-color codes).\n>>> +\t *\n>>>   \t * These are part of the Select Graphic Rendition sequences which\n>>>   \t * contain more than just color sequences, for more details see\n>>>   \t * https://en.wikipedia.org/wiki/ANSI_escape_code#SGR.\n>>> @@ -210,7 +214,7 @@ static int handle_ansi_sequence(struct strbuf *dest, const char *src, int n)\n>>>   \t\t\tstrbuf_add(dest, src, i + 1);\n>>>   \t\t\treturn i;\n>>>   \t\t}\n>>> -\t\tif (!isdigit(src[i]) && src[i] != ';')\n>>> +\t\tif (!isdigit(src[i]) && src[i] != ':' && src[i] != ';')\n>>>   \t\t\tbreak;\n>>>   \t}\n>>>   \n>>> -- \n>>> 2.54.0\n>>>\n>>>\n"}]}