bug#14480: [PATCH] A few error to user-error replacements

bug#14480: [PATCH] A few error to user-error replacements

[Kelly Dean]

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

I put (setq debug-on-error t) in my init.el recently and started hitting some of these.
Patch attached.
There are over 8000 occurrences "(error" in emacs-24.3/lisp and subdirs, probably a lot that should be replaced; should we report them every time we hit new ones or maybe queue for a few months to avoid spamming the bug db?

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: user-error.patch --]
[-- Type: text/x-diff; name="user-error.patch", Size: 1877 bytes --]

--- emacs-24.3/lisp/help-mode.el
+++ emacs-24.3/lisp/help-mode.el
@@ -738,14 +738,14 @@
   (interactive)
   (if help-xref-stack
       (help-xref-go-back (current-buffer))
-    (error "No previous help buffer")))
+    (user-error "No previous help buffer")))
 
 (defun help-go-forward ()
   "Go back to next topic in this help buffer."
   (interactive)
   (if help-xref-forward-stack
       (help-xref-go-forward (current-buffer))
-    (error "No next help buffer")))
+    (user-error "No next help buffer")))
 
 (defun help-do-xref (_pos function args)
   "Call the help cross-reference function FUNCTION with args ARGS.
@@ -768,7 +768,7 @@
 
 For the cross-reference format, see `help-make-xrefs'."
   (interactive)
-  (error "No cross-reference here"))
+  (user-error "No cross-reference here"))
 
 (defun help-follow-symbol (&optional pos)
   "In help buffer, show docs for symbol at POS, defaulting to point.
--- emacs-24.3/lisp/simple.el
+++ emacs-24.3/lisp/simple.el
@@ -3537,7 +3537,7 @@
 doc string for `insert-for-yank-1', which see."
   (interactive "*p")
   (if (not (eq last-command 'yank))
-      (error "Previous command was not a yank"))
+      (user-error "Previous command was not a yank"))
   (setq this-command 'yank)
   (unless arg (setq arg 1))
   (let ((inhibit-read-only t)
@@ -4107,7 +4107,7 @@
 \(Does not affect global mark ring\)."
   (interactive)
   (if (null (mark t))
-      (error "No mark set in this buffer")
+      (user-error "No mark set in this buffer")
     (if (= (point) (mark t))
 	(message "Mark popped"))
     (goto-char (mark t))
@@ -4269,7 +4269,7 @@
   (let ((omark (mark t))
 	(temp-highlight (eq (car-safe transient-mark-mode) 'only)))
     (if (null omark)
-        (error "No mark set in this buffer"))
+        (user-error "No mark set in this buffer"))
     (deactivate-mark)
     (set-mark (point))
     (goto-char omark)

bug#14480: [PATCH2] Re: bug#14480: A few error to user-error replacements

[Kelly Dean]

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

Here are some more. Additional patch attached.

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: user-error2.patch --]
[-- Type: text/x-diff; name="user-error2.patch", Size: 5564 bytes --]

--- emacs-24.3/lisp/winner.el
+++ emacs-24.3/lisp/winner.el
@@ -436,7 +436,7 @@
        (ring-ref winner-pending-undo-ring 0)))
     (unless (eq (selected-window) (minibuffer-window))
       (message "Winner undid undo")))
-   (t (error "Previous command was not a `winner-undo'"))))
+   (t (user-error "Previous command was not a `winner-undo'"))))
 
 (provide 'winner)
 ;;; winner.el ends here
--- emacs-24.3/lisp/windmove.el
+++ emacs-24.3/lisp/windmove.el
@@ -486,10 +486,10 @@
 If no window is at direction DIR, an error is signaled."
   (let ((other-window (windmove-find-other-window dir arg window)))
     (cond ((null other-window)
-           (error "No window %s from selected window" dir))
+           (user-error "No window %s from selected window" dir))
           ((and (window-minibuffer-p other-window)
                 (not (minibuffer-window-active-p other-window)))
-           (error "Minibuffer is inactive"))
+           (user-error "Minibuffer is inactive"))
           (t
            (select-window other-window)))))
 
