bug#6106: 24.0.50; info.el adds 'search-failed to debug-ignored-errors

bug#6106: 24.0.50; info.el adds 'search-failed to debug-ignored-errors

[Helmut Eller]

Around line 4833 in info.el 'search-failed is added to debug-ignored-errors.

That seems counterproductive to me.  I would not expect at all that
viewing an info file would change the way the debugger works.  It would
be less annoying if info mode would only modify the buffer-local binding
of debug-ignored-errors.

bug#6106: 24.0.50; info.el adds 'search-failed to debug-ignored-errors

[Juri Linkov]

> Around line 4833 in info.el 'search-failed is added to debug-ignored-errors.
>
> That seems counterproductive to me.  I would not expect at all that
> viewing an info file would change the way the debugger works.  It would
> be less annoying if info mode would only modify the buffer-local binding
> of debug-ignored-errors.

We are waiting when Lennart will post a newer version of the patch that
introduces new functions `user-error' and `user-signal'.  They will
obsolete `debug-ignored-errors'.  And info.el will call
(user-signal 'search-failed ...)

-- 
Juri Linkov
http://www.jurta.org/emacs/

bug#6106: 24.0.50; info.el adds 'search-failed to debug-ignored-errors

[Helmut Eller]

* Juri Linkov [2010-05-05 20:18+0200] writes:

>> Around line 4833 in info.el 'search-failed is added to debug-ignored-errors.
>>
>> That seems counterproductive to me.  I would not expect at all that
>> viewing an info file would change the way the debugger works.  It would
>> be less annoying if info mode would only modify the buffer-local binding
>> of debug-ignored-errors.
>
> We are waiting when Lennart will post a newer version of the patch that
> introduces new functions `user-error' and `user-signal'.  They will
> obsolete `debug-ignored-errors'.  And info.el will call
> (user-signal 'search-failed ...)

OK. That sounds good.

Helmut

bug#6106: 24.0.50; info.el adds 'search-failed to debug-ignored-errors

[Stefan Monnier]

> We are waiting when Lennart will post a newer version of the patch that
> introduces new functions `user-error' and `user-signal'.  They will
> obsolete `debug-ignored-errors'.  And info.el will call
> (user-signal 'search-failed ...)

Hmm... that would mean that his patch goes quite a bit further than what
I expected (I think he only provides `user-error' and not
`user-signal').


        Stefan

bug#6106: 24.0.50; info.el adds 'search-failed to debug-ignored-errors

[Juri Linkov]

[Cc:ed Lennart]

Lennart, could you please tell at what stage your patch currently is
and how it handles user-level signals?

>> We are waiting when Lennart will post a newer version of the patch that
>> introduces new functions `user-error' and `user-signal'.  They will
>> obsolete `debug-ignored-errors'.  And info.el will call
>> (user-signal 'search-failed ...)
>
> Hmm... that would mean that his patch goes quite a bit further than what
> I expected (I think he only provides `user-error' and not
> `user-signal').

-- 
Juri Linkov
http://www.jurta.org/emacs/

bug#6106: 24.0.50; info.el adds 'search-failed to debug-ignored-errors

[Juri Linkov]

> Around line 4833 in info.el 'search-failed is added to debug-ignored-errors.
>
> That seems counterproductive to me.  I would not expect at all that
> viewing an info file would change the way the debugger works.  It would
> be less annoying if info mode would only modify the buffer-local binding
> of debug-ignored-errors.

`search-failed' is added to `debug-ignored-errors' in info.el
to not enter the debugger when a normal search stops with no results.

