unofficial mirror of bug-gnu-emacs@gnu.org 
 help / color / mirror / code / Atom feed
* bug#30908: woman2-roff-buffer fails to restore set-text-properties, etc. on error
@ 2018-03-22 18:12 Ivan Shmakov
  2018-03-23  0:52 ` Noam Postavsky
  0 siblings, 1 reply; 5+ messages in thread
From: Ivan Shmakov @ 2018-03-22 18:12 UTC (permalink / raw)
  To: 30908

[-- Attachment #1: Type: text/plain, Size: 735 bytes --]

Package: emacs
Version: 25.1.1
Tags: patch

	The woman2-roff-buffer function uses unwind-protect to restore
	the canonically-space-region, insert-and-inherit and
	set-text-properties functions it temporarily overrides.

	Unfortunately, the first thing (conditionally) called in the
	‘unwind’ branch is the woman2-format-paragraphs function, which
	may result in error, thus precluding said restoration, leading to:

set-text-properties is an alias for ‘ignore’.

(set-text-properties &rest IGNORE)

Do nothing and return nil.
This function accepts any number of arguments, but ignores them.

	Please thus consider the patch MIMEd.

-- 
FSF associate member #7257  np. Sacred Armor of Antiriad — Traxer

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: Type: text/diff, Size: 1211 bytes --]

--- woman.el.~1~	2017-11-05 15:00:52.696647297 +0000
+++ woman.el	2018-03-22 17:47:44.014986089 +0000
@@ -3710,13 +3710,14 @@ defun woman2-roff-buffer ()
 	       (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)
-      (set-marker to nil))))
+      (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)))))
 
 (defun woman-find-next-control-line (&optional pat)
   "Find and return start of next control line.

^ permalink raw reply	[flat|nested] 5+ messages in thread

* bug#30908: woman2-roff-buffer fails to restore set-text-properties, etc. on error
  2018-03-22 18:12 bug#30908: woman2-roff-buffer fails to restore set-text-properties, etc. on error Ivan Shmakov
@ 2018-03-23  0:52 ` Noam Postavsky
  2018-03-23  3:24   ` Ivan Shmakov
  0 siblings, 1 reply; 5+ messages in thread
From: Noam Postavsky @ 2018-03-23  0:52 UTC (permalink / raw)
  To: Ivan Shmakov; +Cc: 30908

Ivan Shmakov <ivan@siamics.net> writes:

> --- woman.el.~1~	2017-11-05 15:00:52.696647297 +0000
> +++ woman.el	2018-03-22 17:47:44.014986089 +0000
> @@ -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?





^ permalink raw reply	[flat|nested] 5+ messages in thread

* bug#30908: woman2-roff-buffer fails to restore set-text-properties, etc. on error
  2018-03-23  0:52 ` Noam Postavsky
@ 2018-03-23  3:24   ` Ivan Shmakov
  2018-03-23 10:20     ` Noam Postavsky
  0 siblings, 1 reply; 5+ messages in thread
From: Ivan Shmakov @ 2018-03-23  3:24 UTC (permalink / raw)
  To: 30908; +Cc: Noam Postavsky

[-- Attachment #1: Type: text/plain, Size: 1098 bytes --]

>>>>> Noam Postavsky <npostavs@gmail.com> writes:
>>>>> Ivan Shmakov <ivan@siamics.net> 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/

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: Type: text/diff, Size: 4203 bytes --]

--- -	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)

^ permalink raw reply	[flat|nested] 5+ messages in thread

* bug#30908: woman2-roff-buffer fails to restore set-text-properties, etc. on error
  2018-03-23  3:24   ` Ivan Shmakov
@ 2018-03-23 10:20     ` Noam Postavsky
  2018-04-26 12:01       ` Noam Postavsky
  0 siblings, 1 reply; 5+ messages in thread
From: Noam Postavsky @ 2018-03-23 10:20 UTC (permalink / raw)
  To: Ivan Shmakov; +Cc: 30908

Ivan Shmakov <ivan@siamics.net> writes:

>  > 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.)

Hmm, looking at this again, I'm not sure.  That is, it's clearly
fallback processing in case the loop ends before going through the whole
buffer.  But does it also make sense as fallback processing when some
kind of error was signaled in the loop?  I don't know.






^ permalink raw reply	[flat|nested] 5+ messages in thread

* bug#30908: woman2-roff-buffer fails to restore set-text-properties, etc. on error
  2018-03-23 10:20     ` Noam Postavsky
@ 2018-04-26 12:01       ` Noam Postavsky
  0 siblings, 0 replies; 5+ messages in thread
From: Noam Postavsky @ 2018-04-26 12:01 UTC (permalink / raw)
  To: Ivan Shmakov; +Cc: 30908

tags 30908 fixed
close 30908 27.1
quit

Noam Postavsky <npostavs@gmail.com> writes:

> Ivan Shmakov <ivan@siamics.net> writes:
>
>>  > 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.)
>
> Hmm, looking at this again, I'm not sure.  That is, it's clearly
> fallback processing in case the loop ends before going through the whole
> buffer.  But does it also make sense as fallback processing when some
> kind of error was signaled in the loop?  I don't know.

I've decided that it doesn't make sense to do any further processing
once we've hit an error.

[1: 66dbb787a2]: 2018-04-26 07:37:48 -0400
  Ensure woman2-roff-buffer restores functions on error (Bug#30908)
  https://git.savannah.gnu.org/cgit/emacs.git/commit/?id=66dbb787a22d4ae1d513a3ee27e22eed395f5676





^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2018-04-26 12:01 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-03-22 18:12 bug#30908: woman2-roff-buffer fails to restore set-text-properties, etc. on error Ivan Shmakov
2018-03-23  0:52 ` Noam Postavsky
2018-03-23  3:24   ` Ivan Shmakov
2018-03-23 10:20     ` Noam Postavsky
2018-04-26 12:01       ` Noam Postavsky

Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).