--- emacs-24.3/lisp/register.el
+++ emacs-24.3/lisp/register.el
@@ -163,7 +163,7 @@
       (goto-char (cadr val)))
      ((markerp val)
       (or (marker-buffer val)
-	  (error "That register's buffer no longer exists"))
+	  (user-error "That register's buffer no longer exists"))
       (switch-to-buffer (marker-buffer val))
       (goto-char val))
      ((and (consp val) (eq (car val) 'file))
@@ -171,7 +171,7 @@
      ((and (consp val) (eq (car val) 'file-query))
       (or (find-buffer-visiting (nth 1 val))
 	  (y-or-n-p (format "Visit file %s again? " (nth 1 val)))
-	  (error "Register access aborted"))
+	  (user-error "Register access aborted"))
       (find-file (nth 1 val))
       (goto-char (nth 2 val)))
      ((and (fboundp 'semantic-foreign-tag-p)
@@ -180,7 +180,7 @@
       (switch-to-buffer (semantic-tag-buffer val))
       (goto-char (semantic-tag-start val)))
      (t
-      (error "Register doesn't contain a buffer position or configuration")))))
+      (user-error "Register doesn't contain a buffer position or configuration")))))
 
 (defun register-swap-out ()
   "Turn markers into file-query references when a buffer is killed."
@@ -226,7 +226,7 @@
 	(set-register register (+ number register-val))))
      ((or (not register-val) (stringp register-val))
       (append-to-register register (region-beginning) (region-end) prefix))
-     (t (error "Register does not contain a number or text")))))
+     (t (user-error "Register does not contain a number or text")))))
 
 (defun view-register (register)
   "Display what is contained in register named REGISTER.
@@ -354,7 +354,7 @@
 	   (semantic-foreign-tag-p val))
       (semantic-insert-foreign-tag val))
      (t
-      (error "Register does not contain text"))))
+      (user-error "Register does not contain text"))))
   (if (not arg) (exchange-point-and-mark)))
 
 (defun copy-to-register (register start end &optional delete-flag)
@@ -382,7 +382,7 @@
     (set-register
      register (cond ((not reg) text)
                     ((stringp reg) (concat reg separator text))
-                    (t (error "Register does not contain text")))))
+                    (t (user-error "Register does not contain text")))))
   (setq deactivate-mark t)
   (cond (delete-flag
 	 (delete-region start end))
@@ -401,7 +401,7 @@
     (set-register
      register (cond ((not reg) text)
                     ((stringp reg) (concat text separator reg))
-                    (t (error "Register does not contain text")))))
+                    (t (user-error "Register does not contain text")))))
   (setq deactivate-mark t)
   (cond (delete-flag
 	 (delete-region start end))
--- emacs-24.3/lisp/files.el
+++ emacs-24.3/lisp/files.el
@@ -1539,10 +1539,10 @@
             (confirm-nonexistent-file-or-buffer) file-name)
 	   t)))
   (unless (run-hook-with-args-until-failure 'kill-buffer-query-functions)
-    (error "Aborted"))
+    (user-error "Aborted"))
   (and (buffer-modified-p) buffer-file-name
        (not (yes-or-no-p "Kill and replace the buffer without saving it? "))
-       (error "Aborted"))
+       (user-error "Aborted"))
   (let ((obuf (current-buffer))
 	(ofile buffer-file-name)
 	(onum buffer-file-number)
@@ -1741,7 +1741,7 @@
 	     (not (y-or-n-p (format "File %s is large (%s), really %s? "
 				    (file-name-nondirectory filename)
 				    (file-size-human-readable size) op-type))))
-    (error "Aborted")))
+    (user-error "Aborted")))
 
 (defun find-file-noselect (filename &optional nowarn rawfile wildcards)
   "Read file FILENAME into a buffer and return the buffer.
@@ -3732,7 +3732,7 @@
 	   (not no-query)
 	   (not (y-or-n-p (format "A buffer is visiting %s; proceed? "
 				  filename)))
-	   (error "Aborted")))
+	   (user-error "Aborted")))
     (or (equal filename buffer-file-name)
 	(progn
 	  (and filename (lock-buffer filename))
@@ -3852,7 +3852,7 @@
 	(and confirm
 	     (file-exists-p filename)
 	     (or (y-or-n-p (format "File `%s' exists; overwrite? " filename))
-		 (error "Canceled")))
+		 (user-error "Canceled")))
 	(set-visited-file-name filename (not confirm))))
   (set-buffer-modified-p t)
   ;; Make buffer writable if file is writable.