But this error is raised by `xsignal1 (Qsearch_failed, string)'
in `re-search-forward' (called from `Info-search'),
so I see no way to replace it with a call to the new function
`user-error' (or the same function with any other name).

bug#6106: 24.0.50; info.el adds 'search-failed to debug-ignored-errors

[npostavs]

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

tags 6106 patch
quit

Juri Linkov <juri@jurta.org> writes:

> `search-failed' is added to `debug-ignored-errors' in info.el
> to not enter the debugger when a normal search stops with no results.
>
> But this error is raised by `xsignal1 (Qsearch_failed, string)'
> in `re-search-forward' (called from `Info-search'),
> so I see no way to replace it with a call to the new function
> `user-error' (or the same function with any other name).

Use the NOERROR parameter?


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

From f0b7b943368e9856dda49c3bdb862c1940266189 Mon Sep 17 00:00:00 2001
From: Noam Postavsky <npostavs@gmail.com>
Date: Thu, 23 Mar 2017 23:54:58 -0400
Subject: [PATCH v1] Don't add `search-failed' to ignored errors in info.el
 (Bug#6106)

* lisp/info.el: Stop adding `search-failed' to `debug-ignored-errors'.
(Info-select-node, Info-search): Replace (signal 'search-failed ...)
with (user-error "Search failed: "...).
---
 lisp/info.el | 23 ++++++++++-------------
 1 file changed, 10 insertions(+), 13 deletions(-)

diff --git a/lisp/info.el b/lisp/info.el
index 8dc85f48a4..a06c868fb3 100644
--- a/lisp/info.el
+++ b/lisp/info.el
@@ -1672,7 +1672,7 @@ Info-select-node
 	  (forward-line 2)
 	(if (looking-at "\^_")
 	    (forward-line 1)
-	  (signal 'search-failed (list "\n\^_"))))
+	  (user-error "Search failed: `\n\^_'")))
       ;; Get nodename spelled as it is in the node.
       (re-search-forward "Node:[ \t]*")
       (setq Info-current-node
@@ -1998,19 +1998,20 @@ Info-search
                   Info-isearch-initial-node
                   bound
                   (and found (> found opoint-min) (< found opoint-max)))
-	(signal 'search-failed (list regexp "end of node")))
+	(user-error "Search failed: `%s' (end of node)" regexp))
 
       ;; If no subfiles, give error now.
       (unless (or found Info-current-subfile)
         (if isearch-mode
-            (signal 'search-failed (list regexp "end of manual"))
+            (user-error "Search failed: `%s' (end of manual)" regexp)
           (let ((search-spaces-regexp Info-search-whitespace-regexp))
-            (if backward
-                (re-search-backward regexp)
-              (re-search-forward regexp)))))
+            (unless (if backward
+                        (re-search-backward regexp nil t)
+                      (re-search-forward regexp nil t))
+              (user-error "Search failed: `%s'" regexp)))))
 
       (if (and bound (not found))
-	  (signal 'search-failed (list regexp)))
+          (user-error "Search failed: `%s'" regexp))
 
       (unless (or found bound)
 	(unwind-protect
@@ -2054,9 +2055,8 @@ Info-search
 		    (setq list nil)))
 	      (if found
 		  (message "")
-		(signal 'search-failed (if isearch-mode
-					   (list regexp "end of manual")
-					 (list regexp)))))
+                (user-error "Search failed: `%s'%s"
+                            regexp (if isearch-mode " (end of manual)" ""))))
 	  (if (not found)
 	      (progn (Info-read-subfile osubfile)
 		     (goto-char opoint)
@@ -5237,9 +5237,6 @@ Info-speedbar-buttons
       (erase-buffer))
   (Info-speedbar-hierarchy-buttons nil 0))
 
