bug#32990: 26.1.50; isearch-forward + t-m-m/mark-active doc

classic Classic list List threaded Threaded
8 messages Options
Reply | Threaded
Open this post in threaded view
|

bug#32990: 26.1.50; isearch-forward + t-m-m/mark-active doc

Charles A. Roelli
Another small change for emacs-26:

diff --git a/lisp/isearch.el b/lisp/isearch.el
index 3725779..7833a7b 100644
--- a/lisp/isearch.el
+++ b/lisp/isearch.el
@@ -719,6 +719,10 @@ isearch-forward
 If you try to exit with the search string still empty, it invokes
  nonincremental search.
 
+Exiting pushes the mark where point was before the search
+started, unless `transient-mark-mode' is on and the mark is
+active.
+
 Type \\[isearch-toggle-case-fold] to toggle search case-sensitivity.
 Type \\[isearch-toggle-invisible] to toggle search in invisible text.
 Type \\[isearch-toggle-regexp] to toggle regular-expression mode.


This behavior is already documented in the manual node "Basic
Isearch".



Reply | Threaded
Open this post in threaded view
|

bug#32990: 26.1.50; isearch-forward + t-m-m/mark-active doc

Eli Zaretskii
> Date: Mon, 08 Oct 2018 19:58:15 +0200
> From: [hidden email] (Charles A. Roelli)
>
> Another small change for emacs-26:
>
> diff --git a/lisp/isearch.el b/lisp/isearch.el
> index 3725779..7833a7b 100644
> --- a/lisp/isearch.el
> +++ b/lisp/isearch.el
> @@ -719,6 +719,10 @@ isearch-forward
>  If you try to exit with the search string still empty, it invokes
>   nonincremental search.
>  
> +Exiting pushes the mark where point was before the search
> +started, unless `transient-mark-mode' is on and the mark is
> +active.
> +

Are you sure this is important to say there?  The doc string of that
function is already way beyond annoyingly long.

I'd actually encourage the opposite trend: make the doc string
shorter, and tell users to read the manual for more details.



Reply | Threaded
Open this post in threaded view
|

bug#32990: 26.1.50; isearch-forward + t-m-m/mark-active doc

Charles A. Roelli
> Date: Mon, 08 Oct 2018 23:07:59 +0300
> From: Eli Zaretskii <[hidden email]>
> CC: [hidden email]
>
> Are you sure this is important to say there?  The doc string of that
> function is already way beyond annoyingly long.

It is important, but see below.

> I'd actually encourage the opposite trend: make the doc string
> shorter, and tell users to read the manual for more details.

Ok, that would be better than duplicating all the details in both the
function documentation and the manual.  I will prepare a patch.



Reply | Threaded
Open this post in threaded view
|

bug#32990: 26.1.50; isearch-forward + t-m-m/mark-active doc

Eli Zaretskii
> Date: Tue, 09 Oct 2018 20:14:34 +0200
> From: [hidden email] (Charles A. Roelli)
> CC: [hidden email]
>
> > I'd actually encourage the opposite trend: make the doc string
> > shorter, and tell users to read the manual for more details.
>
> Ok, that would be better than duplicating all the details in both the
> function documentation and the manual.  I will prepare a patch.

Thanks!



Reply | Threaded
Open this post in threaded view
|

bug#32990: 26.1.50; isearch-forward + t-m-m/mark-active doc

Charles A. Roelli
> Date: Tue, 09 Oct 2018 21:21:02 +0300
> From: Eli Zaretskii <[hidden email]>
>
> > Ok, that would be better than duplicating all the details in both the
> > function documentation and the manual.  I will prepare a patch.
>
> Thanks!

This line of isearch-forward's current docstring points to several
issues which make me uncomfortable changing or deleting it yet:

  Type DEL to cancel last input item from end of search string.

(DEL is the binding for "isearch-delete-char").

"Last input" is defined by the documentation of "isearch-delete-char"
as "the last character or the last isearch command that added or
deleted characters from the search string, moved point, toggled regexp
mode or case-sensitivity, etc.".

This definition of "last input" is not in the manual, and DEL is never
advertised to be capable of canceling regexp mode or case-sensitivity
toggling.  DEL is only advertised as being capable of "canceling the
last character of the search string" (in node "Basic Isearch") and
"canceling some C-s characters" (in node "Repeat Isearch").

For example, from emacs -Q, M-< C-s buffer M-r DEL deletes the "r" off
the search string, instead of "canceling the last input item", which I
understand should be the "M-r" regexp mode toggling.

To make matters more complicated, DEL /sometimes/ does cancel the
"last input" of a search: for example, if you have some text on your
kill ring, C-s C-y DEL undoes the yanking of the string into the
search.  (This is surprisingly sophisticated to me, and, I think,
something to document better.)

All of this makes it sound as if "isearch-delete-char" is badly named:
it is a wrapper around "isearch-pop-state", and seems to be a sort of
general (but incomplete) "undo" command for ISEARCH.  Maybe we should
name it differently and even provide a more intuitive binding for it
(on the master branch).  Its name also clashes with
"isearch-del-char".

Thus, some first steps for the emacs-26 branch could be to:

  . correct the definition of "last input item" as mentioned in the
    documentation of "isearch-delete-char"

  . name, index and fully explain "isearch-delete-char" in the Emacs
    manual

  . mention in more places how and when to use "isearch-delete-char"

If anybody can explain more about "isearch-delete-char", please do!



Reply | Threaded
Open this post in threaded view
|

bug#32990: 26.1.50; isearch-forward + t-m-m/mark-active doc

