Hi Nick,
Alex has been waiting on me for a review of this patch. I took longer
than I should have because I thought he was expecting a technical
evaluation of the accuracy of the sequences documented.[1] Now I see it
was just a matter of man(7) and tbl(1) syntactical and style review.
Easy bits first.
> Remove CSI prefix from the list of non-CSI escapes.
+1
> End all items of said list with periods, matching other sections of
> the page.
+1
> Fix up the busted OSC command list (reset palette and set palette).
> ESC ] OSC T{
> -(Should be: Operating system command)
> -ESC ] P \fInrrggbb\fP: set palette, with parameter
> -given in 7 hexadecimal digits after the final P :-(.
> -Here \fIn\fP is the color (0\(en15), and \fIrrggbb\fP indicates
> +Operating System Command prefix.
> +T}
> +ESC ] R Reset palette.
> +ESC ] P T{
> +Set palette, with parameter given in 7 hexadecimal digits \fInrrggbb\fP after
> +the final P. Here \fIn\fP is the color (0\(en15), and \fIrrggbb\fP indicates
> the red/green/blue values (0\(en255).
> -ESC ] R: reset palette
> T}
Yes, this was majorly hosed up, syntactically. What you have is good
repair to obvious damage. +1.
I have some suggestions for further improvement, but please don't gate
the patch application on these.
1. "Operating System Command prefix." does not need to be in a text
block. It can be set as an ordinary table entry just like "Reset
palette."
2. The description of ESC ] P _is_ in a text block. That's good,
because guess what? Since it is _text_, you don't need those pesky font
selection escape sequences. You can use man(7) font macros instead.
You can furthermore apply all of the style rules that attach to ordinary
man page text. And I would tighten the wording, too.
So I would write...
ESC ] P T{
Set palette,
with a parameter of 7 hexadecimal digits
.I nrrggbb
after
.BR P .
.IR n\~ is
the color,
and
.I rrggbb
its 8-bit red/green/blue channel values.
T}
Observe the use of a non-breaking space escape sequence \~ to act as a
"tie", preventing a line break between "n" and the very short word "is".
This is not essential; it is a recommended typographical practice. I
also saw someone ask how to do this on StackExchange recently[2].
I dropped mentions of range because I feel that people using hexadecimal
can be expected to know how to count in that base.
> .ad l
> .TS
> .TE
> .ad
3. Consider dumping these adjustment requests bracketing the tables.
They arise from a misconception that adjustment cannot be manipulated
from within tbl(1) text blocks. It can. I would check the output to
see if leaving adjustment as-is results in ugly table entries, and if
does, add 'na' requests to the beginnings of the affected text blocks on
a case by case basis. We try to discourage the use of *roff requests in
man page text; moving them into tbl(1) content is a slight improvement.
I rewrote the groff tbl(1) man page for 1.23 (forthcoming) because I
found the existing one difficult to understand. Even Lesk's original
paper (CSTR #49) left me wondering in places. (I have a thick skull.)
Here is the subsection on text blocks. Font style changes are lost in
plain text email. More context is available at the source[3].
[[
Text blocks
An ordinary table entry’s contents can make a column, and
therefore the table, excessively wide; the table then exceeds the
line length of the page, and becomes ugly or is exposed to
truncation by the output device. When a table entry requires
more conventional typesetting, breaking across more than one
output line (and thereby increasing the height of its row), it
can be placed within a text block.
tbl interprets a table entry of “T{” at the end of an input line
not as table data, but as a token starting a text block.
Similarly, “T}” at the start of an input line ends a text block.
Text block tokens can share an input line with other table data
(preceding T{ and following T}). Input lines between these
tokens are formatted in a diversion by troff. Text blocks cannot
be nested. Multiple text blocks can occur in a table row.
Like other table entries, text blocks are formatted as was the
text prior to the table, modified by applicable column
descriptors. Specifically, the classifiers A, C, L, N, R, and S
determine a text block’s alignment within its cell, but not its
adjustment. You can add na or ad requests to the beginning of a
text block to alter its adjustment distinctly from other text in
the document. As with other table entries, when a text block
ends, any alterations to its formatting are discarded. They do
not affect subsequent table entries, not even other text blocks.
If w or x modifiers are not specified for all columns of a text
block’s span, the default length of the text block (more
precisely, the line length used to process the text block
diversion) is computed as L×C/(N+1), where L is the current line
length, C the number of columns spanned by the text block, and N
the number of columns in the table. If necessary, you can also
control a text block’s width by including an ll (line length)
request in it prior to any text to be formatted. Because a
diversion is used to format the text block, its width is
subsequently available in the register dl.
]]
Regards,
Branden
[1] I did that for the ones most recast, ESC ] P and ESC ] R, but
unfortunately the console driver on my system suffers from serious
redraw problems on scroll; if you reset the palette from the last
line on the screen, many characters on the screen do not get
repainted in the new color. Keep changing the palette, and color
errors accumulate, so the appearance of color 7 (the default
foreground) can appear in many shades simultaneously even though the
driver, presumably, thinks they're all the same. If the redrawing
errors are deterministic, can I assume that this will never be
rectified because someone out there must have a 24-bit color ASCII
art image viewer for the console, and fixing the bug would ruin it?
[2] https://unix.stackexchange.com/questions/694622/how-can-i-prevent-a-line-break-between-option-and-parameter-using-rb-and-ir/694765#694765
[3] https://git.savannah.gnu.org/cgit/groff.git/tree/src/preproc/tbl/tbl.1.man
Hi, nick, and Branden!
On 3/20/22 17:02, G. Branden Robinson wrote:
> Hi Nick,
>
> Alex has been waiting on me for a review of this patch. I took longer
> than I should have because I thought he was expecting a technical
> evaluation of the accuracy of the sequences documented.[1] Now I see it
> was just a matter of man(7) and tbl(1) syntactical and style review.
No, you were originally right. I did mean both, but especially
"a technical evaluation of the accuracy of the sequences documented".
You did it anyway, so thanks! :)
>
> Easy bits first.
>
>> Remove CSI prefix from the list of non-CSI escapes.
>
> +1
>
>> End all items of said list with periods, matching other sections of
>> the page.
>
> +1
>
>> Fix up the busted OSC command list (reset palette and set palette).
>
>> ESC ] OSC T{
>> -(Should be: Operating system command)
>> -ESC ] P \fInrrggbb\fP: set palette, with parameter
>> -given in 7 hexadecimal digits after the final P :-(.
>> -Here \fIn\fP is the color (0\(en15), and \fIrrggbb\fP indicates
>> +Operating System Command prefix.
>> +T}
>> +ESC ] R Reset palette.
>> +ESC ] P T{
>> +Set palette, with parameter given in 7 hexadecimal digits \fInrrggbb\fP after
>> +the final P. Here \fIn\fP is the color (0\(en15), and \fIrrggbb\fP indicates
>> the red/green/blue values (0\(en255).
>> -ESC ] R: reset palette
>> T}
>
> Yes, this was majorly hosed up, syntactically. What you have is good
> repair to obvious damage. +1.
>
> I have some suggestions for further improvement, but please don't gate
> the patch application on these.
[...]
Okay, thanks!
nick, can you please resend the patch? I've lost the original email.
Cheers,
Alex
--
Alejandro Colomar
Linux man-pages comaintainer; https://www.kernel.org/doc/man-pages/
http://www.alejandro-colomar.es/
G. Branden Robinson left as an exercise for the reader:
> I have some suggestions for further improvement, but please don't gate
> the patch application on these.
always good to hear from you, GBR. i replied to alejandro's
request with the original, not wanting to mess with (and then
test) groff right at the moment, but i've saved your
animadversions. i've got a patch outstanding for the linux
console right now, and once it lands, i'll need come back and
modify this page again (indeed, that's why i was looking at it
in the first place). i'll effect your suggestions then, thanks!
--
nick black -=- https://www.nick-black.com
to make an apple pie from scratch,
you need first invent a universe.
Alejandro Colomar (man-pages) left as an exercise for the reader:
> nick, can you please resend the patch? I've lost the original email.
Fix up the busted OSC command list (reset palette and
set palette). Remove CSI prefix from the list of non-CSI
escapes. End all items of said list with periods,
matching other sections of the page.
Signed-off-by: nick black <[email protected]>
---
man4/console_codes.4 | 22 +++++++++++-----------
1 file changed, 11 insertions(+), 11 deletions(-)
diff --git man4/console_codes.4 man4/console_codes.4
index d549b76a9..001de1955 100644
--- man4/console_codes.4
+++ man4/console_codes.4
@@ -139,29 +139,28 @@ T}
ESC 8 DECRC T{
Restore state most recently saved by ESC 7.
T}
-ESC [ CSI Control sequence introducer
ESC % Start sequence selecting character set
ESC % @ \0\0\0Select default (ISO 646 / ISO 8859-1)
ESC % G \0\0\0Select UTF-8
ESC % 8 \0\0\0Select UTF-8 (obsolete)
ESC # 8 DECALN T{
-DEC screen alignment test \- fill screen with E's
+DEC screen alignment test \- fill screen with E's.
T}
ESC ( T{
Start sequence defining G0 character set
(followed by one of B, 0, U, K, as below)
T}
ESC ( B T{
-Select default (ISO 8859-1 mapping)
+Select default (ISO 8859-1 mapping).
T}
ESC ( 0 T{
-Select VT100 graphics mapping
+Select VT100 graphics mapping.
T}
ESC ( U T{
-Select null mapping \- straight to character ROM
+Select null mapping \- straight to character ROM.
T}
ESC ( K T{
-Select user mapping \- the map that is loaded by the utility \fBmapscrn\fP(8)
+Select user mapping \- the map that is loaded by the utility \fBmapscrn\fP(8).
T}
ESC ) T{
Start sequence defining G1 (followed by one of B, 0, U, K, as above).
@@ -169,12 +168,13 @@ T}
ESC > DECPNM Set numeric keypad mode
ESC = DECPAM Set application keypad mode
ESC ] OSC T{
-(Should be: Operating system command)
-ESC ] P \fInrrggbb\fP: set palette, with parameter
-given in 7 hexadecimal digits after the final P :-(.
-Here \fIn\fP is the color (0\(en15), and \fIrrggbb\fP indicates
+Operating System Command prefix.
+T}
+ESC ] R Reset palette.
+ESC ] P T{
+Set palette, with parameter given in 7 hexadecimal digits \fInrrggbb\fP after
+the final P. Here \fIn\fP is the color (0\(en15), and \fIrrggbb\fP indicates
the red/green/blue values (0\(en255).
-ESC ] R: reset palette
T}
.TE
.ad
--
2.34.1
--
nick black -=- https://www.nick-black.com
to make an apple pie from scratch,
you need first invent a universe.
Alejandro Colomar (man-pages) left as an exercise for the reader:
> If it repeats, I'll try to investigate the reason.
or i can just make sure i'm sending the expected format--it
ought not be beyond my powers =]. thanks for bringing this to my
attention.
--
nick black -=- https://www.nick-black.com
to make an apple pie from scratch,
you need first invent a universe.
Hi nick,
On 3/21/22 09:30, nick black wrote:
> Alejandro Colomar (man-pages) left as an exercise for the reader:
>> nick, can you please resend the patch? I've lost the original email.
>
> Fix up the busted OSC command list (reset palette and
> set palette). Remove CSI prefix from the list of non-CSI
> escapes. End all items of said list with periods,
> matching other sections of the page.
>
> Signed-off-by: nick black <[email protected]>
Patch applied.
However, it's weird: I had to apply the following to your patch before
applying it with `git am`:
/^diff --git/s, man4, a/man4,
/^diff --git/s, man4, b/man4,
/^--- man4/s, man4, a/man4,
/^+++ man4/s, man4, b/man4,
I'm curious, how did you generate the patch?
Cheers,
Alex
> ---
> man4/console_codes.4 | 22 +++++++++++-----------
> 1 file changed, 11 insertions(+), 11 deletions(-)
>
> diff --git man4/console_codes.4 man4/console_codes.4
> index d549b76a9..001de1955 100644
> --- man4/console_codes.4
> +++ man4/console_codes.4
> @@ -139,29 +139,28 @@ T}
> ESC 8 DECRC T{
> Restore state most recently saved by ESC 7.
> T}
> -ESC [ CSI Control sequence introducer
> ESC % Start sequence selecting character set
> ESC % @ \0\0\0Select default (ISO 646 / ISO 8859-1)
> ESC % G \0\0\0Select UTF-8
> ESC % 8 \0\0\0Select UTF-8 (obsolete)
> ESC # 8 DECALN T{
> -DEC screen alignment test \- fill screen with E's
> +DEC screen alignment test \- fill screen with E's.
> T}
> ESC ( T{
> Start sequence defining G0 character set
> (followed by one of B, 0, U, K, as below)
> T}
> ESC ( B T{
> -Select default (ISO 8859-1 mapping)
> +Select default (ISO 8859-1 mapping).
> T}
> ESC ( 0 T{
> -Select VT100 graphics mapping
> +Select VT100 graphics mapping.
> T}
> ESC ( U T{
> -Select null mapping \- straight to character ROM
> +Select null mapping \- straight to character ROM.
> T}
> ESC ( K T{
> -Select user mapping \- the map that is loaded by the utility \fBmapscrn\fP(8)
> +Select user mapping \- the map that is loaded by the utility \fBmapscrn\fP(8).
> T}
> ESC ) T{
> Start sequence defining G1 (followed by one of B, 0, U, K, as above).
> @@ -169,12 +168,13 @@ T}
> ESC > DECPNM Set numeric keypad mode
> ESC = DECPAM Set application keypad mode
> ESC ] OSC T{
> -(Should be: Operating system command)
> -ESC ] P \fInrrggbb\fP: set palette, with parameter
> -given in 7 hexadecimal digits after the final P :-(.
> -Here \fIn\fP is the color (0\(en15), and \fIrrggbb\fP indicates
> +Operating System Command prefix.
> +T}
> +ESC ] R Reset palette.
> +ESC ] P T{
> +Set palette, with parameter given in 7 hexadecimal digits \fInrrggbb\fP after
> +the final P. Here \fIn\fP is the color (0\(en15), and \fIrrggbb\fP indicates
> the red/green/blue values (0\(en255).
> -ESC ] R: reset palette
> T}
> .TE
> .ad
--
Alejandro Colomar
Linux man-pages comaintainer; http://www.kernel.org/doc/man-pages/
http://www.alejandro-colomar.es/
Alejandro Colomar (man-pages) left as an exercise for the reader:
> Patch applied.
> However, it's weird: I had to apply the following to your patch before
> applying it with `git am`:
>
> /^diff --git/s, man4, a/man4,
> /^diff --git/s, man4, b/man4,
> /^--- man4/s, man4, a/man4,
> /^+++ man4/s, man4, b/man4,
>
> I'm curious, how did you generate the patch?
i bounced this out from ~/Mail/sent, and have no idea how i
originally created it, sorry =\. i'm assuming git email-send?
--
nick black -=- https://www.nick-black.com
to make an apple pie from scratch,
you need first invent a universe.
On 3/22/22 13:56, nick black wrote:
> Alejandro Colomar (man-pages) left as an exercise for the reader:
>> Patch applied.
>> However, it's weird: I had to apply the following to your patch before
>> applying it with `git am`:
>>
>> /^diff --git/s, man4, a/man4,
>> /^diff --git/s, man4, b/man4,
>> /^--- man4/s, man4, a/man4,
>> /^+++ man4/s, man4, b/man4,
>>
>> I'm curious, how did you generate the patch?
>
> i bounced this out from ~/Mail/sent, and have no idea how i
> originally created it, sorry =\. i'm assuming git email-send?
>
Yeah, I guessed that you used git format-patch && git send-email,
especially since there's a git version at the end of the patch. The
weird thing is that git always (AFAIK) writes (and needs) those a/ and
b/ prefixes, so... I don't know.
If it repeats, I'll try to investigate the reason.
Thanks!
Alex
--
Alejandro Colomar
Linux man-pages comaintainer; http://www.kernel.org/doc/man-pages/
http://www.alejandro-colomar.es/