@@ -4502,7 +4502,7 @@
 			(error "%s is a directory" filename)
 		      (unless (y-or-n-p (format "File `%s' exists; overwrite? "
 						filename))
-			(error "Canceled")))
+			(user-error "Canceled")))
 		  ;; Signal an error if the specified name refers to a
 		  ;; non-existing directory.
 		  (let ((dir (file-name-directory filename)))

bug#14480: [PATCH3] Re: bug#14480: A few error to user-error replacements

[Kelly Dean]

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

Here's another one:
(setq debug-on-error t)
M-x M-x
It enters debugger.
Also:
M-x C-x o M-x
It enters debugger.

The attached patch fixes it in both cases. It also applies to trunk; just ignore the hunk for undo.c (user_error is already deleted from undo.c in trunk, but the hunk is needed for 24.3 to prevent a compiler error).

Also, in primitive-undo in simple.el in trunk, the 4 occurrences of
(error "Changes to be undone are outside visible portion of buffer")
might be a regression; primitive-undo in undo.c in 24.3 uses user_error in all 4 cases. But I haven't checked to make sure; maybe they really are program errors, not user errors.

Also, between 24.3 and trunk, bitch_at_user in dispnew.c changed from using the error function to manually signalling a user error. It should just use user_error, which does the same thing.

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: user-error3.patch --]
[-- Type: text/x-diff; name="user-error3.patch", Size: 1686 bytes --]

--- emacs-24.3/src/lisp.h
+++ emacs-24.3/src/lisp.h
@@ -3148,6 +3148,7 @@
 extern _Noreturn void error (const char *, ...) ATTRIBUTE_FORMAT_PRINTF (1, 2);
 extern _Noreturn void verror (const char *, va_list)
   ATTRIBUTE_FORMAT_PRINTF (1, 0);
+extern _Noreturn void user_error (const char *);
 extern Lisp_Object un_autoload (Lisp_Object);
 extern Lisp_Object call_debugger (Lisp_Object arg);
 extern void init_eval_once (void);
--- emacs-24.3/src/keyboard.c
+++ emacs-24.3/src/keyboard.c
@@ -782,7 +782,7 @@
   /* Handle throw from read_minibuf when using minibuffer
      while it's active but we're in another window.  */
   if (STRINGP (val))
-    xsignal1 (Qerror, val);
+    xsignal1 (Quser_error, val);
 
   return unbind_to (count, Qnil);
 }
@@ -1207,7 +1207,7 @@
   Fthrow (Qtop_level, Qnil);
 }
 
-static _Noreturn void
+_Noreturn void
 user_error (const char *msg)
 {
   xsignal1 (Quser_error, build_string (msg));
--- emacs-24.3/src/minibuf.c
+++ emacs-24.3/src/minibuf.c
@@ -449,7 +449,7 @@
       && minibuf_level > 0)
     {
       if (EQ (selected_window, minibuf_window))
-	error ("Command attempted to use minibuffer while in minibuffer");
+	user_error ("Command attempted to use minibuffer while in minibuffer");
       else
 	/* If we're in another window, cancel the minibuffer that's active.  */
 	Fthrow (Qexit,
--- emacs-24.3/src/undo.c
+++ emacs-24.3/src/undo.c
@@ -445,12 +445,6 @@
   unbind_to (count, Qnil);
 }
 
-static _Noreturn void
-user_error (const char *msg)
-{
-  xsignal1 (Quser_error, build_string (msg));
-}
-
 \f
 DEFUN ("primitive-undo", Fprimitive_undo, Sprimitive_undo, 2, 2, 0,
        doc: /* Undo N records from the front of the list LIST.

bug#14480: A few error to user-error replacements

[Kelly Dean]

A couple more: in emacs-24.3/lisp/emacs-lisp/lisp.el, the calls to error on lines 187 and 618 should be calls to user-error. Can't say current line numbers in trunk, since they'll probably change by the time somebody reads this, and can't provide context via diff since the FSF requested that I not send any more patches
(see http://debbugs.gnu.org/cgi/bugreport.cgi?bug=14492).

Reporting these minor things in prose seems not worth the effort. Should users even be reporting them? Or is some computer-assisted error->user-error bulk conversion going to take place, and reporting individual ones is unnecessary?

bug#14480: [PATCH] A few error to user-error replacements

[Glenn Morris]

Thanks for the user-error patches, but as you know we can't use them
without a copyright assignment. Since we are aware that it is desirable
to replace error by user-eror where applicable, I don't think it is
necessary to keep this bug report open.