bug#14742: 24.3.50; enhancement request: be able to prepend stuff from buffer when search backward

bug#14742: 24.3.50; enhancement request: be able to prepend stuff from buffer when search backward

[Drew Adams]

Dunno how simple this would be to implement - just a thought/wish.

You can yank stuff from point onto the end of the search string.

When searching backward, it could be handy to be able to yank stuff
BEFORE point progressively onto the BEGINNING of the search string.
IOW, mirror the forward-search behavior.

I realize that in general you add to the end of the search string, even
when searching backward.  But that is presumably partly because there is
no easy way to prepend to it - to do that you need to use `M-e'.

But yanking from the buffer is not like typing text to add to the
search string.  In principle, yanked stuff could be prepended as well as
appended, IIUC.

In GNU Emacs 24.3.50.1 (i686-pc-mingw32)
 of 2013-06-27 on ODIEONE
Bzr revision: 113205 dgutov@yandex.ru-20130627095155-f1lv1c7xf99g1sss
Windowing system distributor `Microsoft Corp.', version 6.1.7601
Configured using:
 `configure --prefix=/c/Devel/emacs/binary --enable-checking=yes,glyphs
 CFLAGS=-O0 -g3 LDFLAGS=-Lc:/Devel/emacs/lib
 CPPFLAGS=-Ic:/Devel/emacs/include'

bug#14742: 24.3.50; enhancement request: be able to prepend stuff from buffer when search backward

[Juri Linkov]

> In principle, yanked stuff could be prepended
> as well as appended, IIUC.

As you may remember, I implemented this two weeks ago
in the patch I sent to emacs-devel, and since then
I have not had problems with it.  So what could be done
is to move this code from `isearch-other-meta-char'
to `isearch-yank-string' and `isearch-del-char' for more
general functionality by adding a new argument `BACK' to them
(or could you propose a better name for the argument).

bug#14742: 24.3.50; enhancement request: be able to prepend stuff from buffer when search backward

[Drew Adams]

> > In principle, yanked stuff could be prepended
> > as well as appended, IIUC.
> 
> As you may remember, I implemented this two weeks ago
> in the patch I sent to emacs-devel,

No, I didn't know that.  Great!

> and since then I have not had problems with it.  So what could
> be done is to move this code from `isearch-other-meta-char'
> to `isearch-yank-string' and `isearch-del-char' for more
> general functionality by adding a new argument `BACK' to them
> (or could you propose a better name for the argument).

I take your word for it wrt the implementation.  After I see it,
I might have a suggestion wrt the name (maybe BACKWARD, as that
seems to be the term used in our search names).

It all sounds good so far.

bug#14742: 24.3.50; enhancement request: be able to prepend stuff from buffer when search backward

[Juri Linkov]

>> and since then I have not had problems with it.  So what could
>> be done is to move this code from `isearch-other-meta-char'
>> to `isearch-yank-string' and `isearch-del-char' for more
>> general functionality by adding a new argument `BACK' to them
>> (or could you propose a better name for the argument).
>
> I take your word for it wrt the implementation.  After I see it,
> I might have a suggestion wrt the name (maybe BACKWARD, as that
> seems to be the term used in our search names).

It's easy to find a name for the new arg of `isearch-yank-string'.
By default, it appends a string to the end of the search string,
so a new arg for opposite meaning could have the name `prepend'
to prepend a string to the beginning of the search string.

But what could be a name for the new arg of `isearch-del-char'?
By default, it deletes a substring from the end of the search string.
And with a non-nil new arg it will delete a substring from the
beginning of the search string.  What about the name `behead'
suggested in
http://english.stackexchange.com/questions/111382/what-are-the-antonyms-of-append-and-prepend

=== modified file 'lisp/isearch.el'
--- lisp/isearch.el	2013-06-13 22:08:45 +0000
+++ lisp/isearch.el	2013-06-29 21:50:03 +0000
@@ -1839,7 +1839,7 @@ (defun isearch-delete-char ()
     (isearch-pop-state))
   (isearch-update))
 
-(defun isearch-del-char (&optional arg)
+(defun isearch-del-char (&optional arg behead)
   "Delete character from end of search string and search again.
 Unlike `isearch-delete-char', it only deletes the last character,
 but doesn't cancel the effect of other isearch command.