-;; FIXME: Really?  Why here?
-(add-to-list 'debug-ignored-errors 'search-failed)
-
 ;;;;  Desktop support
 
 (defun Info-desktop-buffer-misc-data (_desktop-dirname)
-- 
2.11.1

bug#6106: 24.0.50; info.el adds 'search-failed to debug-ignored-errors

[Juri Linkov]

>> `search-failed' is added to `debug-ignored-errors' in info.el
>> to not enter the debugger when a normal search stops with no results.
>>
>> But this error is raised by `xsignal1 (Qsearch_failed, string)'
>> in `re-search-forward' (called from `Info-search'),
>> so I see no way to replace it with a call to the new function
>> `user-error' (or the same function with any other name).
>
> Use the NOERROR parameter?

Looks right, thanks.

bug#6106: 24.0.50; info.el adds 'search-failed to debug-ignored-errors

[npostavs]

tags 6106 fixed
close 6106 26.1
quit

Juri Linkov <juri@jurta.org> writes:

>>> `search-failed' is added to `debug-ignored-errors' in info.el
>>> to not enter the debugger when a normal search stops with no results.
>>>
>>> But this error is raised by `xsignal1 (Qsearch_failed, string)'
>>> in `re-search-forward' (called from `Info-search'),
>>> so I see no way to replace it with a call to the new function
>>> `user-error' (or the same function with any other name).
>>
>> Use the NOERROR parameter?
>
> Looks right, thanks.

Thanks for the review, push to master [1: a62437d4b9].

1: 2017-03-27 20:18:20 -0400 a62437d4b933825a92ac3836836edbca3540fa1c
  Don't add `search-failed' to ignored errors in info.el (Bug#6106)

bug#6106: 24.0.50; info.el adds 'search-failed to debug-ignored-errors

[npostavs]

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

npostavs@users.sourceforge.net writes:

> (Info-select-node, Info-search): Replace (signal 'search-failed ...)
> with (user-error "Search failed: "...).

Turns out this breaks isearch over multiple nodes, as noted by Mark
Oteiza in [1].  I believe this is fixed by the patch below.

[1]: http://lists.gnu.org/archive/html/emacs-devel/2017-03/msg00869.html


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

From 2cb893febdf06282c1183b3e4f4de8728bbe44b9 Mon Sep 17 00:00:00 2001
From: Noam Postavsky <npostavs@gmail.com>
Date: Sat, 1 Apr 2017 09:34:04 -0400
Subject: [PATCH v1] Throw a `search-failed' derived error in Info search

The original fix for Bug#6106 switched from signalling `search-failed'
to `user-error'.  However, this breaks incremental searching over
multiple nodes because the isearch code doesn't expect a `user-error'.

* src/search.c (syms_of_search): New error, `user-search-failed',
with `user-error' and `search-failed' as parents.
* lisp/info.el (Info-search): Use it instead of `user-error' so that
isearch will handle failed searches correctly.
---
 lisp/info.el | 12 ++++++------
 src/search.c |  9 +++++++++
 2 files changed, 15 insertions(+), 6 deletions(-)

diff --git a/lisp/info.el b/lisp/info.el
index a6bab290a7..81e5d29f82 100644
--- a/lisp/info.el
+++ b/lisp/info.el
@@ -1998,20 +1998,20 @@ Info-search
                   Info-isearch-initial-node
                   bound
                   (and found (> found opoint-min) (< found opoint-max)))
-	(user-error "Search failed: `%s' (end of node)" regexp))
+	(signal 'user-search-failed (list regexp "(end of node)")))
 
       ;; If no subfiles, give error now.
       (unless (or found Info-current-subfile)
         (if isearch-mode
-            (user-error "Search failed: `%s' (end of manual)" regexp)
+            (signal 'user-search-failed (list regexp "end of manual"))
           (let ((search-spaces-regexp Info-search-whitespace-regexp))
             (unless (if backward
                         (re-search-backward regexp nil t)
                       (re-search-forward regexp nil t))
-              (user-error "Search failed: `%s'" regexp)))))
+              (signal 'user-seach-failed (list regexp))))))
 
       (if (and bound (not found))
-          (user-error "Search failed: `%s'" regexp))
+          (signal 'user-search-failed (list regexp)))
 
       (unless (or found bound)
 	(unwind-protect
@@ -2055,8 +2055,8 @@ Info-search
 		    (setq list nil)))
 	      (if found
 		  (message "")
-                (user-error "Search failed: `%s'%s"
-                            regexp (if isearch-mode " (end of manual)" ""))))
+                (signal 'user-search-failed
+                        `(,regexp ,@(if isearch-mode '("end of manual"))))))
 	  (if (not found)
 	      (progn (Info-read-subfile osubfile)
 		     (goto-char opoint)
diff --git a/src/search.c b/src/search.c
index 33cb02aa7a..48caea2f51 100644
--- a/src/search.c
+++ b/src/search.c
@@ -3389,6 +3389,10 @@ syms_of_search (void)
   /* Error condition used for failing searches.  */
   DEFSYM (Qsearch_failed, "search-failed");
 
+  /* Error condition used for failing searches started by user, i.e.,
+     where failure should not invoke the debugger.  */
+  DEFSYM (Quser_search_failed, "search-failed");
+
   /* Error condition signaled when regexp compile_pattern fails.  */
   DEFSYM (Qinvalid_regexp, "invalid-regexp");
 
@@ -3397,6 +3401,11 @@ syms_of_search (void)
   Fput (Qsearch_failed, Qerror_message,
 	build_pure_c_string ("Search failed"));
 
+  Fput (Quser_search_failed, Qerror_conditions,
+	listn (CONSTYPE_PURE, 2, Qsearch_failed, Quser_error));
+  Fput (Quser_search_failed, Qerror_message,
+	build_pure_c_string ("Search failed"));
+
   Fput (Qinvalid_regexp, Qerror_conditions,
 	listn (CONSTYPE_PURE, 2, Qinvalid_regexp, Qerror));
   Fput (Qinvalid_regexp, Qerror_message,
-- 
2.11.1

bug#6106: 24.0.50; info.el adds 'search-failed to debug-ignored-errors

[Eli Zaretskii]

> From: npostavs@users.sourceforge.net
> Date: Sat, 01 Apr 2017 09:43:59 -0400
> Cc: Mark Oteiza <mvoteiza@udel.edu>, 6106@debbugs.gnu.org,
> 	Helmut Eller <eller.helmut@gmail.com>
> 
> Turns out this breaks isearch over multiple nodes, as noted by Mark
> Oteiza in [1].  I believe this is fixed by the patch below.
> 
> [1]: http://lists.gnu.org/archive/html/emacs-devel/2017-03/msg00869.html

Thanks.  I think the symbol you are introducing and its plist should
be documented in the ELisp manual and mentioned in NEWS.

bug#6106: 24.0.50; info.el adds 'search-failed to debug-ignored-errors

[Mark Oteiza]

On 01/04/17 at 09:43am, npostavs@users.sourceforge.net wrote:
> npostavs@users.sourceforge.net writes:
> 
> > (Info-select-node, Info-search): Replace (signal 'search-failed ...)
> > with (user-error "Search failed: "...).
> 
> Turns out this breaks isearch over multiple nodes, as noted by Mark
> Oteiza in [1].  I believe this is fixed by the patch below.
> 
> [1]: http://lists.gnu.org/archive/html/emacs-devel/2017-03/msg00869.html

Thanks for addressing.  I get the following with your patch:

  peculiar error: "cats", "(end of node)"

bug#6106: 24.0.50; info.el adds 'search-failed to debug-ignored-errors

[npostavs]

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

Eli Zaretskii <eliz@gnu.org> writes:
>
> Thanks.  I think the symbol you are introducing and its plist should
> be documented in the ELisp manual and mentioned in NEWS.

I added it to `(elisp) Standard Errors' after `user-error'.  Or should
it go next to `search-failed'?  It says the list is "grouped by concept"
though it seems to be alphabetical.

Mark Oteiza <mvoteiza@udel.edu> writes:

> On 01/04/17 at 09:43am, npostavs@users.sourceforge.net wrote:
>> npostavs@users.sourceforge.net writes:
>> 
>> > (Info-select-node, Info-search): Replace (signal 'search-failed ...)
>> > with (user-error "Search failed: "...).
>> 
>> Turns out this breaks isearch over multiple nodes, as noted by Mark
>> Oteiza in [1].  I believe this is fixed by the patch below.
>> 
>> [1]: http://lists.gnu.org/archive/html/emacs-devel/2017-03/msg00869.html
>
> Thanks for addressing.  I get the following with your patch:
>
>   peculiar error: "cats", "(end of node)"

Sorry, I messed up.  I did an initial implementation using
`define-error' in lisp, and I didn't retest after moving that part to
C.  Here's a corrected patch.


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

From bc0b21552dd782190662fe1fcbf7cd8197a7822e Mon Sep 17 00:00:00 2001
From: Noam Postavsky <npostavs@gmail.com>
Date: Sat, 1 Apr 2017 09:34:04 -0400
Subject: [PATCH v2] Throw a `search-failed' derived error in Info search

The original fix for Bug#6106 switched from signalling `search-failed'
to `user-error'.  However, this breaks incremental searching over
multiple nodes because the isearch code doesn't expect a `user-error'.

* src/search.c (syms_of_search): New error, `user-search-failed',
with `user-error' and `search-failed' as parents.
* doc/lispref/errors.texi (Standard Errors): Document it.
* etc/NEWS: Announce it.
* lisp/info.el (Info-search): Use it instead of `user-error' so that
isearch will handle failed searches correctly.
---
 doc/lispref/errors.texi |  6 ++++++
 etc/NEWS                |  4 ++++
 lisp/info.el            | 12 ++++++------
 src/search.c            | 10 ++++++++++
 4 files changed, 26 insertions(+), 6 deletions(-)

diff --git a/doc/lispref/errors.texi b/doc/lispref/errors.texi
index 2ec1a108ea..1f67819c34 100644
--- a/doc/lispref/errors.texi
+++ b/doc/lispref/errors.texi
@@ -186,6 +186,12 @@ Standard Errors
 @item user-error
 The message is the empty string.  @xref{Signaling Errors}.
 
+@item user-search-failed
+This is like @samp{search-failed}, but doesn't trigger the debugger,
+like @samp{user-error}.  @xref{Signaling Errors}, and @xref{Searching
+and Matching}.  This is used for searching in Info files, @xref{Search
+Text,,,info,Info}.
+
 @item void-function
 The message is @samp{Symbol's function definition is void}.
 @xref{Function Cells}.
diff --git a/etc/NEWS b/etc/NEWS
index c09cc390bb..f4627ac3a6 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -1067,6 +1067,10 @@ its window gets deleted by 'delete-other-windows'.
 *** New command 'window-swap-states' swaps the states of two live
 windows.
 
++++
+*** New error type 'user-search-failed' like 'search-failed' but
+avoids debugger like 'user-error'.
+
 \f
 * Changes in Emacs 26.1 on Non-Free Operating Systems
 
diff --git a/lisp/info.el b/lisp/info.el
index a06c868fb3..28534f13e6 100644
--- a/lisp/info.el
+++ b/lisp/info.el
@@ -1998,20 +1998,20 @@ Info-search
                   Info-isearch-initial-node
                   bound
                   (and found (> found opoint-min) (< found opoint-max)))
-	(user-error "Search failed: `%s' (end of node)" regexp))
+	(signal 'user-search-failed (list regexp "(end of node)")))
 
       ;; If no subfiles, give error now.
       (unless (or found Info-current-subfile)
         (if isearch-mode
-            (user-error "Search failed: `%s' (end of manual)" regexp)
+            (signal 'user-search-failed (list regexp "end of manual"))
           (let ((search-spaces-regexp Info-search-whitespace-regexp))
             (unless (if backward
                         (re-search-backward regexp nil t)
                       (re-search-forward regexp nil t))
-              (user-error "Search failed: `%s'" regexp)))))
+              (signal 'user-seach-failed (list regexp))))))
 
       (if (and bound (not found))
-          (user-error "Search failed: `%s'" regexp))
+          (signal 'user-search-failed (list regexp)))
 
       (unless (or found bound)
 	(unwind-protect
@@ -2055,8 +2055,8 @@ Info-search
 		    (setq list nil)))
 	      (if found
 		  (message "")
-                (user-error "Search failed: `%s'%s"
-                            regexp (if isearch-mode " (end of manual)" ""))))
+                (signal 'user-search-failed
+                        `(,regexp ,@(if isearch-mode '("end of manual"))))))
 	  (if (not found)
 	      (progn (Info-read-subfile osubfile)
 		     (goto-char opoint)
diff --git a/src/search.c b/src/search.c
index 33cb02aa7a..c0deb57213 100644
--- a/src/search.c
+++ b/src/search.c
@@ -3389,6 +3389,10 @@ syms_of_search (void)
   /* Error condition used for failing searches.  */
   DEFSYM (Qsearch_failed, "search-failed");
 
+  /* Error condition used for failing searches started by user, i.e.,
+     where failure should not invoke the debugger.  */
+  DEFSYM (Quser_search_failed, "user-search-failed");
+
   /* Error condition signaled when regexp compile_pattern fails.  */
   DEFSYM (Qinvalid_regexp, "invalid-regexp");
 
@@ -3397,6 +3401,12 @@ syms_of_search (void)
   Fput (Qsearch_failed, Qerror_message,
 	build_pure_c_string ("Search failed"));
 
+  Fput (Quser_search_failed, Qerror_conditions,
+        listn (CONSTYPE_PURE, 4,
+               Quser_search_failed, Quser_error, Qsearch_failed, Qerror));
+  Fput (Quser_search_failed, Qerror_message,
+        build_pure_c_string ("Search failed"));
+
   Fput (Qinvalid_regexp, Qerror_conditions,
 	listn (CONSTYPE_PURE, 2, Qinvalid_regexp, Qerror));
   Fput (Qinvalid_regexp, Qerror_message,
-- 
2.11.1

bug#6106: 24.0.50; info.el adds 'search-failed to debug-ignored-errors

[Eli Zaretskii]

> From: npostavs@users.sourceforge.net
> Cc: 6106@debbugs.gnu.org,  juri@jurta.org,  mvoteiza@udel.edu,  eller.helmut@gmail.com
> Date: Sat, 01 Apr 2017 17:08:42 -0400
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> >
> > Thanks.  I think the symbol you are introducing and its plist should
> > be documented in the ELisp manual and mentioned in NEWS.
> 
> I added it to `(elisp) Standard Errors' after `user-error'.  Or should
> it go next to `search-failed'?  It says the list is "grouped by concept"
> though it seems to be alphabetical.

It's not exactly alphabetical, so after user-error is fine, IMO.

bug#6106: 24.0.50; info.el adds 'search-failed to debug-ignored-errors

[Mark Oteiza]

On 01/04/17 at 05:08pm, npostavs@users.sourceforge.net wrote:
> Mark Oteiza <mvoteiza@udel.edu> writes:
> 
> > On 01/04/17 at 09:43am, npostavs@users.sourceforge.net wrote:
> >> npostavs@users.sourceforge.net writes:
> >> 
> >> > (Info-select-node, Info-search): Replace (signal 'search-failed ...)
> >> > with (user-error "Search failed: "...).
> >> 
> >> Turns out this breaks isearch over multiple nodes, as noted by Mark
> >> Oteiza in [1].  I believe this is fixed by the patch below.
> >> 
> >> [1]: http://lists.gnu.org/archive/html/emacs-devel/2017-03/msg00869.html
> >
> > Thanks for addressing.  I get the following with your patch:
> >
> >   peculiar error: "cats", "(end of node)"
> 
> Sorry, I messed up.  I did an initial implementation using
> `define-error' in lisp, and I didn't retest after moving that part to
> C.  Here's a corrected patch.

Works for me--thank you for the fix.

bug#6106: 24.0.50; info.el adds 'search-failed to debug-ignored-errors

[npostavs]

Mark Oteiza <mvoteiza@udel.edu> writes:
>
> Works for me--thank you for the fix.

Thanks for testing, pushed to master [2: 3887c54544].

2: 2017-04-03 19:36:14 -0400 3887c54544bc2e5f8c2e7c12973887f9b2b88c40
  Throw a `search-failed' derived error in Info search