Re: isearch.el patch for `M-e' to put point at mismatch position

[Juri Linkov <juri@jurta.org>]

> 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'?