Subject: Re: bug#15839: 24.3.50; `isearch-allow-scroll': be able to scroll point off screen temporarily

From: Juri Linkov <[email-masked]>
Date: Thu, 29 Nov 2018 01:01:00 +0200

> But users should preferably not need to worry
> about variable interactions.  The doc for a given
> variable should make clear just what it does, and
> each variable should preferably have one behavior
> (per value chosen).

I agree that it's better to make it clear in the docstring.
Fixed in a new patch below.

> I'm guessing that nil `search-exit-option' does
> not just have "the same effect".  But (see above)
> even if it does, that doesn't mean that option
> `search-exit-option' has the same effect, because
> setting it to non-nil, ONLY to NOT have the
> effect of `unlimited' `isearch-allow-scroll',
> would presumably also have some other effect
> unrelated to allowing scrolling.

I agree that `search-exit-option' is too confusing variable
for such features.  So we have to offload it from all
unrelated features.

As the first step, I moved the recently added shift-select
feature from `search-exit-option' to its own clearly named
customizable variable `isearch-allow-shift-select'.

For the same reason, unlimited scrolling was moved to the new option
`unlimited' of `isearch-allow-scroll'.

Now finally everything looks right.  Please try a new patch:

diff --git a/lisp/isearch.el b/lisp/isearch.el
index eb0b25f9b1..8c54ab92db 100644
--- a/lisp/isearch.el
+++ b/lisp/isearch.el
@@ -72,21 +72,11 @@ search-exit-option
 If t, random control and meta characters terminate the search
 and are then executed normally.
 If `edit', edit the search string instead of exiting.
-If `move', extend the search string by motion commands
-that have the `isearch-move' property on their symbols
-equal to `enabled', or the shift-translated command is
-not disabled by the value `disabled' of the same property.
-If `shift-move', extend the search string by motion commands
-while holding down the shift key.
-Both `move' and `shift-move' extend the search string by yanking text
-that ends at the new position after moving point in the current buffer.
 If `append', the characters which you type that are not interpreted by
 the incremental search are simply appended to the search string.
 If nil, run the command without exiting Isearch."
   :type '(choice (const :tag "Terminate incremental search" t)
                  (const :tag "Edit the search string" edit)
-                 (const :tag "Extend the search string by motion commands" move)
-                 (const :tag "Extend the search string by shifted motion keys" shift-move)
                  (const :tag "Append control characters to the search string" append)
                  (const :tag "Don't terminate incremental search" nil))
   :version "27.1")
@@ -2747,8 +2737,12 @@ isearch-allow-scroll
   "Whether scrolling is allowed during incremental search.
 If non-nil, scrolling commands can be used in Isearch mode.
 However, the current match will never scroll offscreen.
+If `unlimited', the current match can scroll offscreen.
+You may want to enable `lazy-highlight-buffer' as well.
 If nil, scrolling commands will first cancel Isearch mode."
-  :type 'boolean
+  :type '(choice (const :tag "Disable scrolling" nil)
+                 (const :tag "Allow scrolling within screen" t)
+                 (const :tag "Allow scrolling offscreen" unlimited))
   :group 'isearch)
 
 (defcustom isearch-allow-prefix t
@@ -2812,6 +2806,21 @@ isearch-back-into-window
 (defvar isearch-pre-scroll-point nil)
 (defvar isearch-pre-move-point nil)
 
+(defcustom isearch-allow-shift-select nil
+  "Whether motion is allowed to select text during incremental search.
+If t, extend the search string by motion commands while holding down
+the shift key.  The search string is extended by yanking text that
+ends at the new position after moving point in the current buffer.
+If `move', extend the search string without the shift key pressed
+by motion commands that have the `isearch-move' property on their
+symbols equal to `enabled', or the shift-translated command is
+not disabled by the value `disabled' of the same property."
+  :type '(choice (const :tag "Disable shift selection" nil)
+                 (const :tag "Shifted motion keys extend the search string" t)
+                 (const :tag "Motion keys extend the search string" move))
+  :group 'isearch
+  :version "27.1")
+
 (defun isearch-pre-command-hook ()
   "Decide whether to exit Isearch mode before executing the command.
 Don't exit Isearch if the key sequence that invoked this command
@@ -2845,7 +2854,7 @@ isearch-pre-command-hook
 	       (symbolp this-command)
 	       (or (eq (get this-command 'isearch-scroll) t)
 		   (eq (get this-command 'scroll-command) t))))
-      (when isearch-allow-scroll
+      (when (and isearch-allow-scroll (not (eq isearch-allow-scroll 'unlimited)))
 	(setq isearch-pre-scroll-point (point))))
      ;; A mouse click on the isearch message starts editing the search string.
      ((and (eq (car-safe main-event) 'down-mouse-1)
@@ -2854,13 +2863,13 @@ isearch-pre-command-hook
       (read-event)
       (setq this-command 'isearch-edit-string))
      ;; Don't terminate the search for motion commands.
-     ((or (and (eq search-exit-option 'move)
+     ((or (and (eq isearch-allow-shift-select 'move)
                (symbolp this-command)
                (or (eq (get this-command 'isearch-move) 'enabled)
                    (and (not (eq (get this-command 'isearch-move) 'disabled))
                         (stringp (nth 1 (interactive-form this-command)))
                         (string-match-p "^^" (nth 1 (interactive-form this-command))))))
-          (and (eq search-exit-option 'shift-move)
+          (and isearch-allow-shift-select (not (eq isearch-allow-shift-select 'move))
                this-command-keys-shift-translated))
       (setq this-command-keys-shift-translated nil)
       (setq isearch-pre-move-point (point)))
@@ -2883,7 +2892,7 @@ isearch-post-command-hook
 	(goto-char isearch-pre-scroll-point)))
     (setq isearch-pre-scroll-point nil)
     (isearch-update))
-   ((memq search-exit-option '(move shift-move))
+   (isearch-allow-shift-select
     (when (and isearch-pre-move-point
                (not (eq isearch-pre-move-point (point))))
       (let ((string (buffer-substring-no-properties