@@ -1847,9 +1847,13 @@ (defun isearch-del-char (&optional arg)
   (interactive "p")
   (if (= 0 (length isearch-string))
       (ding)
-    (setq isearch-string (substring isearch-string 0
-				    (- (min (or arg 1)
-					    (length isearch-string))))
+    (setq isearch-string (if behead
+			     (substring isearch-string
+					(min (or arg 1)
+					     (length isearch-string)))
+			   (substring isearch-string 0
+				      (- (min (or arg 1)
+					      (length isearch-string)))))
           isearch-message (mapconcat 'isearch-text-char-description
                                      isearch-string "")))
   ;; Use the isearch-other-end as new starting point to be able
@@ -1858,17 +1862,23 @@ (defun isearch-del-char (&optional arg)
    (isearch-push-state)
    (isearch-update)))
 
-(defun isearch-yank-string (string)
+(defun isearch-yank-string (string &optional prepend)
   "Pull STRING into search string."
   ;; Downcase the string if not supposed to case-fold yanked strings.
   (if (and isearch-case-fold-search
 	   (eq 'not-yanks search-upper-case))
       (setq string (downcase string)))
   (if isearch-regexp (setq string (regexp-quote string)))
-  (setq isearch-string (concat isearch-string string)
+  (setq isearch-string (if prepend
+			   (concat string isearch-string)
+			 (concat isearch-string string))
 	isearch-message
 	(concat isearch-message
 		(mapconcat 'isearch-text-char-description

bug#14742: 24.3.50; enhancement request: be able to prepend stuff from buffer when search backward

[Drew Adams]

> It's easy to find a name for the new arg of `isearch-yank-string'.
> By default, it appends a string to the end of the search string,
> so a new arg for opposite meaning could have the name `prepend'
> to prepend a string to the beginning of the search string.
> 
> But what could be a name for the new arg of `isearch-del-char'?
> By default, it deletes a substring from the end of the search string.
> And with a non-nil new arg it will delete a substring from the
> beginning of the search string.  What about the name `behead'

1. I would stay away from something like "behead" - that's
stretching things.  We don't commonly think of the "head" of a
search string.

I'd suggest not looking for a verb at all - the verb is already
in the command name.

2. Just use a name for the position, not the action: e.g. START or
BEGINNING.  (I would stay away from FRONT & BACK too, as references
to the search-string ends.  BACK is particularly problematic.)

PREPEND is not too bad if you do prefer a verb, but if you use
a position name then you can use the same name for multiple actions.
For this reason, for instance, BEGINNING is better than
FROM-BEGINNING & TO-BEGINNING.

3. To me, yanking stuff to the beginning is more important than
deleting stuff from the beginning.  I was thinking either have a
separate key (or set of keys) that is (are) analogous to the
yank-at-end key(s) or else just reuse the same keys when searching
backward.

I would probably go with the latter (same keys).  How useful is
it to incrementally yank more stuff in a forward-buffer direction
when searching backward?

I guess the same could be done for deletion as for yanking: reuse
the same keys.

bug#14742: 24.3.50; enhancement request: be able to prepend stuff from buffer when search backward

[Juri Linkov]

> 2. Just use a name for the position, not the action: e.g. START or
> BEGINNING.  (I would stay away from FRONT & BACK too, as references
> to the search-string ends.  BACK is particularly problematic.)

Then let's use BEGINNING:

=== modified file 'lisp/isearch.el'
--- lisp/isearch.el	2013-06-13 22:08:45 +0000
+++ lisp/isearch.el	2013-07-02 22:35:17 +0000
@@ -1839,7 +1839,7 @@ (defun isearch-delete-char ()
     (isearch-pop-state))
   (isearch-update))
 
-(defun isearch-del-char (&optional arg)
+(defun isearch-del-char (&optional arg beginning)
   "Delete character from end of search string and search again.
 Unlike `isearch-delete-char', it only deletes the last character,
 but doesn't cancel the effect of other isearch command.
@@ -1847,9 +1847,13 @@ (defun isearch-del-char (&optional arg)
   (interactive "p")
   (if (= 0 (length isearch-string))
       (ding)
-    (setq isearch-string (substring isearch-string 0
-				    (- (min (or arg 1)
-					    (length isearch-string))))
+    (setq isearch-string (if beginning
+			     (substring isearch-string
+					(min (or arg 1)
+					     (length isearch-string)))
+			   (substring isearch-string 0
+				      (- (min (or arg 1)
+					      (length isearch-string)))))
           isearch-message (mapconcat 'isearch-text-char-description
                                      isearch-string "")))
   ;; Use the isearch-other-end as new starting point to be able
@@ -1858,17 +1862,23 @@ (defun isearch-del-char (&optional arg)
    (isearch-push-state)
    (isearch-update)))
 
-(defun isearch-yank-string (string)
+(defun isearch-yank-string (string &optional beginning)
   "Pull STRING into search string."
   ;; Downcase the string if not supposed to case-fold yanked strings.
   (if (and isearch-case-fold-search
 	   (eq 'not-yanks search-upper-case))
       (setq string (downcase string)))
   (if isearch-regexp (setq string (regexp-quote string)))
-  (setq isearch-string (concat isearch-string string)
+  (setq isearch-string (if beginning
+			   (concat string isearch-string)
+			 (concat isearch-string string))
 	isearch-message
 	(concat isearch-message
 		(mapconcat 'isearch-text-char-description


> 3. To me, yanking stuff to the beginning is more important than
> deleting stuff from the beginning.  I was thinking either have a
> separate key (or set of keys) that is (are) analogous to the
> yank-at-end key(s) or else just reuse the same keys when searching
> backward.

I have no idea about new keys, but I need this change to implement
yanking/deletion by motion keys (disabled by default).

bug#14742: 24.3.50; enhancement request: be able to prepend stuff from buffer when search backward

[Drew Adams]

> > 2. Just use a name for the position, not the action:
> >    e.g. START or BEGINNING. 
> Then let's use BEGINNING:

Good.  But I don't see where you documented parameter BEGINNING.

> > 3. To me, yanking stuff to the beginning is more important than
> > deleting stuff from the beginning.  I was thinking either have a
> > separate key (or set of keys) that is (are) analogous to the
> > yank-at-end key(s) or else just reuse the same keys when searching
> > backward.
> 
> I have no idea about new keys, but I need this change to implement
> yanking/deletion by motion keys (disabled by default).

Dunno what that means.  Motion keys will yank or delete?
Doesn't sound too good, a priori.
Guess I missed the functional spec for that. ;-)

Why not just reuse the keys we already use to yank, but have them yank
at the beginning when you search in reverse?  E.g.:

C-w     - `isearch-yank-word-or-char', but at the beginning.
C-y     - `isearch-yank-kill', but at the beginning.
C-M-y   - `isearch-yank-char', but at the beginning.
M-y     - `isearch-yank-pop', but at the beginning.
M-x C-e - `isearch-yank-line', but at the beginning.

Doesn't that seem natural?

In Isearch+ I have even more than that for yanking.  I would much
rather have a user be able to reuse the same keys in the opposite
direction.

bug#14742: 24.3.50; enhancement request: be able to prepend stuff from buffer when search backward

[Juri Linkov]

>> > 3. To me, yanking stuff to the beginning is more important than
>> > deleting stuff from the beginning.  I was thinking either have a
>> > separate key (or set of keys) that is (are) analogous to the
>> > yank-at-end key(s) or else just reuse the same keys when searching
>> > backward.
>>
>> I have no idea about new keys, but I need this change to implement
>> yanking/deletion by motion keys (disabled by default).
>
> Dunno what that means.  Motion keys will yank or delete?
> Doesn't sound too good, a priori.
> Guess I missed the functional spec for that. ;-)

No one tried this feature, but surprisingly it's very convenient.
So I could implement only low-level support for this feature in isearch.el,
then more high-level commands could be implemented somewhere else.

> Why not just reuse the keys we already use to yank, but have them yank
> at the beginning when you search in reverse?  E.g.:
>
> C-w     - `isearch-yank-word-or-char', but at the beginning.
> C-y     - `isearch-yank-kill', but at the beginning.
> C-M-y   - `isearch-yank-char', but at the beginning.
> M-y     - `isearch-yank-pop', but at the beginning.
> M-x C-e - `isearch-yank-line', but at the beginning.
>
> Doesn't that seem natural?

No, unfortunately there keys are not natural.

> In Isearch+ I have even more than that for yanking.  I would much
> rather have a user be able to reuse the same keys in the opposite
> direction.

Do you mean a new boolean user option to define whether `C-M-y'
should yank but at the beginning/end in a reverse search?

bug#14742: 24.3.50; enhancement request: be able to prepend stuff from buffer when search backward

[Drew Adams]

> >> > 3. To me, yanking stuff to the beginning is more important than
> >> > deleting stuff from the beginning.  I was thinking either have a
> >> > separate key (or set of keys) that is (are) analogous to the
> >> > yank-at-end key(s) or else just reuse the same keys when searching
> >> > backward.
> >>
> >> I have no idea about new keys, but I need this change to implement
> >> yanking/deletion by motion keys (disabled by default).
> >
> > Dunno what that means.  Motion keys will yank or delete?
> > Doesn't sound too good, a priori.
> > Guess I missed the functional spec for that. ;-)
> 
> No one tried this feature, but surprisingly it's very convenient.
> So I could implement only low-level support for this feature in isearch.el,
> then more high-level commands could be implemented somewhere else.

Is it described somewhere, in user language?  If not, can you please
describe it?

> > Why not just reuse the keys we already use to yank, but have them yank
> > at the beginning when you search in reverse?  E.g.:
> >
> > C-w     - `isearch-yank-word-or-char', but at the beginning.
> > C-y     - `isearch-yank-kill', but at the beginning.
> > C-M-y   - `isearch-yank-char', but at the beginning.
> > M-y     - `isearch-yank-pop', but at the beginning.
> > M-x C-e - `isearch-yank-line', but at the beginning.
> >
> > Doesn't that seem natural?
> 
> No, unfortunately there keys are not natural.

Are you saying they are unnatural when going backward but natural when
going forward?

> > In Isearch+ I have even more than that for yanking.  I would much
> > rather have a user be able to reuse the same keys in the opposite
> > direction.
> 
> Do you mean a new boolean user option to define whether `C-M-y'
> should yank but at the beginning/end in a reverse search?

An option would be fine.  But why not just do it (by default)?
I don't understand why you think C-y yanking to the end when searching
forward is more natural than it yanking to the beginning when searching
backward.

bug#14742: 24.3.50; enhancement request: be able to prepend stuff from buffer when search backward

[Drew Adams]

Oops.  It just occurred to me what you might be up to, which I did not
understand.  I haven't seen your description of what you have in mind.
But if it is what I imagine now, then I think it sounds truly elegant.

Is this the idea: cursor movements would extend isearch by yanking the
text the cursor would move over in the buffer, starting from `point'
(where isearch starts)?

E.g., for forward search C-f, M-f, C-M-f, C-e, etc. would yank the next
char, word, symbol, line etc. to the end of the search string.  And for
reverse search C-b, M-b, C-M-b, C-a, etc. would yank the previous (next
going backward) char, word, symbol, line etc. to the beginning of the
search string.

And presumably backward cursor movements when searching forward (e.g.,
C-b, M-b, C-M-b, C-a) would undo, in the sense of removing yanked text
(but only yanked text, presumably) from the end.  And forward cursor
movements when searching backward would remove yanked text from the
beginning.

Is that the idea?  If so, bravo; I'm a supporter, a priori.  And yes,
IMO that would be quite natural.

I imagine it should be optional, though (even if it could perhaps
be/become the default), since some people might object to things like
C-M-f not ending isearch.  My guess is that most people would want this
behavior immediately - cursor movement does not currently do anything
except exit isearch (and there are plenty of other ways to do that).

TBD would be what to do with the equivalent of the combination yanks that
exist today, e.g., C-w for word-or-char.  Did you have another idea in
that area also, or did you intend to just leave them as is?  Perhaps you
were thinking of having things like C-f/right/C-b/left doing what C-w does?
If so, I'm guessing it would be better to just have them yank a char at a
time (easy enough to hold them pressed, or change to M-f/M-b etc.), in
order to keep the parallel with cursor movements.

Do you have this working already?  Have you described what you have in
mind somewhere?  Did you bring this up in emacs-devel?  If not, please
consider it - that is the best place to toss such an idea around.

Another possibility (an extension) just occurred to me: There is only one
starting position: `point' where isearch is started.  But it might be
useful to be able to toggle off the yanking, perform some more cursor
movements without yanking to, in effect, move the yanking position in the
buffer, then toggle yanking back on and resume yanking from the new
position.

Even if we made no provision (at least at first) for showing the yank
position in the buffer, I think it would be clear enough, just from the
result of yanking a char, say, (and then unyanking it if it was yanked
just to check where the yank position is).  And being able to move the
yank position using cursor movements would also be helpful when
combined with other yanking operations (e.g., combo's like C-w).

Dunno whether I am clear in these last two paragraphs, or whether you
have already considered something like this.  Anyway, bravo - great
idea to play with, and likely a real improvement to isearch.

bug#14742: 24.3.50; enhancement request: be able to prepend stuff from buffer when search backward

[Juri Linkov]

> Is this the idea: cursor movements would extend isearch by yanking the
> text the cursor would move over in the buffer, starting from `point'
> (where isearch starts)?
>
> E.g., for forward search C-f, M-f, C-M-f, C-e, etc. would yank the next
> char, word, symbol, line etc. to the end of the search string.  And for
> reverse search C-b, M-b, C-M-b, C-a, etc. would yank the previous (next
> going backward) char, word, symbol, line etc. to the beginning of the
> search string.
>
> And presumably backward cursor movements when searching forward (e.g.,
> C-b, M-b, C-M-b, C-a) would undo, in the sense of removing yanked text
> (but only yanked text, presumably) from the end.  And forward cursor
> movements when searching backward would remove yanked text from the
> beginning.
>
> Is that the idea?

Exactly.

> Do you have this working already?  Have you described what you have in
> mind somewhere?  Did you bring this up in emacs-devel?  If not, please
> consider it - that is the best place to toss such an idea around.

Yes, it is working and I described it on emacs-devel a month ago.
Now I'll prepare a minimal patch that implements low-level support, then
send separate high-level code to activate it, so everyone could try it.

bug#14742: 24.3.50; enhancement request: be able to prepend stuff from buffer when search backward

[Drew Adams]

> > Do you have this working already?  Have you described what you have in
> > mind somewhere?  Did you bring this up in emacs-devel?  If not, please
> > consider it - that is the best place to toss such an idea around.
> 
> Yes, it is working and I described it on emacs-devel a month ago.
> Now I'll prepare a minimal patch that implements low-level support, then
> send separate high-level code to activate it, so everyone could try it.

Sounds good; looking forward to it.

What is the emacs-devel thread?  Somehow I must have missed it.

bug#14742: 24.3.50; enhancement request: be able to prepend stuff from buffer when search backward

[Juri Linkov]

>> > Do you have this working already?  Have you described what you have in
>> > mind somewhere?  Did you bring this up in emacs-devel?  If not, please
>> > consider it - that is the best place to toss such an idea around.
>>
>> Yes, it is working and I described it on emacs-devel a month ago.
>> Now I'll prepare a minimal patch that implements low-level support, then
>> send separate high-level code to activate it, so everyone could try it.
>
> Sounds good; looking forward to it.

The following minimal patch implements low-level support
for the motion yanking feature that could be later enabled
by separate higher-level commands.

It adds a new option `isearch-allow-move' with the nil default value.
Like the user option `transient-mark-mode' having the certain special
value of (only . OLDVAL) that enables Transient Mark mode temporarily,
until any subsequent point motion command that is not shift-translated
sets the value of `transient-mark-mode' back to OLDVAL, the new option
`isearch-allow-move' does the same.

This patch relies on the previously sent patch that adds a new argument
`BEGINNING' to `isearch-yank-string' and `isearch-del-char'.

=== modified file 'lisp/isearch.el'
--- lisp/isearch.el	2013-06-13 22:08:45 +0000
+++ lisp/isearch.el	2013-07-04 23:28:33 +0000
@@ -987,6 +1046,8 @@ (defun isearch-update ()
   ;; We must prevent the point moving to the end of composition when a
   ;; part of the composition has just been searched.
   (setq disable-point-adjustment t)
+  (when (eq (car-safe isearch-allow-move) 'only)
+    (setq isearch-allow-move (cdr isearch-allow-move)))
   (run-hooks 'isearch-update-post-hook))
 
 (defun isearch-done (&optional nopush edit)
@@ -2267,6 +2433,77 @@ (defun isearch-lookup-scroll-key (key-se
 	     (eq (get binding 'scroll-command) t))
          binding)))
 
+(defcustom isearch-allow-move nil
+  "Whether cursor movement is allowed to yank text in Isearch mode.
+If nil, point motion commands will exit Isearch mode immediately.
+If non-nil, point motion commands extend Isearch by yanking the text
+the cursor moves over in the buffer.  E.g., C-f yanks the next char,
+C-M-f yanks the next expression, etc. to the end of the search string
+in forward search.  In reverse search, C-b yanks the previous char,
+C-M-b yanks the previous expression, etc. to the beginning of the
+search string.  And conversely, backward cursor movements, e.g., C-b
+removes text from the end of the search string when searching forward.
+When searching backward, forward cursor movements, e.g., C-f removes
+text from the beginning of the search string.
+
+Lisp programs may give this variable a special value of
+\(only . OLDVAL) to enable motion yanking temporarily.
+After any subsequent Isearch command that is not point motion,
+the value of `isearch-allow-move' is set to OLDVAL."
+  :type 'boolean
+  :version "24.4"
+  :group 'isearch)
+
+(defun isearch-lookup-move-key (key-seq)
+  "If KEY-SEQ is bound to a motion command, return it as a symbol.
+Otherwise return nil."
+  (let* ((overriding-terminal-local-map nil)
+         (binding (key-binding key-seq)))
+    (and binding (symbolp binding) (commandp binding)
+         (eq (get binding 'isearch-move) t)
+         binding)))
+
 (defalias 'isearch-other-control-char 'isearch-other-meta-char)
 
 (defun isearch-other-meta-char (&optional arg)
@@ -2379,7 +2616,67 @@ (defun isearch-other-meta-char (&optiona
              (if ab-bel
                  (isearch-back-into-window (eq ab-bel 'above) isearch-point)
                (goto-char isearch-point)))
           (isearch-update))
+	  ;; Handle a motion function.
+	  ((and isearch-allow-move
+		(progn (setq key (isearch-reread-key-sequence-naturally keylist))
+		       (setq keylist (listify-key-sequence key))
+		       (setq main-event (aref key 0))
+		       (setq move-command (or
+					   (isearch-lookup-move-key key)
+					   (isearch-lookup-move-key
+					    ;; Use the last key in the sequence.
+					    (vector (aref key (1- (length key)))))))))
+	   (setq prefix-arg arg)
+	   (let* ((old-point (point))
+		  (new-point (save-excursion
+			       (condition-case ()
+				   (command-execute move-command)
+				 (error nil))
+			       (point))))
+	     ;; Change search direction between forward and backward.
+	     (when (if isearch-forward
+			 (< new-point old-point)
+		       (> new-point old-point))
+	       (setq isearch-forward (not isearch-forward))
+	       (setq isearch-string "" isearch-message "")
+	       (if isearch-other-end (setq old-point isearch-other-end)))
+	     (if (< old-point new-point)
+		 (if isearch-forward
+		     (isearch-yank-string
+		      (buffer-substring-no-properties old-point new-point))
+		   (isearch-del-char (- new-point old-point) t))
+	       (if isearch-forward
+		   (isearch-del-char (- old-point new-point))
+		 (isearch-yank-string
+		  (buffer-substring-no-properties old-point new-point) t))))
+	   (setq isearch-allow-move (cons 'only isearch-allow-move)))
 	  ;; A mouse click on the isearch message starts editing the search string
 	  ((and (eq (car-safe main-event) 'down-mouse-1)
 		(window-minibuffer-p (posn-window (event-start main-event))))



> What is the emacs-devel thread?  Somehow I must have missed it.

The emacs-devel thread is here:
http://lists.gnu.org/archive/html/emacs-devel/2013-06/msg00211.html

bug#14742: 24.3.50; enhancement request: be able to prepend stuff from buffer when search backward

[Stefan Monnier]

> @@ -2379,7 +2616,67 @@ (defun isearch-other-meta-char (&optiona
>               (if ab-bel
>                   (isearch-back-into-window (eq ab-bel 'above) isearch-point)
>                 (goto-char isearch-point)))
>            (isearch-update))
> +	  ;; Handle a motion function.
> +	  ((and isearch-allow-move
> +		(progn (setq key (isearch-reread-key-sequence-naturally keylist))
> +		       (setq keylist (listify-key-sequence key))
> +		       (setq main-event (aref key 0))
> +		       (setq move-command (or
> +					   (isearch-lookup-move-key key)
> +					   (isearch-lookup-move-key
> +					    ;; Use the last key in the sequence.
> +					    (vector (aref key (1- (length key)))))))))
> +	   (setq prefix-arg arg)
> +	   (let* ((old-point (point))
> +		  (new-point (save-excursion
> +			       (condition-case ()
> +				   (command-execute move-command)
> +				 (error nil))
> +			       (point))))
> +	     ;; Change search direction between forward and backward.
> +	     (when (if isearch-forward
> +			 (< new-point old-point)
> +		       (> new-point old-point))
> +	       (setq isearch-forward (not isearch-forward))
> +	       (setq isearch-string "" isearch-message "")
> +	       (if isearch-other-end (setq old-point isearch-other-end)))
> +	     (if (< old-point new-point)
> +		 (if isearch-forward
> +		     (isearch-yank-string
> +		      (buffer-substring-no-properties old-point new-point))
> +		   (isearch-del-char (- new-point old-point) t))
> +	       (if isearch-forward
> +		   (isearch-del-char (- old-point new-point))
> +		 (isearch-yank-string
> +		  (buffer-substring-no-properties old-point new-point) t))))
> +	   (setq isearch-allow-move (cons 'only isearch-allow-move)))
>  	  ;; A mouse click on the isearch message starts editing the search string
>  	  ((and (eq (car-safe main-event) 'down-mouse-1)
>  		(window-minibuffer-p (posn-window (event-start main-event))))

Could you move this new code to a separate function?


        Stefan

bug#14742: 24.3.50; enhancement request: be able to prepend stuff from buffer when search backward

[Juri Linkov]

> Could you move this new code to a separate function?

In the following patch I moved new code to a separate function
`isearch-yank-or-del' that works like a combination of the existing
functions `isearch-yank-string' and `isearch-del-char'.
`isearch-yank-or-del' is general enough that it can be used
separately from the movement-yanking feature.

The remaining code in `isearch-other-meta-char' is also general enough
that it can be used to implement other features as well.  For example,
I tried to replace the call `(isearch-yank-or-del (point) new-point)'
with `(goto-char new-point)' and got another useful fully working feature
mentioned by Drew in this thread where cursor movement doesn't yank text.

This raises the question how users would be able to select
between these alternatives.  Maybe `isearch-allow-move'
should provide two options:

- `jump' that doesn't yank text during cursor movement;

- `yank' to yank text during cursor movement.

These options could be implemented in the next patch,
whereas this patch just moves yanking/deleting to a new function
`isearch-yank-or-del'.

=== modified file 'lisp/isearch.el'
--- lisp/isearch.el	2013-06-13 22:08:45 +0000
+++ lisp/isearch.el	2013-07-05 22:27:08 +0000
@@ -987,6 +1046,8 @@ (defun isearch-update ()
   ;; We must prevent the point moving to the end of composition when a
   ;; part of the composition has just been searched.
   (setq disable-point-adjustment t)
+  (when (eq (car-safe isearch-allow-move) 'only)
+    (setq isearch-allow-move (cdr isearch-allow-move)))
   (run-hooks 'isearch-update-post-hook))
 
 (defun isearch-done (&optional nopush edit)
@@ -1969,6 +2142,56 @@ (defun isearch-yank-line (&optional arg)
    (lambda () (let ((inhibit-field-text-motion t))
 		(line-end-position (if (eolp) (1+ arg) arg))))))
 
+(defun isearch-yank-or-del (old-point new-point)
+  "Add or delete text in search string, depending on two given positions.
+Given an old position of point in OLD-POINT and a new position NEW-POINT,
+either pull text from the buffer between these positions into the search
+string, or delete text from the search string to the length of the text
+between these positions.
+In forward search, when NEW-POINT is greater than OLD-POINT,
+yank text from the buffer between NEW-POINT and OLD-POINT;
+when NEW-POINT is less than OLD-POINT, delete text from
+the search string between NEW-POINT and OLD-POINT.
+In reverse search, when NEW-POINT is greater than OLD-POINT,
+delete text from the search string between NEW-POINT and OLD-POINT;
+when NEW-POINT is less than OLD-POINT, yank text from the buffer
+between NEW-POINT and OLD-POINT.
+When NEW-POINT is less than the beginning of the currently matched string
+in forward search, then change search direction to reverse search.
+When NEW-POINT is greater than the end of the currently matched string
+in reverse search, then change search direction to forward search."
+  ;; Change search direction between forward and backward.
+  (when (if isearch-other-end
+	    (and (not isearch-error)	; for regexp incomplete input
+		 (if isearch-forward
+		     (< new-point isearch-other-end)
+		   (> new-point isearch-other-end)))
+	  (if isearch-forward
+	      (< new-point old-point)
+	    (> new-point old-point)))
+    (setq isearch-forward (not isearch-forward))
+    (setq isearch-string "" isearch-message "")
+    (if isearch-other-end (setq old-point isearch-other-end)))
+  (if (< old-point new-point)
+      (if isearch-forward
+	  ;; Add text to the search string.
+	  (isearch-yank-string
+	   (buffer-substring-no-properties old-point new-point))
+	;; In reverse search, lax whitespace doesn't work,
+	;; so go to the new point explicitly.
+	(goto-char new-point)
+	;; In reverse search, delete text from beginning of search string.
+	(isearch-del-char (- new-point old-point) t))
+    (if isearch-forward
+	;; Delete text from the search string.
+	(isearch-del-char (- old-point new-point))
+      ;; In reverse search, lax whitespace doesn't work,
+      ;; so go to the new point explicitly.
+      (goto-char new-point)
+      ;; In reverse search, add text to beginning of search string.
+      (isearch-yank-string
+       (buffer-substring-no-properties old-point new-point) t))))
+
 (defun isearch-char-by-name (&optional count)
   "Read a character by its Unicode name and add it to the search string.
 Completion is available like in `read-char-by-name' used by `insert-char'.
@@ -2267,6 +2483,77 @@ (defun isearch-lookup-scroll-key (key-se
 	     (eq (get binding 'scroll-command) t))
          binding)))
 
+(defcustom isearch-allow-move nil
+  "Whether cursor movement is allowed to yank text in Isearch mode.
+If nil, point motion commands will exit Isearch mode immediately.
+If non-nil, point motion commands extend Isearch by yanking the text
+the cursor moves over in the buffer.  E.g., `C-f' yanks the next char,
+`C-M-f' yanks the next expression, etc. to the end of the search string
+in forward search.  In reverse search, `C-b' yanks the previous char,
+`C-M-b' yanks the previous expression, etc. to the beginning of the
+search string.  And conversely, backward cursor movements, e.g., `C-b'
+deletes text from the end of the search string when searching forward.
+When searching backward, forward cursor movements, e.g., `C-f' deletes
+text from the beginning of the search string.
+
+Lisp programs may give this variable a special value of
+\(only . OLDVAL) to enable motion yanking temporarily.
+After any subsequent Isearch command that is not point motion,
+the value of `isearch-allow-move' is set to OLDVAL."
+  :type 'boolean
+  :version "24.4"
+  :group 'isearch)
+
 (defalias 'isearch-other-control-char 'isearch-other-meta-char)
 
 (defun isearch-other-meta-char (&optional arg)
@@ -2292,7 +2579,7 @@ (defun isearch-other-meta-char (&optiona
                 (this-command-keys)))
 	 (main-event (aref key 0))
 	 (keylist (listify-key-sequence key))
-         scroll-command isearch-point)
+         scroll-command move-command isearch-point)
     (cond ((and (= (length key) 1)
 		(let ((lookup (lookup-key local-function-key-map key)))
 		  (not (or (null lookup) (integerp lookup)
@@ -2379,7 +2666,35 @@ (defun isearch-other-meta-char (&optiona
              (if ab-bel
                  (isearch-back-into-window (eq ab-bel 'above) isearch-point)
                (goto-char isearch-point)))
           (isearch-update))
+	  ;; Handle a motion function.
+	  ((and isearch-allow-move
+		(progn (setq key (isearch-reread-key-sequence-naturally keylist))
+		       (setq keylist (listify-key-sequence key))
+		       (setq main-event (aref key 0))
+		       (setq move-command (or
+					   (isearch-lookup-move-key key)
+					   (isearch-lookup-move-key
+					    ;; Use the last key in the sequence.
+					    (vector (aref key (1- (length key)))))))))
+	   (setq prefix-arg arg)
+	   (let ((new-point (save-excursion
+			      (condition-case ()
+				  (command-execute move-command)
+				(error nil))
+			      (point))))
+	     ;; An alternative to move without yanking is (goto-char new-point)
+	     (isearch-yank-or-del (point) new-point))
+	   (unless (eq (car-safe isearch-allow-move) 'only)
+	     (setq isearch-allow-move (cons 'only isearch-allow-move))))
 	  ;; A mouse click on the isearch message starts editing the search string
 	  ((and (eq (car-safe main-event) 'down-mouse-1)
 		(window-minibuffer-p (posn-window (event-start main-event))))

bug#14742: 24.3.50; enhancement request: be able to prepend stuff from buffer when search backward

[Stefan Monnier]

> +	  ;; Handle a motion function.
> +	  ((and isearch-allow-move
> +		(progn (setq key (isearch-reread-key-sequence-naturally keylist))
> +		       (setq keylist (listify-key-sequence key))
> +		       (setq main-event (aref key 0))
> +		       (setq move-command (or
> +					   (isearch-lookup-move-key key)
> +					   (isearch-lookup-move-key
> +					    ;; Use the last key in the sequence.
> +					    (vector (aref key (1- (length key)))))))))
> +	   (setq prefix-arg arg)
> +	   (let ((new-point (save-excursion
> +			      (condition-case ()
> +				  (command-execute move-command)
> +				(error nil))
> +			      (point))))
> +	     ;; An alternative to move without yanking is (goto-char new-point)
> +	     (isearch-yank-or-del (point) new-point))
> +	   (unless (eq (car-safe isearch-allow-move) 'only)
> +	     (setq isearch-allow-move (cons 'only isearch-allow-move))))

Hmm... that still adds a crap load of code in this already too-long function.
Can't we bring it down to something like

  +       ((isearch-handle-motion-function <blabla>))


        Stefan

bug#14742: 24.3.50; enhancement request: be able to prepend stuff from buffer when search backward

[Juri Linkov]

>> +	  ;; Handle a motion function.
>> +	  ((and isearch-allow-move
>> +		(progn (setq key (isearch-reread-key-sequence-naturally keylist))
>> +		       (setq keylist (listify-key-sequence key))
>> +		       (setq main-event (aref key 0))
>> +		       (setq move-command (or
>> +					   (isearch-lookup-move-key key)
>> +					   (isearch-lookup-move-key
>> +					    ;; Use the last key in the sequence.
>> +					    (vector (aref key (1- (length key)))))))))
>> +	   (setq prefix-arg arg)
>> +	   (let ((new-point (save-excursion
>> +			      (condition-case ()
>> +				  (command-execute move-command)
>> +				(error nil))
>> +			      (point))))
>> +	     ;; An alternative to move without yanking is (goto-char new-point)
>> +	     (isearch-yank-or-del (point) new-point))
>> +	   (unless (eq (car-safe isearch-allow-move) 'only)
>> +	     (setq isearch-allow-move (cons 'only isearch-allow-move))))
>
> Hmm... that still adds a crap load of code in this already too-long function.
> Can't we bring it down to something like
>
>   +       ((isearch-handle-motion-function <blabla>))

This code is mostly a copy of code that implements the `isearch-allow-scroll'
feature.  If it's necessary to refactor the whole `isearch-other-meta-char'
then code for `isearch-allow-scroll' should be moved to a separate
function like `isearch-handle-scroll-function' too.

But still I don't understand how this would be possible to handle
outside of `isearch-other-meta-char', because this code changes the
local variables `key', `keylist', `main-event', `move-command'
locally bound in `isearch-other-meta-char'.

bug#14742: 24.3.50; enhancement request: be able to prepend stuff from buffer when search backward

[Stefan Monnier]

> then code for `isearch-allow-scroll' should be moved to a separate
> function like `isearch-handle-scroll-function' too.

That would be nice, as well, yes.

> But still I don't understand how this would be possible to handle
> outside of `isearch-other-meta-char', because this code changes the
> local variables `key', `keylist', `main-event', `move-command'
> locally bound in `isearch-other-meta-char'.

AFAICT, `move-command' is not used by the rest of the code, so `setq'ing
it is not needed.  For the other three, I guess you could do something
like

   +       ((let ((hmf (isearch-handle-motion-function <blabla>)))
   +          (when hmf (setq key (nth 0 hmf))
   +                    (setq keylist (nth 1 hmf))
   +                    (setq main-event (nth 2 hmf))
   +                    (nth 3 hmf))))

It's not super elegant, but the reason is the messed up data-flow, so at
least this formulation makes the mes more clear and self-evident.
Hopefully someone will find a way to clean it up at some point.


        Stefan

bug#14742: 24.3.50; enhancement request: be able to prepend stuff from buffer when search backward

[Lars Ingebrigtsen]

Juri Linkov <juri@jurta.org> writes:

>> Could you move this new code to a separate function?
>
> In the following patch I moved new code to a separate function
> `isearch-yank-or-del' that works like a combination of the existing
> functions `isearch-yank-string' and `isearch-del-char'.
> `isearch-yank-or-del' is general enough that it can be used
> separately from the movement-yanking feature.

Stefan had some further comments about the amount of code left in
isearch-other-meta-char, but since the code alters several variables
local to that function, the proposed solution for that looked (to me) a
lot less understandable than Juri's patch.

But it seemed like everybody agreed that the feature was good, so
perhaps it should be applied now?  It probably needs some documentation,
though.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#14742: 24.3.50; enhancement request: be able to prepend stuff from buffer when search backward

[Drew Adams]

> Stefan had some further comments about the amount of code left in
> isearch-other-meta-char, but since the code alters several variables
> local to that function, the proposed solution for that looked (to me) a
> lot less understandable than Juri's patch.
> 
> But it seemed like everybody agreed that the feature was good, so
> perhaps it should be applied now?  It probably needs some documentation,
> though.

Thanks for working on this.

From my point of view, separate, smaller changes are
better, as that will make it easier to adapt my code
in isearch+.el.  IOW, if this were done in stages,
especially any heavy refactoring, that would be helpful.

But any way it's done will be good.

bug#14742: 24.3.50; enhancement request: be able to prepend stuff from buffer when search backward

[Juri Linkov]

>> Stefan had some further comments about the amount of code left in
>> isearch-other-meta-char, but since the code alters several variables
>> local to that function, the proposed solution for that looked (to me) a
>> lot less understandable than Juri's patch.
>>
>> But it seemed like everybody agreed that the feature was good, so
>> perhaps it should be applied now?  It probably needs some documentation,
>> though.
>
> From my point of view, separate, smaller changes are
> better, as that will make it easier to adapt my code
> in isearch+.el.  IOW, if this were done in stages,
> especially any heavy refactoring, that would be helpful.

I agree that smaller changes are better.  So first we need
to decide whether every proposed feature is useful for many users,
then implement it in the most compact and elegant way.