* isearch.el patch for `M-e' to put point at mismatch position
@ 2011-05-16 21:36 Drew Adams
2011-05-16 22:14 ` Juri Linkov
` (3 more replies)
0 siblings, 4 replies; 12+ messages in thread
From: Drew Adams @ 2011-05-16 21:36 UTC (permalink / raw)
To: emacs-devel
[-- Attachment #1: Type: text/plain, Size: 442 bytes --]
The attached patch makes `M-e' (`isearch-edit-string') put point at the first
mismatch position (or the search-string end if no mismatch).
When you hit `M-e' you are ready to edit at the mismatch position. This makes
it easy to change or insert a char or two to make the string match. (`C-g', on
the other hand just removes all of the mismatch.)
(This feature complements the use of highlighting to show users where the
mismatch starts.)
[-- Attachment #2: isearch-2011-05-16.patch --]
[-- Type: application/octet-stream, Size: 1968 bytes --]
diff -cw isearch.el isearch-patched-2011-05-16.el
*** isearch.el Mon May 16 11:38:22 2011
--- isearch-patched-2011-05-16.el Mon May 16 14:21:04 2011
***************
*** 1060,1065 ****
--- 1060,1080 ----
(defvar minibuffer-history-symbol) ;; from external package gmhist.el
+ (defun isearch-fail-pos ()
+ "Position of first mismatch in search string, or its length if none."
+ (let ((cmds isearch-cmds)
+ succ-msg)
+ (if (and isearch-success (not isearch-error))
+ (length isearch-message)
+ (while (or (not (isearch-success-state (car cmds)))
+ (isearch-error-state (car cmds)))
+ (pop cmds))
+ (setq succ-msg (and cmds (isearch-message-state (car cmds))))
+ (if (and (stringp succ-msg) (< (length succ-msg) (length isearch-message))
+ (equal succ-msg (substring isearch-message 0 (length succ-msg))))
+ (length succ-msg)
+ 0))))
+
(defun isearch-edit-string ()
"Edit the search string in the minibuffer.
The following additional command keys are active while editing.
***************
*** 1139,1145 ****
(setq isearch-new-string
(read-from-minibuffer
(isearch-message-prefix nil nil isearch-nonincremental)
! isearch-string
minibuffer-local-isearch-map nil
(if isearch-regexp
(cons 'regexp-search-ring
--- 1154,1160 ----
(setq isearch-new-string
(read-from-minibuffer
(isearch-message-prefix nil nil isearch-nonincremental)
! (cons isearch-string (1+ (isearch-fail-pos)))
minibuffer-local-isearch-map nil
(if isearch-regexp
(cons 'regexp-search-ring
Diff finished. Mon May 16 14:23:06 2011
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: isearch.el patch for `M-e' to put point at mismatch position
2011-05-16 21:36 isearch.el patch for `M-e' to put point at mismatch position Drew Adams
@ 2011-05-16 22:14 ` Juri Linkov
2011-05-16 22:37 ` Drew Adams
2011-05-17 3:08 ` Stefan Monnier
` (2 subsequent siblings)
3 siblings, 1 reply; 12+ messages in thread
From: Juri Linkov @ 2011-05-16 22:14 UTC (permalink / raw)
To: Drew Adams; +Cc: emacs-devel
> The attached patch makes `M-e' (`isearch-edit-string') put point at the first
> mismatch position (or the search-string end if no mismatch).
Wouldn't this be too surprising for users? Maybe there should be some
visual indication to explain why the cursor is in the middle of the
search string? E.g. the same highlighting of the failed part with the
`isearch-fail' face. Or more conveniently - activating the region on
the failed part, so the user can easily delete it with one keystroke.
^ permalink raw reply [flat|nested] 12+ messages in thread* RE: isearch.el patch for `M-e' to put point at mismatch position
2011-05-16 22:14 ` Juri Linkov
@ 2011-05-16 22:37 ` Drew Adams
0 siblings, 0 replies; 12+ messages in thread
From: Drew Adams @ 2011-05-16 22:37 UTC (permalink / raw)
To: 'Juri Linkov'; +Cc: emacs-devel
> > The attached patch makes `M-e' (`isearch-edit-string') put
> > point at the first mismatch position (or the search-string
> > end if no mismatch).
>
> Wouldn't this be too surprising for users?
No, I don't think so, or I wouldn't be using it (in isearch+.el), and I wouldn't
be proposing it here. ;-)
A user who hits `M-e' while looking at a search string with mismatch
highlighting will not be surprised to find the editing cursor positioned where
the highlighting began.
(This simply uses `read-from-minibuffer' with a cons INITIAL-CONTENTS argument.
Emacs doesn't take special measures when it does this.)
> Maybe there should be some
> visual indication to explain why the cursor is in the middle of the
> search string? E.g. the same highlighting of the failed part with the
> `isearch-fail' face.
pro: For the reason you suppose.
con: It could give the impression that the highlighting is active (actively
updated), whereas it represents only a snapshot of the original state. This
could be remedied by removing the highlighting as soon as any editing is done
(any change is made). That would be OK.
But I really do not think it is needed. The user just saw the highlighting,
before hitting `M-e'.
> Or more conveniently - activating the region on
> the failed part, so the user can easily delete it with one keystroke.
No, please. C-k does the same thing, and it does not interfere, as activating
the region would. And `C-g' after/before editing does the same thing.
There is no want of a quick way to remove the mismatched portion, once the
cursor is placed at the first mismatch position. It is that positioning that
takes users time. And it requires them to remember where the mismatch was.
FWIW, for a long time I did not move point to the mismatch position
automatically; I just bound `M-e', in the edit keymap, to let users move it
there on demand. IOW, after `M-e' a second `M-e' moved point to the mismatch
position.
I used that approach for a long time, but I found this to be much better: (a) no
need to hit a key; (b) no need to remember which key does that; (c) I find I
want point positioned there 100% of the time. For me it's a no-brainer. But do
as you like.
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: isearch.el patch for `M-e' to put point at mismatch position
2011-05-16 21:36 isearch.el patch for `M-e' to put point at mismatch position Drew Adams
2011-05-16 22:14 ` Juri Linkov
@ 2011-05-17 3:08 ` Stefan Monnier
2011-05-27 20:54 ` Drew Adams
2011-05-28 1:27 ` Stefan Monnier
2011-09-10 11:54 ` Juri Linkov
3 siblings, 1 reply; 12+ messages in thread
From: Stefan Monnier @ 2011-05-17 3:08 UTC (permalink / raw)
To: Drew Adams; +Cc: emacs-devel
> The attached patch makes `M-e' (`isearch-edit-string') put point at the first
> mismatch position (or the search-string end if no mismatch).
That's a cute idea. I like it.
Stefan
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: isearch.el patch for `M-e' to put point at mismatch position
2011-05-16 21:36 isearch.el patch for `M-e' to put point at mismatch position Drew Adams
2011-05-16 22:14 ` Juri Linkov
2011-05-17 3:08 ` Stefan Monnier
@ 2011-05-28 1:27 ` Stefan Monnier
2011-09-10 11:54 ` Juri Linkov
3 siblings, 0 replies; 12+ messages in thread
From: Stefan Monnier @ 2011-05-28 1:27 UTC (permalink / raw)
To: Drew Adams; +Cc: emacs-devel
> The attached patch makes `M-e' (`isearch-edit-string') put point at the first
> mismatch position (or the search-string end if no mismatch).
Installed. In the future, please:
- include a changelog.
- stay within 80 columns.
Stefan
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: isearch.el patch for `M-e' to put point at mismatch position
2011-05-16 21:36 isearch.el patch for `M-e' to put point at mismatch position Drew Adams
` (2 preceding siblings ...)
2011-05-28 1:27 ` Stefan Monnier
@ 2011-09-10 11:54 ` Juri Linkov
2011-09-10 16:13 ` Drew Adams
3 siblings, 1 reply; 12+ messages in thread
From: Juri Linkov @ 2011-09-10 11:54 UTC (permalink / raw)
To: Drew Adams; +Cc: emacs-devel
> The attached patch makes `M-e' (`isearch-edit-string') put point at the first
> mismatch position (or the search-string end if no mismatch).
>
> When you hit `M-e' you are ready to edit at the mismatch position. This makes
> it easy to change or insert a char or two to make the string match. (`C-g', on
> the other hand just removes all of the mismatch.)
>
> (This feature complements the use of highlighting to show users where the
> mismatch starts.)
>
> *** isearch.el Mon May 16 11:38:22 2011
> --- isearch-patched-2011-05-16.el Mon May 16 14:21:04 2011
> ***************
> *** 1060,1065 ****
> --- 1060,1080 ----
>
> (defvar minibuffer-history-symbol) ;; from external package gmhist.el
>
> + (defun isearch-fail-pos ()
> + "Position of first mismatch in search string, or its length if none."
> + (let ((cmds isearch-cmds)
> + succ-msg)
> + (if (and isearch-success (not isearch-error))
> + (length isearch-message)
> + (while (or (not (isearch-success-state (car cmds)))
> + (isearch-error-state (car cmds)))
> + (pop cmds))
> + (setq succ-msg (and cmds (isearch-message-state (car cmds))))
> + (if (and (stringp succ-msg) (< (length succ-msg) (length isearch-message))
> + (equal succ-msg (substring isearch-message 0 (length succ-msg))))
> + (length succ-msg)
> + 0))))
> +
> (defun isearch-edit-string ()
> "Edit the search string in the minibuffer.
> The following additional command keys are active while editing.
> ***************
> *** 1139,1145 ****
> (setq isearch-new-string
> (read-from-minibuffer
> (isearch-message-prefix nil nil isearch-nonincremental)
> ! isearch-string
> minibuffer-local-isearch-map nil
> (if isearch-regexp
> (cons 'regexp-search-ring
> --- 1154,1160 ----
> (setq isearch-new-string
> (read-from-minibuffer
> (isearch-message-prefix nil nil isearch-nonincremental)
> ! (cons isearch-string (1+ (isearch-fail-pos)))
I discovered now that you can't mix `isearch-message' and `isearch-string'.
Test case to reproduce the bug:
1. emacs -Q
2. insert a few TABs with e.g. `C-q TAB C-q TAB C-q TAB'
3. `M-<' (`beginning-of-buffer')
4. `C-s C-q TAB M-e'
fails with the error message "End of buffer".
The reason is that `isearch-message' is longer than `isearch-string',
because `isearch-text-char-description' replaces a single TAB
with two characters "^I" in `isearch-message'.
`isearch-fail-pos' calculates the position based on `isearch-message', but
uses it for `isearch-string' in `(cons isearch-string (1+ (isearch-fail-pos)))'
in `isearch-edit-string'.
Replacing `isearch-message' with `isearch-string' in `isearch-fail-pos'
will fix this bug.
BTW, there is exactly the same algorithm used in two functions
`isearch-fail-pos' and `isearch-message'. Currently both functions
use the same variable `isearch-message', but after this fix the same algorithm
will use different variables `isearch-message' and `isearch-string'.
So sharing code between them will be more difficult.
Maybe it would be better to add a new argument to `isearch-fail-pos'
that will define whether it should return the failed position in
`isearch-message' or `isearch-string'?
^ permalink raw reply [flat|nested] 12+ messages in thread* RE: isearch.el patch for `M-e' to put point at mismatch position
2011-09-10 11:54 ` Juri Linkov
@ 2011-09-10 16:13 ` Drew Adams
2011-09-10 16:22 ` Drew Adams
0 siblings, 1 reply; 12+ messages in thread
From: Drew Adams @ 2011-09-10 16:13 UTC (permalink / raw)
To: 'Juri Linkov'; +Cc: emacs-devel
> > When you hit `M-e' you are ready to edit at the mismatch
> > position. This makes it easy to change or insert a char
> > or two to make the string match. (`C-g', on
> > the other hand just removes all of the mismatch.)
>
> I discovered now that you can't mix `isearch-message' and
> `isearch-string'.
> Test case to reproduce the bug:
> 1. emacs -Q
> 2. insert a few TABs with e.g. `C-q TAB C-q TAB C-q TAB'
> 3. `M-<' (`beginning-of-buffer')
> 4. `C-s C-q TAB M-e'
> fails with the error message "End of buffer".
Good catch.
> The reason is that `isearch-message' is longer than `isearch-string',
> because `isearch-text-char-description' replaces a single TAB
> with two characters "^I" in `isearch-message'.
>
> `isearch-fail-pos' calculates the position based on
> `isearch-message', but uses it for `isearch-string' in
> `(cons isearch-string (1+ (isearch-fail-pos)))' in
> `isearch-edit-string'.
>
> Replacing `isearch-message' with `isearch-string' in
> `isearch-fail-pos' will fix this bug.
Sounds good. Please go for it.
> BTW, there is exactly the same algorithm used in two functions
> `isearch-fail-pos' and `isearch-message'.
Well, not exactly, but close.
> Currently both functions use the same variable `isearch-message',
> but after this fix the same algorithm
> will use different variables `isearch-message' and `isearch-string'.
> So sharing code between them will be more difficult.
There's no real need to share code between them.
> Maybe it would be better to add a new argument to `isearch-fail-pos'
> that will define whether it should return the failed position in
> `isearch-message' or `isearch-string'?
That might be OK too, but I'd suggest just making your suggested change above:
replace `isearch-message' with `isearch-string' in `isearch-fail-pos'.
^ permalink raw reply [flat|nested] 12+ messages in thread
* RE: isearch.el patch for `M-e' to put point at mismatch position
2011-09-10 16:13 ` Drew Adams
@ 2011-09-10 16:22 ` Drew Adams
2011-09-12 11:28 ` Juri Linkov
0 siblings, 1 reply; 12+ messages in thread
From: Drew Adams @ 2011-09-10 16:22 UTC (permalink / raw)
To: 'Juri Linkov'; +Cc: emacs-devel
> > Replacing `isearch-message' with `isearch-string' in
> > `isearch-fail-pos' will fix this bug.
>
> Sounds good. Please go for it.
Actually, unless I'm missing something that doesn't work either.
emacs -Q
Insert a few tabs using C-q TAB. Insert `w'.
M-<.
C-s C-q TAB k
Highlighting shows that the TAB char is matched but the `k' char is a mismatch.
Now hit M-e. Instead of placing the cursor at the mismatch (just before the
`k'), it places it before the TAB, at the beginning of the search string.
(Without your fix, the cursor was placed after the `k', which is also
incorrect.)
Am I missing something?
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: isearch.el patch for `M-e' to put point at mismatch position
2011-09-10 16:22 ` Drew Adams
@ 2011-09-12 11:28 ` Juri Linkov
2011-09-12 14:58 ` Drew Adams
0 siblings, 1 reply; 12+ messages in thread
From: Juri Linkov @ 2011-09-12 11:28 UTC (permalink / raw)
To: Drew Adams; +Cc: emacs-devel
> Actually, unless I'm missing something that doesn't work either.
>
> emacs -Q
> Insert a few tabs using C-q TAB. Insert `w'.
> M-<.
> C-s C-q TAB k
>
> Highlighting shows that the TAB char is matched but the `k' char is a mismatch.
>
> Now hit M-e. Instead of placing the cursor at the mismatch (just before the
> `k'), it places it before the TAB, at the beginning of the search string.
>
> (Without your fix, the cursor was placed after the `k', which is also
> incorrect.)
>
> Am I missing something?
Perhaps you forgot to replace `isearch-message-state' with `isearch-string-state'?
^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2011-09-14 16:08 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-05-16 21:36 isearch.el patch for `M-e' to put point at mismatch position Drew Adams
2011-05-16 22:14 ` Juri Linkov
2011-05-16 22:37 ` Drew Adams
2011-05-17 3:08 ` Stefan Monnier
2011-05-27 20:54 ` Drew Adams
2011-05-28 1:27 ` Stefan Monnier
2011-09-10 11:54 ` Juri Linkov
2011-09-10 16:13 ` Drew Adams
2011-09-10 16:22 ` Drew Adams
2011-09-12 11:28 ` Juri Linkov
2011-09-12 14:58 ` Drew Adams
2011-09-14 16:08 ` Juri Linkov
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).