Eli Zaretskii
> Date: Sun, 14 Oct 2018 21:53:31 +0200
> From: [hidden email] (Charles A. Roelli)
> CC: [hidden email]
>
> Thus, some first steps for the emacs-26 branch could be to:
>
>   . correct the definition of "last input item" as mentioned in the
>     documentation of "isearch-delete-char"
>
>   . name, index and fully explain "isearch-delete-char" in the Emacs
>     manual
>
>   . mention in more places how and when to use "isearch-delete-char"

Sounds like a good plan to me, thanks.

I wouldn't rename isearch-delete-char; instead, I'd explain that what
it does is a kind of "undo", as you describe.



Reply | Threaded
Open this post in threaded view
|

bug#32990: 26.1.50; isearch-forward + t-m-m/mark-active doc

Charles A. Roelli
> Date: Mon, 15 Oct 2018 18:02:16 +0300
> From: Eli Zaretskii <[hidden email]>
>
> Sounds like a good plan to me, thanks.
>
> I wouldn't rename isearch-delete-char; instead, I'd explain that what
> it does is a kind of "undo", as you describe.

Ok, how about the following?



diff --git a/doc/emacs/search.texi b/doc/emacs/search.texi
index 053603e..58a7658 100644
--- a/doc/emacs/search.texi
+++ b/doc/emacs/search.texi
@@ -99,10 +99,18 @@ Basic Isearch
 that customize this highlighting.  The current search string is also
 displayed in the echo area.
 
-  If you make a mistake typing the search string, type @key{DEL}.
-Each @key{DEL} cancels the last character of the search string.
-@xref{Error in Isearch}, for more about dealing with unsuccessful
-search.
+@cindex isearch input item
+@cindex input item, isearch
+@findex isearch-delete-char
+@kindex DEL @r{(Incremental search)}
+  If you make a mistake typing the search string, type @key{DEL}
+(@code{isearch-delete-char}).  Each @key{DEL} cancels the last input
+item entered during the search.  Emacs records a new @dfn{input item}
+whenever you type a command that changes the search string, the
+position of point, the success or failure of the search, the direction
+of the search, the position of the other end of the current search
+result, or the ``wrappedness'' of the search.  @xref{Error in
+Isearch}, for more about dealing with unsuccessful search.
 
 @cindex exit incremental search
 @cindex incremental search, exiting
@@ -283,14 +291,15 @@ Error in Isearch
 @code{isearch-fail}.
 
   At this point, there are several things you can do.  If your string
-was mistyped, you can use @key{DEL} to erase some of it and correct
-it, or you can type @kbd{M-e} and edit it.  If you like the place you
-have found, you can type @key{RET} to remain there.  Or you can type
-@kbd{C-g}, which removes from the search string the characters that
-could not be found (the @samp{T} in @samp{FOOT}), leaving those that
-were found (the @samp{FOO} in @samp{FOOT}).  A second @kbd{C-g} at
-that point cancels the search entirely, returning point to where it
-was when the search started.
+was mistyped, use @key{DEL} to cancel a previous input item
+(@pxref{Basic Isearch}), @kbd{C-M-w} to erase one character at a time,
+or @kbd{M-e} to edit it.  If you like the place you have found, you
+can type @key{RET} to remain there.  Or you can type @kbd{C-g}, which
+removes from the search string the characters that could not be found
+(the @samp{T} in @samp{FOOT}), leaving those that were found (the
+@samp{FOO} in @samp{FOOT}).  A second @kbd{C-g} at that point cancels
+the search entirely, returning point to where it was when the search
+started.
 
 @cindex quitting (in search)
 @kindex C-g @r{(Incremental search)}
diff --git a/lisp/isearch.el b/lisp/isearch.el
index 31571e1..40b0799 100644
--- a/lisp/isearch.el
+++ b/lisp/isearch.el
@@ -1956,11 +1956,14 @@ isearch-highlight-regexp
 
 
 (defun isearch-delete-char ()
-  "Discard last input item and move point back.
-Last input means the last character or the last isearch command
-that added or deleted characters from the search string,
-moved point, toggled regexp mode or case-sensitivity, etc.
-If no previous match was done, just beep."
+  "Undo last input item during a search.
+
+An input item is a command that pushes a new state of isearch (as
+recorded by the `isearch--state' structure) to `isearch-cmds'.
+Info node `Basic Isearch' explains when Emacs pushes a new
+isearch state.
+
+If no input items have been entered yet, just beep."
   (interactive)
   (if (null (cdr isearch-cmds))
       (ding)



Reply | Threaded
Open this post in threaded view
|

bug#32990: 26.1.50; isearch-forward + t-m-m/mark-active doc

Eli Zaretskii
> Date: Tue, 16 Oct 2018 20:57:13 +0200
> From: [hidden email] (Charles A. Roelli)
> CC: [hidden email]
>
> Ok, how about the following?

LGTM, with 2 comments:

>  (defun isearch-delete-char ()
> -  "Discard last input item and move point back.
> -Last input means the last character or the last isearch command
> -that added or deleted characters from the search string,
> -moved point, toggled regexp mode or case-sensitivity, etc.
> -If no previous match was done, just beep."
> +  "Undo last input item during a search.
> +
> +An input item is a command that pushes a new state of isearch (as

I think you meant to say that "An input item is the result of a
command that pushes a new state ...".  Right?

> +Info node `Basic Isearch' explains when Emacs pushes a new
> +isearch state.

It is better to use `(emacs)Basic Search' as a link to the manual;
that's what we do elsewhere.

Thanks.