Subject: Re: bug#30908: woman2-roff-buffer fails to restore set-text-properties, etc. on error

From: Ivan Shmakov <[email-masked]>
Date: Fri, 23 Mar 2018 03:24:17 +0000

>>>>> Noam Postavsky <[email-masked]> writes:
>>>>> Ivan Shmakov <[email-masked]> writes:

 >> @@ -3710,13 +3710,14 @@ defun woman2-roff-buffer ()

 >> +      (unwind-protect
 >> +          (if (not (eobp))             ; This should not happen, but ...
 >> +              (woman2-format-paragraphs (copy-marker (point-max) t)
 >> +                                        woman-left-margin))
 >> +        (fset 'canonically-space-region canonically-space-region)
 >> +        (fset 'set-text-properties set-text-properties)
 >> +        (fset 'insert-and-inherit insert-and-inherit)
 >> +        (set-marker to nil)))))

 > Shouldn’t this rather be combined into the existing unwind-protect
 > around the while?

	Due to the (not (eobp)) guard, I’ve assumed that the
	woman2-format-paragraphs call in the ‘unwind’ branch is for some
	sort of fallback processing (in the case the while loop fails.)

	If the call ended up there by mistake, indeed a progn should be
	used, as per the updated patch MIMEd.

-- 
FSF associate member #7257  http://am-1.org/~ivan/
--- -	2018-03-23 02:20:53.907229515 +0000
+++ 81FOuhEi.el	2018-03-23 02:20:39.083966234 +0000
@@ -3673,46 +3673,47 @@
     (fset 'insert-and-inherit (symbol-function 'insert))
     (fset 'set-text-properties 'ignore)
     (unwind-protect
-	(while
-	    ;; Find next control line:
-            (re-search-forward woman-request-regexp nil t)
-          (cond
-           ;; Construct woman function to call:
-           ((setq fn (intern-soft
-                      (concat "woman2-"
-                              (setq woman-request (match-string 1)))))
-            ;; Delete request or macro name:
-            (woman-delete-match 0))
-           ;; Unrecognized request:
-           ((prog1 nil
-              ;; (WoMan-warn ".%s request ignored!" woman-request)
-              (WoMan-warn-ignored woman-request "ignored!")
-              ;; (setq fn 'woman2-LP)
-              ;; AVOID LEAVING A BLANK LINE!
-              ;; (setq fn 'woman2-format-paragraphs)
-              ))
-           ;; .LP assumes it is at eol and leaves a (blank) line,
-           ;; so leave point at end of line before paragraph:
-           ((or (looking-at "[ \t]*$") ; no argument
-                woman-ignore)          ; ignore all
-            ;; (beginning-of-line) (kill-line)
-            ;; AVOID LEAVING A BLANK LINE!
-            (beginning-of-line) (woman-delete-line 1))
-           (t (end-of-line) (insert ?\n))
-           )
-           (if (not (or fn
-                        (and (not (memq (following-char) '(?. ?')))
-                             (setq fn 'woman2-format-paragraphs))))
-               ()
-             ;; Find next control line:
-	     (if (equal woman-request "TS")
-		 (set-marker to (woman-find-next-control-line "TE"))
-	       (set-marker to (woman-find-next-control-line)))
-             ;; Call the appropriate function:
-             (funcall fn to)))
-      (if (not (eobp))			; This should not happen, but ...
-	  (woman2-format-paragraphs (copy-marker (point-max) t)
-                                    woman-left-margin))
+        (progn
+          (while
+              ;; Find next control line:
+              (re-search-forward woman-request-regexp nil t)
+            (cond
+             ;; Construct woman function to call:
+             ((setq fn (intern-soft
+                        (concat "woman2-"
+                                (setq woman-request (match-string 1)))))
+              ;; Delete request or macro name:
+              (woman-delete-match 0))
+             ;; Unrecognized request:
+             ((prog1 nil
+                ;; (WoMan-warn ".%s request ignored!" woman-request)
+                (WoMan-warn-ignored woman-request "ignored!")
+                ;; (setq fn 'woman2-LP)
+                ;; AVOID LEAVING A BLANK LINE!
+                ;; (setq fn 'woman2-format-paragraphs)
+                ))
+             ;; .LP assumes it is at eol and leaves a (blank) line,
+             ;; so leave point at end of line before paragraph:
+             ((or (looking-at "[ \t]*$") ; no argument
+                  woman-ignore)          ; ignore all
+              ;; (beginning-of-line) (kill-line)
+              ;; AVOID LEAVING A BLANK LINE!
+              (beginning-of-line) (woman-delete-line 1))
+             (t (end-of-line) (insert ?\n))
+             )
+            (if (not (or fn
+                         (and (not (memq (following-char) '(?. ?')))
+                              (setq fn 'woman2-format-paragraphs))))
+                ()
+              ;; Find next control line:
+              (if (equal woman-request "TS")
+                  (set-marker to (woman-find-next-control-line "TE"))
+                (set-marker to (woman-find-next-control-line)))
+              ;; Call the appropriate function:
+              (funcall fn to)))
+          (if (not (eobp))             ; This should not happen, but ...
+              (woman2-format-paragraphs (copy-marker (point-max) t)
+                                        woman-left-margin)))
       (fset 'canonically-space-region canonically-space-region)
       (fset 'set-text-properties set-text-properties)
       (fset 'insert-and-inherit insert-and-inherit)