Re: Man-fontify-manpage does not handle man, version 1.5o1, ANSI escape sequences

Re: Man-fontify-manpage does not handle man, version 1.5o1, ANSI escape sequences

[Stefan Monnier]

> Please describe exactly what actions triggered the bug
> and the precise symptoms of the bug:

> My GNU/Linux system recently had several upgrades:
>     kernel upgraded to 2.6.9
>     glibc  upgraded to 2.3.3
>     man    upgraded to 1.5o1
>     (other unknown upgrades, I'd have to ask administrator)

> Since then my M-x man output has been full of ANSI escape sequences that
> weren't previously there. I traced this to the fact that
> Man-fontify-manpage assumes that the ANSI sequences will be terminated
> by "\e[0m". However, the new "man" output uses more specific attribute
> termination sequences. For example:

>   bold       "\e[22m"
>   underline  "\e[24m"
>   reverse    "\e[27m"

> I append a fix below. Basically I pull out the code that previously only
> handled ANSI bold sequences and replace it with a new function
> Man-fontify-manpage-ANSI that I call from Man-fontify-manpage to handle
> bold, underlining, and reverse video.

Can you try the patch below instead?  It tries to handle the case where the
code does \e[1m...\e[4m....\e[0m where the 0 turns off both bold
and underline.

BTW, is it right that bold is turned on with \e[1m and turned off with
\e[22m?  It seems odd that it isn't \e[21m to turn it off or \e[2m to turn
it on, seeing how the other fit the \e[Nm and \e[2Nm rule.


        Stefan


--- man.el	03 nov 2004 00:44:39 -0500	1.138
+++ man.el	29 nov 2004 18:53:50 -0500	
@@ -1,6 +1,7 @@
 ;;; man.el --- browse UNIX manual pages -*- coding: iso-8859-1 -*-
 
-;; Copyright (C) 1993, 1994, 1996, 1997, 2001, 2003, 2004 Free Software Foundation, Inc.
+;; Copyright (C) 1993, 1994, 1996, 1997, 2001, 2003, 2004
+;;           Free Software Foundation, Inc.
 
 ;; Author: Barry A. Warsaw <bwarsaw@cen.com>
 ;; Maintainer: FSF
@@ -94,6 +95,7 @@
 \f
 ;;; Code:
 
+(eval-when-compile (require 'cl))
 (require 'assoc)
 (require 'button)
 
@@ -153,6 +155,11 @@
   :type 'face
   :group 'man)
 
+(defcustom Man-reverse-face 'secondary-selection
+  "*Face to use when fontifying reverse video."
+  :type 'face
+  :group 'man)
+
 ;; Use the value of the obsolete user option Man-notify, if set.
 (defcustom Man-notify-method (if (boundp 'Man-notify) Man-notify 'friendly)
   "*Selects the behavior when manpage is ready.
@@ -813,13 +820,27 @@
   (interactive)
   (message "Please wait: formatting the %s man page..." Man-arguments)
   (goto-char (point-min))
-  (while (search-forward "\e[1m" nil t)
-    (delete-backward-char 4)
-    (put-text-property (point)
-		       (progn (if (search-forward "\e[0m" nil 'move)
-				  (delete-backward-char 4))
-			      (point))
-		       'face Man-overstrike-face))
+  ;; Fontify ANSI escapes.
+  (let ((faces nil)
+	(start (point))
+	code)
+    (while (re-search-forward "\\e[\\([1470]\\|2\\([247]\\)\\)m" nil t)
+      (if faces (put-text-property start (match-beginning 0) 'face faces))
+      (setq faces 
+	    (if (match-beginning 2)
+		(delq (case (char-after (match-beginning 2))
+			(2 Man-overstrike-face)
+			(4 Man-underline-face)
+			(7 Man-reverse-face))
+		      faces)
+	      (cons (case (char-after (match-beginning 1))
+		      (1 Man-overstrike-face)
+		      (4 Man-underline-face)
+		      (7 Man-reverse-face)
+		      (t nil))
+		    faces)))
+      (delete-region (match-beginning 0) (match-end 0))))
+  ;; Other highlighting.
   (if (< (buffer-size) (position-bytes (point-max)))
       ;; Multibyte characters exist.
       (progn
@@ -1372,5 +1393,5 @@
 
 (provide 'man)
 
-;;; arch-tag: 587cda76-8e23-4594-b1f3-89b6b09a0d47
+;; arch-tag: 587cda76-8e23-4594-b1f3-89b6b09a0d47
 ;;; man.el ends here

Re: Man-fontify-manpage does not handle man, version 1.5o1, ANSI escape sequences

[Brian D. Carlstrom]

Stefan Monnier writes:
 > > Please describe exactly what actions triggered the bug
 > > and the precise symptoms of the bug:
 > 
 > > My GNU/Linux system recently had several upgrades:
 > >     kernel upgraded to 2.6.9
 > >     glibc  upgraded to 2.3.3
 > >     man    upgraded to 1.5o1
 > >     (other unknown upgrades, I'd have to ask administrator)
 > 
 > > Since then my M-x man output has been full of ANSI escape sequences that
 > > weren't previously there. I traced this to the fact that
 > > Man-fontify-manpage assumes that the ANSI sequences will be terminated
 > > by "\e[0m". However, the new "man" output uses more specific attribute
 > > termination sequences. For example:
 > 
 > >   bold       "\e[22m"
 > >   underline  "\e[24m"
 > >   reverse    "\e[27m"
 > 
 > > I append a fix below. Basically I pull out the code that previously only
 > > handled ANSI bold sequences and replace it with a new function
 > > Man-fontify-manpage-ANSI that I call from Man-fontify-manpage to handle
 > > bold, underlining, and reverse video.
 > 
 > Can you try the patch below instead?  It tries to handle the case where the
 > code does \e[1m...\e[4m....\e[0m where the 0 turns off both bold
 > and underline.

This patch did not work as is. Here is a list of issues and fixes/workarounds. I
append my new working version at the end.

1. The regexp should be:
    "\e\\[\\([1470]\\|2\\([247]\\)\\)m"
specifically, I changed \\e to \e and [ to \\[. Actually "3." below
requires a further change.

2. The numeric (I'm not sure its safe to assume ASCII) character values
   need to be changed to the corresponding number. So I changed these expressions
   (char-after (match-beginning 2))
   (char-after (match-beginning 1))
to
   (- (char-after (match-beginning 2)) ?0)
   (- (char-after (match-beginning 1)) ?0)
I not at all confident in the mutli-byte character set portability of this.

3. The old code lumped the "clear all attributes" code \e[0m in with the
   [147] codes marking the start of attributes:

 > +    (while (re-search-forward "\\e[\\([1470]\\|2\\([247]\\)\\)m" nil t)
...
 > +	      (cons (case (char-after (match-beginning 1))
 > +		      (1 Man-overstrike-face)
 > +		      (4 Man-underline-face)
 > +		      (7 Man-reverse-face)
 > +		      (t nil))
 > +		    faces)))

However, this led to the faces list being filled with nils. I instead
break out the \e[0m case and use it to set faces to nil.

    (while (re-search-forward "\e\\[\\([147]\\|\\(0\\)\\|2\\([247]\\)\\)m" nil t)
      (if faces (put-text-property start (match-beginning 0) 'face faces))
      (setq start (match-beginning 0))
      (setq faces 
	    (cond ((match-beginning 3)
		   (message "before case 3 %s" faces)
		   (delq (case (- (char-after (match-beginning 3)) ?0)
			   (2 Man-overstrike-face)
			   (4 Man-underline-face)
			   (7 Man-reverse-face)
			   (t (error "Unexpected case 3")))
			 faces))
		  ((match-beginning 2)
		   (message "before case 2 %s" faces)
		   nil)
		  ((match-beginning 1)
		   (message "before case 1 %s" faces)
		   (cons (case (- (char-after (match-beginning 1)) ?0)
			   (1 Man-overstrike-face)
			   (4 Man-underline-face)
			   (7 Man-reverse-face)
			   (t (error "Unexpected case 1 %s" (- (char-after (match-beginning 1)) ?0))))
			 faces))
		  (t (error "Unexpeced case"))))

4. the "start" value is never advanced so the attributes would always
   span from the start of the buffer. I added this to advance the pointer
      (setq start (match-beginning 0))

I will say that your approach is more bullet proof in terms of handling
overlapping escape sequences but a little harder to read. I guess I
might just comment the three cases and the magic constants, which were
more self explanatory in my original version.

> BTW, is it right that bold is turned on with \e[1m and turned off with
> \e[22m?  It seems odd that it isn't \e[21m to turn it off or \e[2m to turn
> it on, seeing how the other fit the \e[Nm and \e[2Nm rule.

Yeah I noticed that "almost" rule. Here is the reference I used:
 http://www.catalyst.com/support/help/cstools3/visual/terminal/escapeseq.html
 <ESC>[nm Select display attributes and color
 n Value Description
 0 Reset to default attributes and color
 1 Bold attribute
 2 Dim attribute
 4 Underline attribute
 5 Blink attribute (ignored)
 7 Reverse attribute
 8 Hidden attribute
 22 Clear bold attribute
 24 Clear underline attribute
 25 Clear blink attribute (ignored)
 27 Clear reverse attribute

here is another corroborating reference:
 http://www.isthe.com/chongo/tech/comp/ansi_escapes.html
 00 for normal display (or just 0)
 01 for bold on (or just 1)
 02 faint (or just 2)
 03 standout (or just 3)
 04 underline (or just 4)
 05 blink on (or just 5)
 07 reverse video on (or just 7)
 08 nondisplayed (invisible) (or just 8)
 22 normal
 23 no-standout
 24 no-underline
 25 no-blink
 27 no-reverse

note the second says that they can actually have leading zeros on the
singal digit codes which the code we are discussing doesn't handle but
would be easy to fix... (sorry I might have fixed it if I had noticed
this first)

-bri

  ;; Fontify ANSI escapes.
  (let ((faces nil)
	(start (point))
	code)
    (while (re-search-forward "\e\\[\\([147]\\|\\(0\\)\\|2\\([247]\\)\\)m" nil t)
      (if faces (put-text-property start (match-beginning 0) 'face faces))
      (setq start (match-beginning 0))
      (setq faces 
	    (cond ((match-beginning 3)
		   (message "before case 3 %s" faces)
		   (delq (case (- (char-after (match-beginning 3)) ?0)
			   (2 Man-overstrike-face)
			   (4 Man-underline-face)
			   (7 Man-reverse-face)
			   (t (error "Unexpected case 3")))
			 faces))
		  ((match-beginning 2)
		   (message "before case 2 %s" faces)
		   nil)
		  ((match-beginning 1)
		   (message "before case 1 %s" faces)
		   (cons (case (- (char-after (match-beginning 1)) ?0)
			   (1 Man-overstrike-face)
			   (4 Man-underline-face)
			   (7 Man-reverse-face)
			   (t (error "Unexpected case 1 %s" (- (char-after (match-beginning 1)) ?0))))
			 faces))
		  (t (error "Unexpeced case"))))
      (delete-region (match-beginning 0) (match-end 0))))

Re: Man-fontify-manpage does not handle man, version 1.5o1, ANSI escape sequences

[Stefan]

> This patch did not work as is.  Here is a list of issues and
> fixes/workarounds. I append my new working version at the end.

Thanks, I actually reread the code this morning and was about to send
a "sorry, I must have been stoned or something".

> 2. The numeric (I'm not sure its safe to assume ASCII) character values
>    need to be changed to the corresponding number. So I changed these expressions
>    (char-after (match-beginning 2))
>    (char-after (match-beginning 1))
> to
>    (- (char-after (match-beginning 2)) ?0)
>    (- (char-after (match-beginning 1)) ?0)
> I not at all confident in the mutli-byte character set portability of this.

There's no multibyte char in sight, so don't worry.
Instead of (- <foo> ?0) I'd use ?1 ?4 ?7 in the case branches.

> 		  (t (error "Unexpeced case"))))

This should say "Unexpected internal error" or somesuch, to make it clear
that the problem is not that we saw an unexpected sequence (which should not
trigger an error but should just be ignored) but that we bumped into a bug
in our own code.

>> BTW, is it right that bold is turned on with \e[1m and turned off with
>> \e[22m?  It seems odd that it isn't \e[21m to turn it off or \e[2m to turn
>> it on, seeing how the other fit the \e[Nm and \e[2Nm rule.
> Yeah I noticed that "almost" rule. Here is the reference I used:

Thank you,


        Stefan

Re: Man-fontify-manpage does not handle man, version 1.5o1, ANSI escape sequences

[Werner LEMBERG]

> BTW, is it right that bold is turned on with \e[1m and turned off
> with \e[22m?

This is correct.  \e[21m sets the `doubly underlined' rendition mode.
Note that \e[22m both switches off bold (\e[1m) and faint (\e[2m).


    Werner

Re: Man-fontify-manpage does not handle man, version 1.5o1, ANSI escape sequences

[Richard Stallman]

I too had rewritten his patch, but in a much simpler way.
Your version is fine.

However, I think that `highlight' is a more appropriate choice
for Man-reverse-face.