bug#11378: 24.1.50; Suggestion: Let M-i in isearch cycle `search-invisible'

bug#11378: 24.1.50; Suggestion: Let M-i in isearch cycle `search-invisible'

[Michael Heerdegen]

Hi,

I want to suggest something like that:

   (defun isearch-cycle-search-invisible ()
     "Cycle the value of `search-invisible'.
   Bound to \\<isearch-mode-map>\\[isearch-cycle-search-invisible] in `isearch'."
     (interactive)
     (setq search-invisible
           (case search-invisible
             ((nil) t)
             ((t)   'open)
             (else  nil)))
     (message "search-invisible: %s" search-invisible)
     (sit-for 1.)
     (setq isearch-success t isearch-adjusted t)
     (isearch-update))
   
   (define-key isearch-mode-map [?\M-i]
          'isearch-cycle-search-invisible)

Because like regexp search, word search and case sensitivity,
`search-invisible' is not something a user will set once and forever
in his config - different values are useful in different contexts.

Many users will want to use the default value 'open most of the time,
but the other two possible values are also very useful sometimes,
e.g. to limit searching to visible parts in a folded buffer (headlines
in org, or an outline produced with hideshow-mode etc.).

So, it would be good if the user could change the value of
`search-invisible' as well directly from isearch.


Thanks,

Michael.


In GNU Emacs 24.1.50.1 (i486-pc-linux-gnu, GTK+ Version 3.4.1)
 of 2012-04-23 on zelenka, modified by Debian
 (emacs-snapshot package, version 2:20120423-1)
Windowing system distributor `The X.Org Foundation', version 11.0.11104000
Configured using:
 `configure '--build' 'i486-linux-gnu' '--host' 'i486-linux-gnu'
 '--prefix=/usr' '--sharedstatedir=/var/lib' '--libexecdir=/usr/lib'
 '--localstatedir=/var' '--infodir=/usr/share/info'
 '--mandir=/usr/share/man' '--with-pop=yes'
 '--enable-locallisppath=/etc/emacs-snapshot:/etc/emacs:/usr/local/share/emacs/24.1.50/site-lisp:/usr/local/share/emacs/site-lisp:/usr/share/emacs/24.1.50/site-lisp:/usr/share/emacs/site-lisp'
 '--without-compress-info' '--with-crt-dir=/usr/lib/i386-linux-gnu/'
 '--with-x=yes' '--with-x-toolkit=gtk3' '--with-imagemagick=yes'
 'build_alias=i486-linux-gnu' 'host_alias=i486-linux-gnu'
 'CFLAGS=-DDEBIAN -DSITELOAD_PURESIZE_EXTRA=5000 -g -O2''

bug#11378: 24.1.50; Suggestion: Let M-i in isearch cycle `search-invisible'

[Stefan Monnier]

>    (defun isearch-cycle-search-invisible ()
>      "Cycle the value of `search-invisible'.
>    Bound to \\<isearch-mode-map>\\[isearch-cycle-search-invisible] in `isearch'."
>      (interactive)
>      (setq search-invisible
>            (case search-invisible
>              ((nil) t)
>              ((t)   'open)
>              (else  nil)))

I personally find the "search within hidden text without opening it" to
be a completely useless option, so I couldn't want to cycle between all
3 options, but only between nil and `open'.

>    (define-key isearch-mode-map [?\M-i]
>           'isearch-cycle-search-invisible)

You'll have to place it in the M-s prefix map instead, which was setup
specifically to avoid eating up more key combinations which currently
make isearch exit.

> So, it would be good if the user could change the value of
> `search-invisible' as well directly from isearch.

I'm not sure I'd find it useful, but I can imagine some people might
like it and there's plenty of room under the M-s prefix for such
a thing.
So if you can address the above two comments, and provide the result as
a patch against isearch.el, we can install it.


        Stefan

bug#11378: 24.1.50; Suggestion: Let M-i in isearch cycle `search-invisible'

[Drew Adams]

> I personally find the "search within hidden text without 
> opening it" to be a completely useless option, so I couldn't
> want to cycle between all 3 options, but only between nil and
> `open'.

FWIW, in isearch+.el I have a key that toggles between nil and the last non-nil
value of the option.  So if the user customizes the option to `open' then the
key toggles between `open' and nil.  But Michael might be right that some users
will want to switch between any of the 3 values.

> > So, it would be good if the user could change the value of
> > `search-invisible' as well directly from isearch.
> 
> I'm not sure I'd find it useful,

FWIW, I find the toggle useful.

bug#11378: 24.1.50; Suggestion: Let M-i in isearch cycle `search-invisible'

[Lennart Borgman]

[-- Attachment #1: Type: text/plain, Size: 1223 bytes --]

On Sun, Apr 29, 2012 at 16:46, Stefan Monnier <monnier@iro.umontreal.ca>wrote:

> >    (defun isearch-cycle-search-invisible ()
> >      "Cycle the value of `search-invisible'.
> >    Bound to \\<isearch-mode-map>\\[isearch-cycle-search-invisible] in
> `isearch'."
> >      (interactive)
> >      (setq search-invisible
> >            (case search-invisible
> >              ((nil) t)
> >              ((t)   'open)
> >              (else  nil)))
>
> I personally find the "search within hidden text without opening it" to
> be a completely useless option, so I couldn't want to cycle between all
> 3 options, but only between nil and `open'.
>
> >    (define-key isearch-mode-map [?\M-i]
> >           'isearch-cycle-search-invisible)
>
> You'll have to place it in the M-s prefix map instead, which was setup
> specifically to avoid eating up more key combinations which currently
> make isearch exit.
>
> > So, it would be good if the user could change the value of
> > `search-invisible' as well directly from isearch.
>
> I'm not sure I'd find it useful, but I can imagine some people might
> like it and there's plenty of room under the M-s prefix for such
> a thing.
>
> It is in my opinion very useful in org-mode.

[-- Attachment #2: Type: text/html, Size: 1725 bytes --]

bug#11378: 24.1.50; Suggestion: Let M-i in isearch cycle `search-invisible'

[Juri Linkov]

> You'll have to place it in the M-s prefix map instead, which was setup
> specifically to avoid eating up more key combinations which currently
> make isearch exit.

Or more specifically to the submap `M-s f' (where `f' means `filter')
such as e.g. `M-s f i' for `search-invisible' (or maybe `M-s f v'
since the filter predicate name is `isearch-filter-visible').

There are more isearch filters that could share the same map `M-s f'.

bug#11378: 24.1.50; Suggestion: Let M-i in isearch cycle `search-invisible'

[Juri Linkov]

> There are more isearch filters that could share the same map `M-s f'.

Actually, currently these isearch filters can be used only one at a time.

It would be more useful to allow multiple filters by transforming
`isearch-filter-predicate' from the variable defining a predicate to the
hook-like list defining a set of predicates all of which should satisfy
for the search hit.

The default value could be a list with the current default element
`isearch-filter-visible'.  And it still could allow a symbol as its value
for backward compatibility.

bug#11378: 24.1.50; Suggestion: Let M-i in isearch cycle `search-invisible'

[Stefan Monnier]

>> There are more isearch filters that could share the same map `M-s f'.
> Actually, currently these isearch filters can be used only one at a time.

> It would be more useful to allow multiple filters by transforming
> `isearch-filter-predicate' from the variable defining a predicate to the
> hook-like list defining a set of predicates all of which should satisfy
> for the search hit.

Sounds good to me.  Not sure how you'll fix the

	     (and (eq isearch-filter-predicate 'isearch-filter-visible)
		  search-invisible))

(which I already had annotated in my local tree with the comment
"FIXME: equality comparisons on functions is asking for trouble.")

> The default value could be a list with the current default element
> `isearch-filter-visible'.  And it still could allow a symbol as its value
> for backward compatibility.

Yes, you could just `run-hook-with-args-until-success'.


        Stefan

bug#11378: 24.1.50; Suggestion: Let M-i in isearch cycle `search-invisible'

[Drew Adams]

> It would be more useful to allow multiple filters by transforming
> `isearch-filter-predicate' from the variable defining a 
> predicate to the
> hook-like list defining a set of predicates all of which 
> should satisfy for the search hit.
> 
> The default value could be a list with the current default element
> `isearch-filter-visible'.  And it still could allow a symbol 
> as its value for backward compatibility.

+1 

bug#11378: 24.1.50; Suggestion: Let M-i in isearch cycle `search-invisible'

[Juri Linkov]

> Sounds good to me.  Not sure how you'll fix the
>
> 	     (and (eq isearch-filter-predicate 'isearch-filter-visible)
> 		  search-invisible))

This condition is used to set the value of
`inhibit-point-motion-hooks'.  But I don't understand
the purpose of using `inhibit-point-motion-hooks' here
and why it depends on `search-invisible'.

The documentation says that when `inhibit-point-motion-hooks' is
non-nil, the `intangible' property is ignored.  But how this is
related to the `invisible' property?

Maybe we could bind `inhibit-point-motion-hooks' to t
unconditionally because `isearch-done' takes care
about moving point outside of intangible text,
and probably there are no more bad effects.

Another reason why this condition should be removed
is that other filters call `isearch-filter-visible' too
(as the first thing they test in their function body).
And it's impossible to list all such filters in the condition
that binds the value of `inhibit-point-motion-hooks' to t.

bug#11378: 24.1.50; Suggestion: Let M-i in isearch cycle `search-invisible'

[Juri Linkov]

> It would be more useful to allow multiple filters by transforming
> `isearch-filter-predicate' from the variable defining a predicate to the
> hook-like list defining a set of predicates all of which should satisfy
> for the search hit.

This is implemented by the following patch that also simplifies
the filter usage in other packages and adds two useful filters for
comments/strings:

=== modified file 'lisp/isearch.el'
--- lisp/isearch.el	2012-05-29 09:46:06 +0000
+++ lisp/isearch.el	2012-05-29 16:35:09 +0000
@@ -179,12 +179,13 @@
   "Function to save a function restoring the mode-specific Isearch state
 to the search status stack.")
 
-(defvar isearch-filter-predicate 'isearch-filter-visible
-  "Predicate that filters the search hits that would normally be available.
-Search hits that dissatisfy the predicate are skipped.  The function
-has two arguments: the positions of start and end of text matched by
-the search.  If this function returns nil, continue searching without
-stopping at this match.")
+(defvar isearch-filter-predicate '(isearch-filter-visible)
+  "Predicates that filter the search hits that would normally be available.
+Search hits that dissatisfy the list of predicates are skipped.
+Each function in this list has two arguments: the positions of
+start and end of text matched by the search.
+If `run-hook-with-args-until-failure' returns nil for all predicates,
+continue searching without stopping at this match.")
 
 ;; Search ring.
 
@@ -506,6 +516,9 @@ (defvar isearch-mode-map
     (define-key map "\M-sw" 'isearch-toggle-word)
     (define-key map "\M-s_" 'isearch-toggle-symbol)
 
+    (define-key map "\M-sfs" 'isearch-toggle-filter-strings)
+    (define-key map "\M-sfc" 'isearch-toggle-filter-comments)
+
     (define-key map [?\M-%] 'isearch-query-replace)
     (define-key map [?\C-\M-%] 'isearch-query-replace-regexp)
     (define-key map "\M-so" 'isearch-occur)
@@ -665,6 +681,10 @@ (defun isearch-forward (&optional regexp
 Type \\[isearch-toggle-regexp] to toggle regular-expression mode.
 Type \\[isearch-toggle-word] to toggle word mode.
 Type \\[isearch-toggle-symbol] to toggle symbol mode.
+
+Type \\[isearch-toggle-filter-strings] to toggle searching inside strings.
+Type \\[isearch-toggle-filter-comments] to toggle searching inside comments.
+
 Type \\[isearch-edit-string] to edit the search string in the minibuffer.
 
 Also supported is a search ring of the previous 16 search strings.
@@ -1036,6 +1056,9 @@ (defsubst isearch-case-fold-search-state
 (defsubst isearch-pop-fun-state (frame)
   "Return the function restoring the mode-specific Isearch state in FRAME."
   (aref frame 11))
+(defsubst isearch-filter-predicate-state (frame)
+  "Return the filter predicates in FRAME."
+  (aref frame 12))
 
 (defun isearch-top-state ()
   (let ((cmd (car isearch-cmds)))
@@ -1048,7 +1071,8 @@ (defun isearch-top-state ()
 	  isearch-error (isearch-error-state cmd)
 	  isearch-wrapped (isearch-wrapped-state cmd)
 	  isearch-barrier (isearch-barrier-state cmd)
-	  isearch-case-fold-search (isearch-case-fold-search-state cmd))
+	  isearch-case-fold-search (isearch-case-fold-search-state cmd)
+	  isearch-filter-predicate (isearch-filter-predicate-state cmd))
     (if (functionp (isearch-pop-fun-state cmd))
 	(funcall (isearch-pop-fun-state cmd) cmd))
     (goto-char (isearch-point-state cmd))))
@@ -1065,7 +1089,8 @@ (defun isearch-push-state ()
 		      isearch-error isearch-wrapped isearch-barrier
 		      isearch-case-fold-search
 		      (if isearch-push-state-function
-			  (funcall isearch-push-state-function)))
+			  (funcall isearch-push-state-function))
+		      isearch-filter-predicate)
 	      isearch-cmds)))
 
 \f
@@ -1411,6 +1436,40 @@ (defun isearch-toggle-case-fold ()
   (sit-for 1)
   (isearch-update))
 
+(defun isearch-toggle-filter-strings ()
+  "Toggle searching inside strings on or off."
+  (interactive)
+  (setq isearch-filter-predicate
+	(if (memq 'isearch-filter-strings isearch-filter-predicate)
+	    (delq 'isearch-filter-strings isearch-filter-predicate)
+	  (cons 'isearch-filter-strings isearch-filter-predicate)))
+  (setq isearch-success t isearch-adjusted t)
+  (isearch-update))
+
+(defun isearch-filter-strings (beg end)
+  "Test whether the current search hit is inside strings.
+Return non-nil if the text from BEG to END is inside strings."
+  (nth 3 (parse-partial-sexp (point-min) (point))))
+
+(put 'isearch-filter-strings 'isearch-message-prefix "string ")
+
+(defun isearch-toggle-filter-comments ()
+  "Toggle searching inside comments on or off."
+  (interactive)
+  (setq isearch-filter-predicate
+	(if (memq 'isearch-filter-comments isearch-filter-predicate)
+	    (delq 'isearch-filter-comments isearch-filter-predicate)
+	  (cons 'isearch-filter-comments isearch-filter-predicate)))
+  (setq isearch-success t isearch-adjusted t)
+  (isearch-update))
+
+(defun isearch-filter-comments (beg end)
+  "Test whether the current search hit is inside comments.
+Return non-nil if the text from BEG to END is inside comments."
+  (nth 4 (parse-partial-sexp (point-min) (point))))
+
+(put 'isearch-filter-comments 'isearch-message-prefix "comment ")
+
 \f
 ;; Word search
 
@@ -2373,6 +2439,11 @@ (defun isearch-message-prefix (&optional
 			      (< (point) isearch-opoint)))
 		       "over")
 		   (if isearch-wrapped "wrapped ")
+		   (mapconcat (lambda (s)
+				(and (symbolp s)
+				     (get s 'isearch-message-prefix)))
+			      isearch-filter-predicate
+			      "")
 		   (if isearch-word
 		       (or (and (symbolp isearch-word)
 				(get isearch-word 'isearch-message-prefix))
@@ -2489,13 +2560,11 @@ (defun isearch-search ()
       (setq isearch-case-fold-search
 	    (isearch-no-upper-case-p isearch-string isearch-regexp)))
   (condition-case lossage
-      (let ((inhibit-point-motion-hooks
-	     (and (eq isearch-filter-predicate 'isearch-filter-visible)
-		  search-invisible))
+      (let ((inhibit-point-motion-hooks search-invisible)
 	    (inhibit-quit nil)
 	    (case-fold-search isearch-case-fold-search)
 	    (search-spaces-regexp search-whitespace-regexp)
@@ -2509,8 +2578,9 @@ (defun isearch-search ()
 	  (if (or (not isearch-success)
 		  (bobp) (eobp)
 		  (= (match-beginning 0) (match-end 0))
-		  (funcall isearch-filter-predicate
-			   (match-beginning 0) (match-end 0)))
+		  (run-hook-with-args-until-failure
+		   'isearch-filter-predicate
+		   (match-beginning 0) (match-end 0)))
 	      (setq retry nil)))
 	(setq isearch-just-started nil)
 	(if isearch-success
@@ -2895,8 +3005,9 @@ (defun isearch-lazy-highlight-search ()
 	  (if (or (not success)
 		  (= (point) bound) ; like (bobp) (eobp) in `isearch-search'.
 		  (= (match-beginning 0) (match-end 0))
-		  (funcall isearch-filter-predicate
-			   (match-beginning 0) (match-end 0)))
+		  (run-hook-with-args-until-failure
+		   'isearch-filter-predicate
+		   (match-beginning 0) (match-end 0)))
 	      (setq retry nil)))
 	success)
     (error nil)))

=== modified file 'lisp/dired-aux.el'
--- lisp/dired-aux.el	2012-04-17 01:52:00 +0000
+++ lisp/dired-aux.el	2012-05-29 16:35:30 +0000
@@ -2424,8 +2425,6 @@ (defcustom dired-isearch-filenames nil
   :group 'dired
   :version "23.1")
 
-(defvar dired-isearch-filter-predicate-orig nil)
-
 (defun dired-isearch-filenames-toggle ()
   "Toggle file names searching on or off.
 When on, Isearch skips matches outside file names using the predicate
@@ -2433,9 +2432,9 @@ (defun dired-isearch-filenames-toggle ()
 When off, it uses the original predicate."
   (interactive)
   (setq isearch-filter-predicate
-	(if (eq isearch-filter-predicate 'dired-isearch-filter-filenames)
-	    dired-isearch-filter-predicate-orig
-	  'dired-isearch-filter-filenames))
+	(if (memq 'dired-isearch-filter-filenames isearch-filter-predicate)
+	    (delq 'dired-isearch-filter-filenames isearch-filter-predicate)
+	  (cons 'dired-isearch-filter-filenames isearch-filter-predicate)))
   (setq isearch-success t isearch-adjusted t)
   (isearch-update))
 
@@ -2446,29 +2445,29 @@ (defun dired-isearch-filenames-setup ()
   (when (or (eq dired-isearch-filenames t)
 	    (and (eq dired-isearch-filenames 'dwim)
 		 (get-text-property (point) 'dired-filename)))
-    (setq isearch-message-prefix-add "filename ")
-    (define-key isearch-mode-map "\M-sf" 'dired-isearch-filenames-toggle)
-    (setq dired-isearch-filter-predicate-orig
-	  (default-value 'isearch-filter-predicate))
-    (setq-default isearch-filter-predicate 'dired-isearch-filter-filenames)
+    (define-key isearch-mode-map "\M-sff" 'dired-isearch-filenames-toggle)
+    (setq isearch-filter-predicate
+	  (cons 'dired-isearch-filter-filenames isearch-filter-predicate))
     (add-hook 'isearch-mode-end-hook 'dired-isearch-filenames-end nil t)))
 
 (defun dired-isearch-filenames-end ()
   "Clean up the Dired file name search after terminating isearch."
   (setq isearch-message-prefix-add nil)
-  (define-key isearch-mode-map "\M-sf" nil)
-  (setq-default isearch-filter-predicate dired-isearch-filter-predicate-orig)
+  (define-key isearch-mode-map "\M-sff" nil)
+  (setq isearch-filter-predicate
+	(delq 'dired-isearch-filter-filenames isearch-filter-predicate))
   (remove-hook 'isearch-mode-end-hook 'dired-isearch-filenames-end t))
 
 (defun dired-isearch-filter-filenames (beg end)
-  "Test whether the current search hit is a visible file name.
+  "Test whether the current search hit is a file name.
 Return non-nil if the text from BEG to END is part of a file
-name (has the text property `dired-filename') and is visible."
-  (and (isearch-filter-visible beg end)
-       (if dired-isearch-filenames
-	   (text-property-not-all (min beg end) (max beg end)
-				  'dired-filename nil)
-	 t)))
+name (has the text property `dired-filename')."
+  (if dired-isearch-filenames
+      (text-property-not-all (min beg end) (max beg end)
+			     'dired-filename nil)
+    t))
+
+(put 'dired-isearch-filter-filenames 'isearch-message-prefix "filename ")
 
 ;;;###autoload
 (defun dired-isearch-filenames ()

=== modified file 'lisp/info.el'
--- lisp/info.el	2012-05-29 09:09:38 +0000
+++ lisp/info.el	2012-05-29 16:39:00 +0000
@@ -1772,7 +1775,8 @@ (defun Info-search (regexp &optional bou
 			      (point-max)))
 	  (while (and (not give-up)
 		      (or (null found)
-			  (not (funcall isearch-filter-predicate beg-found found))))
+			  (not (run-hook-with-args-until-failure
+				'isearch-filter-predicate beg-found found))))
 	    (let ((search-spaces-regexp
 		   (if (or (not isearch-mode) isearch-regexp)
 		       Info-search-whitespace-regexp)))
@@ -1854,7 +1858,8 @@ (defun Info-search (regexp &optional bou
 		(setq give-up nil found nil)
 		(while (and (not give-up)
 			    (or (null found)
-				(not (funcall isearch-filter-predicate beg-found found))))
+				(not (run-hook-with-args-until-failure
+				      'isearch-filter-predicate beg-found found))))
 		  (let ((search-spaces-regexp
 			 (if (or (not isearch-mode) isearch-regexp)
 			     Info-search-whitespace-regexp)))
@@ -4049,7 +4054,7 @@ (define-derived-mode Info-mode nil "Info
   (set (make-local-variable 'isearch-push-state-function)
        'Info-isearch-push-state)
   (set (make-local-variable 'isearch-filter-predicate)
-       'Info-isearch-filter)
+       '(Info-isearch-filter))
   (set (make-local-variable 'search-whitespace-regexp)
        Info-search-whitespace-regexp)
   (set (make-local-variable 'revert-buffer-function)

=== modified file 'lisp/wdired.el'
--- lisp/wdired.el	2012-01-19 07:21:25 +0000
+++ lisp/wdired.el	2012-05-29 16:35:52 +0000
@@ -216,8 +216,7 @@ (defun wdired-change-to-wdired-mode ()
        (buffer-substring (point-min) (point-max)))
   (set (make-local-variable 'wdired-old-point) (point))
   (set (make-local-variable 'query-replace-skip-read-only) t)
-  (set (make-local-variable 'isearch-filter-predicate)
-       'wdired-isearch-filter-read-only)
+  (add-hook 'isearch-filter-predicate 'wdired-isearch-filter-read-only nil t)
   (use-local-map wdired-mode-map)
   (force-mode-line-update)
   (setq buffer-read-only nil)
@@ -245,9 +244,8 @@ (defun wdired-change-to-wdired-mode ()
 
 (defun wdired-isearch-filter-read-only (beg end)
   "Skip matches that have a read-only property."
-  (and (isearch-filter-visible beg end)
-       (not (text-property-not-all (min beg end) (max beg end)
-				   'read-only nil))))
+  (not (text-property-not-all (min beg end) (max beg end)
+			      'read-only nil)))
 
 ;; Protect the buffer so only the filenames can be changed, and put
 ;; properties so filenames (old and new) can be easily found.

bug#11378: 24.1.50; Suggestion: Let M-i in isearch cycle `search-invisible'

[Stefan Monnier]

> This is implemented by the following patch that also simplifies
> the filter usage in other packages and adds two useful filters for
> comments/strings:

Thanks, see comments below.

> -(defvar isearch-filter-predicate 'isearch-filter-visible
> -  "Predicate that filters the search hits that would normally be available.
> -Search hits that dissatisfy the predicate are skipped.  The function
> -has two arguments: the positions of start and end of text matched by
> -the search.  If this function returns nil, continue searching without
> -stopping at this match.")
> +(defvar isearch-filter-predicate '(isearch-filter-visible)
> +  "Predicates that filter the search hits that would normally be available.
> +Search hits that dissatisfy the list of predicates are skipped.
> +Each function in this list has two arguments: the positions of
> +start and end of text matched by the search.
> +If `run-hook-with-args-until-failure' returns nil for all predicates,
> +continue searching without stopping at this match.")

I suggest renaming it to isearch-filter-predicates or
isearch-filter-functions.  Also the docstring should describe its
`isearch-message-prefix' property (BTW, why "prefix"?).

> +Type \\[isearch-toggle-filter-strings] to toggle searching inside strings.
> +Type \\[isearch-toggle-filter-comments] to toggle searching inside comments.

To me, the most useful filter would be "search in comments and strings"
or "search in code only (i.e. exclude comments and strings)", neither of
which is covered by those two toggles.

> +(defsubst isearch-filter-predicate-state (frame)
> +  "Return the filter predicates in FRAME."
> +  (aref frame 12))

Any taker for (defstruct isearch--state ...)?

> +  (nth 3 (parse-partial-sexp (point-min) (point))))
[...]
> +  (nth 4 (parse-partial-sexp (point-min) (point))))

Please use syntax-ppss instead.

> @@ -2489,13 +2560,11 @@ (defun isearch-search ()
>        (setq isearch-case-fold-search
>  	    (isearch-no-upper-case-p isearch-string isearch-regexp)))
>    (condition-case lossage
> -      (let ((inhibit-point-motion-hooks
> -	     (and (eq isearch-filter-predicate 'isearch-filter-visible)
> -		  search-invisible))
> +      (let ((inhibit-point-motion-hooks search-invisible)
>  	    (inhibit-quit nil)
>  	    (case-fold-search isearch-case-fold-search)
>  	    (search-spaces-regexp search-whitespace-regexp)

Why is this correct?


        Stefan

bug#11378: 24.1.50; Suggestion: Let M-i in isearch cycle `search-invisible'

[Juri Linkov]

> I suggest renaming it to isearch-filter-predicates or
> isearch-filter-functions.

isearch-filter-functions is more usual name for the hook-like
functionality than isearch-filter-predicates.

> Also the docstring should describe its
> `isearch-message-prefix' property (BTW, why "prefix"?).

The prefix is what's displayed before the search string in the message.
The suffix is after the search string in square brackets.

>> +Type \\[isearch-toggle-filter-strings] to toggle searching inside strings.
>> +Type \\[isearch-toggle-filter-comments] to toggle searching inside comments.
>
> To me, the most useful filter would be "search in comments and strings"
> or "search in code only (i.e. exclude comments and strings)", neither of
> which is covered by those two toggles.

Then the combination of comments and strings could be named "text" with
its filter bound to `M-s f t'.  And the inverse filter to exclude
comments and strings could be bound to `M-s f T'.  Also it could be
enabled only in prog-mode.

>> @@ -2489,13 +2560,11 @@ (defun isearch-search ()
>>        (setq isearch-case-fold-search
>>  	    (isearch-no-upper-case-p isearch-string isearch-regexp)))
>>    (condition-case lossage
>> -      (let ((inhibit-point-motion-hooks
>> -	     (and (eq isearch-filter-predicate 'isearch-filter-visible)
>> -		  search-invisible))
>> +      (let ((inhibit-point-motion-hooks search-invisible)
>>  	    (inhibit-quit nil)
>>  	    (case-fold-search isearch-case-fold-search)
>>  	    (search-spaces-regexp search-whitespace-regexp)
>
> Why is this correct?

I believe that binding `inhibit-point-motion-hooks' to t unconditionally
shouldn't cause an unwanted effect because `isearch-done' takes care
about moving point outside of intangible text at the end of the search
(the source lines that begin with the comment ";; If we ended in the middle
of some intangible text, move to the further end of that intangible text.")
But currently I know no test case to prove the effect of
`inhibit-point-motion-hooks'.

BTW, in etc/TODO I noticed this task:

*** Outlining
**** Support for incremental search automatically making hidden text visible.

It seems this is already implemented at least for outline
(but needs more testing in org-mode).

Anyway I intend to rewrite the filter `isearch-filter-visible' to just
check if the text is visible.  The code that opens overlays should be
moved to the main search loop to work independently from the filter.

bug#11378: 24.1.50; Suggestion: Let M-i in isearch cycle `search-invisible'

[Stefan Monnier]

>> I suggest renaming it to isearch-filter-predicates or
>> isearch-filter-functions.
> isearch-filter-functions is more usual name for the hook-like
> functionality than isearch-filter-predicates.

By convention, "-functions" is used for non-standard hooks, of which
predicates are one particular subset.  But "-predicates" is fine by me.

> Then the combination of comments and strings could be named "text" with
> its filter bound to `M-s f t'.  And the inverse filter to exclude
> comments and strings could be bound to `M-s f T'.  Also it could be
> enabled only in prog-mode.

I was thinking that maybe a cycling behavior would be better than
toggling for these: off / foo-only / foo-excluded.

> I believe that binding `inhibit-point-motion-hooks' to t unconditionally
> shouldn't cause an unwanted effect because `isearch-done' takes care
> about moving point outside of intangible text at the end of the search
> (the source lines that begin with the comment ";; If we ended in the middle
> of some intangible text, move to the further end of that intangible text.")
> But currently I know no test case to prove the effect of
> `inhibit-point-motion-hooks'.

I see.  Since I hate intangible, it's probably OK ;-)


        Stefan

bug#11378: 24.1.50; Suggestion: Let M-i in isearch cycle `search-invisible'

[Juri Linkov]

>> Then the combination of comments and strings could be named "text" with
>> its filter bound to `M-s f t'.  And the inverse filter to exclude
>> comments and strings could be bound to `M-s f T'.  Also it could be
>> enabled only in prog-mode.
>
> I was thinking that maybe a cycling behavior would be better than
> toggling for these: off / foo-only / foo-excluded.

Maybe something like this:

(define-key isearch-mode-map "\M-sft" 'isearch-cycle-filter-text)

(defun isearch-cycle-filter-text ()
  "Cycle searching inside code vs strings/comments vs off."
  (interactive)
  (setq isearch-filter-predicates
	(cond ((memq 'isearch-filter-text isearch-filter-predicates)
               (cons 'isearch-filter-nontext
                     (delq 'isearch-filter-text isearch-filter-predicates)))
              ((memq 'isearch-filter-nontext isearch-filter-predicates)
               (delq 'isearch-filter-nontext isearch-filter-predicates))
              (t (cons 'isearch-filter-text isearch-filter-predicates))))
  (setq isearch-success t isearch-adjusted t)
  (isearch-update))

(defun isearch-filter-nontext (beg end)
  "Test whether the current search hit is not inside text.
Return non-nil if the text from BEG to END is not inside
strings or comments."
  (not (isearch-filter-text beg end)))

(put 'isearch-filter-nontext 'isearch-message-prefix "code ")

(defun isearch-filter-text (beg end)
  "Test whether the current search hit is inside text.
Return non-nil if the text from BEG to END is inside
strings or comments."
  (or (isearch-filter-comments beg end)
      (isearch-filter-strings beg end)))

(put 'isearch-filter-text 'isearch-message-prefix "text ")

(defun isearch-filter-strings (beg end)
  "Test whether the current search hit is inside strings.
Return non-nil if the text from BEG to END is inside strings."
  (save-match-data (nth 3 (syntax-ppss))))

(put 'isearch-filter-strings 'isearch-message-prefix "string ")

(defun isearch-filter-comments (beg end)
  "Test whether the current search hit is inside comments.
Return non-nil if the text from BEG to END is inside comments."
  (save-match-data (nth 4 (syntax-ppss))))

(put 'isearch-filter-comments 'isearch-message-prefix "comment ")

bug#11378: 24.1.50; Suggestion: Let M-i in isearch cycle `search-invisible'

[Stefan Monnier]

>> I was thinking that maybe a cycling behavior would be better than
>> toggling for these: off / foo-only / foo-excluded.
> Maybe something like this:

Sounds about right, thank you,


        Stefan

bug#11378: 24.1.50; Suggestion: Let M-i in isearch cycle `search-invisible'

[Juri Linkov]

> Anyway I intend to rewrite the filter `isearch-filter-visible' to just
> check if the text is visible.  The code that opens overlays should be
> moved to the main search loop to work independently from the filter.

I believe the following patch implements the correct interaction of
filter predicates with search-invisible.

It also removes `isearch-filter-visible' from the default value of
`isearch-filter-predicates' because in the current implementation
`isearch-filter-visible' in the list of filters makes no sense since
it can't be removed from the filter list.  Removing it from the list
with the intention to find hidden matches won't open overlays with hidden text
ignoring the value of `search-invisible'.

This requires moving the `isearch-range-invisible' call to the
main search loop, just after running hook-with-args
with `isearch-filter-predicates'.

There is one remaining question:
do we still need a filter for visible text?

Such filter could just check if the match is visible,
but it should not open overlays.  This can be implemented like:

(defun isearch-filter-visible (beg end)
  (let ((search-invisible nil))
    (not (isearch-range-invisible beg end))))

Adding this filter to `isearch-filter-predicates' will skip hidden text,
removing it will find matches in hidden text (and the main search loop
will open overlays).

But apparently this filter is not necessary, because handling
of hidden text (depending on the customizable `search-invisible')
will be moved to the main search loop.

So the question is whether a new command to toggle searching hidden text
should add/remove `isearch-filter-visible' from `isearch-filter-predicates',
or just change the value of `search-invisible'.

I guess to change the value of `search-invisible' (defined by `defcustom')
with a new toggling command is not the right way to change the value
of customizable variables (or maybe not).

OTOH, `isearch-range-invisible' might be called twice: one call from
`isearch-filter-visible' and another from the main search loop,
and this is not optimal.

Anyway below is the patch that should work for a list of filters
other than `isearch-filter-visible':

=== modified file 'lisp/isearch.el'
--- lisp/isearch.el	2012-05-29 09:46:06 +0000
+++ lisp/isearch.el	2012-06-11 23:34:18 +0000
@@ -179,12 +179,15 @@ (defvar isearch-push-state-function nil
   "Function to save a function restoring the mode-specific Isearch state
 to the search status stack.")
 
-(defvar isearch-filter-predicate 'isearch-filter-visible
-  "Predicate that filters the search hits that would normally be available.
-Search hits that dissatisfy the predicate are skipped.  The function
-has two arguments: the positions of start and end of text matched by
-the search.  If this function returns nil, continue searching without
-stopping at this match.")
+(defvar isearch-filter-predicates nil
+  "Predicates that filter the search hits that would normally be available.
+Search hits that dissatisfy the list of predicates are skipped.
+Each function in this list has two arguments: the positions of
+start and end of text matched by the search.
+If `run-hook-with-args-until-failure' returns nil for all predicates,
+continue searching without stopping at this match.
+The property `isearch-message-prefix' put on the predicate's symbol
+specifies the prefix string displyed in the search message.")
 
 ;; Search ring.
 
@@ -2509,8 +2553,15 @@ (defun isearch-search ()
 	  (if (or (not isearch-success)
 		  (bobp) (eobp)
 		  (= (match-beginning 0) (match-end 0))
-		  (funcall isearch-filter-predicate
-			   (match-beginning 0) (match-end 0)))
+		  ;; When one of filter predicates returns nil,
+		  ;; retry the search.  Otherwise, act according
+		  ;; to search-invisible (open overlays, etc.)
+		  (and (run-hook-with-args-until-failure
+			'isearch-filter-predicates
+			(match-beginning 0) (match-end 0))
+		       (or (eq search-invisible t)
+			   (not (isearch-range-invisible
+				 (match-beginning 0) (match-end 0))))))
 	      (setq retry nil)))
 	(setq isearch-just-started nil)
 	(if isearch-success
@@ -2873,7 +2964,6 @@ (defun isearch-lazy-highlight-search ()
 	    (isearch-regexp isearch-lazy-highlight-regexp)
 	    (search-spaces-regexp isearch-lazy-highlight-space-regexp)
 	    (isearch-word isearch-lazy-highlight-word)
-	    (search-invisible nil)	; don't match invisible text
 	    (retry t)
 	    (success nil)
 	    (isearch-forward isearch-lazy-highlight-forward)
@@ -2895,8 +2985,13 @@ (defun isearch-lazy-highlight-search ()
 	  (if (or (not success)
 		  (= (point) bound) ; like (bobp) (eobp) in `isearch-search'.
 		  (= (match-beginning 0) (match-end 0))
-		  (funcall isearch-filter-predicate
-			   (match-beginning 0) (match-end 0)))
+		  (and (run-hook-with-args-until-failure
+			'isearch-filter-predicates
+			(match-beginning 0) (match-end 0))
+		       ;; Don't match invisible text.
+		       (let ((search-invisible nil))
+			 (not (isearch-range-invisible
+			       (match-beginning 0) (match-end 0))))))
 	      (setq retry nil)))
 	success)
     (error nil)))

bug#11378: 24.1.50; Suggestion: Let M-i in isearch cycle `search-invisible'

[Juri Linkov]

> There is one remaining question:
> do we still need a filter for visible text?

I tend to conclude that a filter for visible text is not needed,
because to handle the value of `search-invisible' correctly
this filter should reside in `isearch-filter-predicates' permanently
that makes little sense.

This assumes that the variable `search-invisible' is not going to be
marked obsolete in favor of filters.  Only in this case filters would
make sense that will require two separate filters: one to just skip
hidden text (implementing the current value `nil' of `search-invisible'),
another filter to open hidden overlays (implementing the value `open' of
`search-invisible'), and no filter means the value `t' of `search-invisible'.

Otherwise, searching in hidden text should handle the variable
`search-invisible' without using filters.

bug#11378: 24.1.50; Suggestion: Let M-i in isearch cycle `search-invisible'

[Juri Linkov]

>> Anyway I intend to rewrite the filter `isearch-filter-visible' to just
>> check if the text is visible.  The code that opens overlays should be
>> moved to the main search loop to work independently from the filter.
>
> I believe the following patch implements the correct interaction of
> filter predicates with search-invisible.

I installed this patch.  Now it's possible to implement
two separate features:

1. `isearch-toggle-invisible'.  While testing the initial approach
that toggles the value of `search-invisible', I noticed one problem:
after toggling invisibility in one search, it is unexpectedly
reused later in another search.

This is not like `M-s c' (isearch-toggle-case-fold) works
where toggling the value of `isearch-case-fold-search'
affects only the current search.  But a new search is
always started with the default value of the user option
`case-fold-search'.

I think a new command `isearch-toggle-invisible' (`M-s i')
should not change the default value of the user option
`search-invisible' defined with `defcustom'.
It should be changed only via Customization UI.

I'll submit a new patch soon that adds a new variable
`isearch-search-invisible' (or maybe just `isearch-invisible')
to toggle its value without changing the value of the user option
`search-invisible'.

2. A patch that implements new search filters
`isearch-filter-text', `isearch-filter-nontext',
`isearch-filter-strings', `isearch-filter-comments'
will be posted to a separate feature request.

bug#11378: 24.1.50; Suggestion: Let M-i in isearch cycle `search-invisible'

[Juri Linkov]

> I'll submit a new patch soon that adds a new variable
> `isearch-search-invisible' (or maybe just `isearch-invisible')
> to toggle its value without changing the value of the user option
> `search-invisible'.

Here is a patch that adds `isearch-toggle-invisible' that
toggles `isearch-invisible' between nil and a non-nil default value
of `search-invisible', i.e.

if search-invisible is `open' then toggles isearch-invisible
between `open' and `nil'

if search-invisible is `t' then toggles isearch-invisible
between `t' and `nil'

if search-invisible is `nil' then toggles isearch-invisible
between `nil' and `open'

=== modified file 'lisp/isearch.el'
--- lisp/isearch.el	2013-05-27 22:42:11 +0000
+++ lisp/isearch.el	2013-05-28 21:41:15 +0000
@@ -514,6 +514,7 @@ (defvar isearch-mode-map
     (define-key map "\M-e" 'isearch-edit-string)
 
     (define-key map "\M-sc" 'isearch-toggle-case-fold)
+    (define-key map "\M-si" 'isearch-toggle-invisible)
     (define-key map "\M-sr" 'isearch-toggle-regexp)
     (define-key map "\M-sw" 'isearch-toggle-word)
     (define-key map "\M-s_" 'isearch-toggle-symbol)
@@ -602,6 +607,11 @@ (defvar isearch-start-hscroll 0)	; hscro
 ;;   case in the search string is ignored.
 (defvar isearch-case-fold-search nil)
 
+;; search-invisible while searching.
+;;   either nil, t, or 'open.  'open means the same as t except that
+;;   opens hidden overlays.
+(defvar isearch-invisible 'open)
+
 (defvar isearch-last-case-fold-search nil)
 
 ;; Used to save default value while isearch is active
@@ -700,6 +711,7 @@ (defun isearch-forward (&optional regexp
  nonincremental search.
 
 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.
 Type \\[isearch-toggle-word] to toggle word mode.
 Type \\[isearch-toggle-symbol] to toggle symbol mode.
@@ -836,6 +867,7 @@ (defun isearch-mode (forward &optional r
 	isearch-op-fun op-fun
 	isearch-last-case-fold-search isearch-case-fold-search
 	isearch-case-fold-search case-fold-search
+	isearch-invisible search-invisible
 	isearch-string ""
 	isearch-message ""
 	isearch-cmds nil
@@ -1487,6 +1527,23 @@ (defun isearch-toggle-case-fold ()
   (sit-for 1)
   (isearch-update))
 
+(defun isearch-toggle-invisible ()
+  "Toggle searching in invisible text on or off.
+Toggles the variable `isearch-invisible' between values nil
+and a non-nil default value of `search-invisible' (or `open'
+if `search-invisible' is nil by default)."
+  (interactive)
+  (setq isearch-invisible
+	(if isearch-invisible nil (or search-invisible 'open)))
+  (let ((message-log-max nil))
+    (message "%s%s [match %svisible text]"
+	     (isearch-message-prefix nil isearch-nonincremental)
+	     isearch-message
+	     (if isearch-invisible "in" "")))
+  (setq isearch-success t isearch-adjusted t)
+  (sit-for 1)
+  (isearch-update))
+
 \f
 ;; Word search
 
@@ -1622,6 +1679,7 @@ (defun isearch-query-replace (&optional
 	;; set `search-upper-case' to nil to not call
 	;; `isearch-no-upper-case-p' in `perform-replace'
 	(search-upper-case nil)
+	(search-invisible isearch-invisible)
 	(replace-lax-whitespace
 	 isearch-lax-whitespace)
 	(replace-regexp-lax-whitespace
@@ -2634,9 +2739,10 @@ (defun isearch-search ()
       (setq isearch-case-fold-search
 	    (isearch-no-upper-case-p isearch-string isearch-regexp)))
   (condition-case lossage
-      (let ((inhibit-point-motion-hooks search-invisible)
+      (let ((inhibit-point-motion-hooks isearch-invisible)
 	    (inhibit-quit nil)
 	    (case-fold-search isearch-case-fold-search)
+	    (search-invisible isearch-invisible)
 	    (retry t))
 	(setq isearch-error nil)
 	(while retry

bug#11378: 24.1.50; Suggestion: Let M-i in isearch cycle `search-invisible'

[Drew Adams]

> > I'll submit a new patch soon that adds a new variable
> > `isearch-search-invisible' (or maybe just `isearch-invisible')
> > to toggle its value without changing the value of the user option
> > `search-invisible'.
> 
> Here is a patch that adds `isearch-toggle-invisible' that
> toggles `isearch-invisible' between nil and a non-nil default value
> of `search-invisible', i.e.
> 
> if search-invisible is `open' then toggles isearch-invisible
> between `open' and `nil'
> 
> if search-invisible is `t' then toggles isearch-invisible
> between `t' and `nil'
> 
> if search-invisible is `nil' then toggles isearch-invisible
> between `nil' and `open'

This is similar to what I've been doing in isearch+.el for several
years now.  Glad it will be added to Emacs.

A major difference is that my code does not hesitate to use the
command to toggle the user option value.  I realize you consider
that a no-no.  I think that commands to toggle user options are
often a plus, not a minus.  (Think minor-mode variables, if that helps.
Or think of toggling an option via the options menu.)

isearch+.el is here, FWIW:
http://www.emacswiki.org/emacs-en/download/isearch%2b.el

bug#11378: 24.1.50; Suggestion: Let M-i in isearch cycle `search-invisible'

[Juri Linkov]

> A major difference is that my code does not hesitate to use the
> command to toggle the user option value.  I realize you consider
> that a no-no.  I think that commands to toggle user options are
> often a plus, not a minus.  (Think minor-mode variables, if that helps.
> Or think of toggling an option via the options menu.)

Most togglable search variables should preferably have two versions
anyway: the default value (persistent in case of `defcustom') used
initially for every new search (and often consulted in toggling
commands to get the initial default value) and a transient value
used in the currently active search.

This is what `case-fold-search' and `isearch-case-fold-search'
already do and this is a flexible approach.  I intend to
implement the same approach for other search variables as well,
e.g. adding `search-filter-predicates' to hold the default value
for `isearch-filter-predicates'.  The prefix `search-' is more
appropriate for such variables when they are used outside of isearch.
So `isearch-filter-predicates' could be used to incrementally
add/remove filters in isearch-mode, but to use `search-filter-predicates'
in `query-replace', etc.

bug#11378: 24.1.50; Suggestion: Let M-i in isearch cycle `search-invisible'

[Drew Adams]

> > A major difference is that my code does not hesitate to use the
> > command to toggle the user option value.  I realize you consider
> > that a no-no.  I think that commands to toggle user options are
> > often a plus, not a minus.  (Think minor-mode variables, if that helps.
> > Or think of toggling an option via the options menu.)
> 
> Most togglable search variables should preferably have two versions
> anyway: the default value (persistent in case of `defcustom') used
> initially for every new search (and often consulted in toggling
> commands to get the initial default value) and a transient value
> used in the currently active search.
> 
> This is what `case-fold-search' and `isearch-case-fold-search'
> already do and this is a flexible approach.  I intend to
> implement the same approach for other search variables as well,
> e.g. adding `search-filter-predicates' to hold the default value
> for `isearch-filter-predicates'.  The prefix `search-' is more
> appropriate for such variables when they are used outside of isearch.
> So `isearch-filter-predicates' could be used to incrementally
> add/remove filters in isearch-mode, but to use `search-filter-predicates'
> in `query-replace', etc.

I understand that approach.  I disagree, that's all.

I believe it is good to be able to toggle the option value (what you call
the default value) on the fly (during a search) in this case, and thus
change the behavior for subsequent searches as well (until toggled again).
That is a more useful UI, IMO, in this case and in many others.

I am not sure what I think about case-fold-search.  Generally I think the
usual approach makes more sense for it.  But there is an argument to be
made for it too to "stay put" across searches.  Partly, such "sticky"
behavior requires clear feedback to the user so s?he is aware of the current
"mode" of behavior.

It is simple enough to toggle back again.  Even for a one-off search using
a different "mode" from usual, it just means two toggles instead of one.
And if you perform multiple searches in different modes (a mix: some
case-sensitive, some not),  then the sticky/modal behavior typically means
less toggling.

I still use the usual approach for case-fold-search, because (a) I am
used to it and (b) I use case-insensitive search rarely.  But in Icicles
I have a similar toggle for completion, and it is sticky/modal.  Again,
good feedback wrt the current behavior is more important for the sticky
behavior: you are not always thrown back into some default behavior.

Ideally perhaps the choice of behavior itself would be up to the user,
and perhaps even itself via a toggle.  No, I don't mean ojust a single
option value reflecting the choice, but the ability to toggle which
behavior to use for any behavior choice (toggle or cycle).

(No, I don't really expect others to agree.)

bug#11378: 24.1.50; Suggestion: Let M-i in isearch cycle `search-invisible'

[Juri Linkov]

> I understand that approach.  I disagree, that's all.

It's no surprise that you disagree because both preferences make sense:

(1) Start each new search afresh with default values.

(2) Keep the current search state for subsequent searches.

There are more possible preferences:

(3) Someone even might prefer to keep the current search state
    within the confines of the current buffer (easy to do with
    `make-local-variable' on the transient search variables).

(4) Reuse the previous search values only for the repeated search
    `C-s C-s'.  This what the variable `isearch-last-case-fold-search'
    is used for.  But why `isearch-last-case-fold-search' only?
    Other search variables could be supported too.

This suggests two new customizable options: one to define which search
variables to keep for the next search to be able to choose the behavior
between (1) and (2) for every search variable, and another option for (4)
to choose which search variables to keep for the repeated search `C-s C-s'
(its default value could reuse the last case-fold only as it currently does).

bug#11378: 24.1.50; Suggestion: Let M-i in isearch cycle `search-invisible'

[Drew Adams]

> > I understand that approach.  I disagree, that's all.
> 
> It's no surprise that you disagree because both preferences make sense:

And we agree about that.

> (1) Start each new search afresh with default values.
> 
> (2) Keep the current search state for subsequent searches.
> 
> There are more possible preferences:
> 
> (3) Someone even might prefer to keep the current search state
>     within the confines of the current buffer (easy to do with
>     `make-local-variable' on the transient search variables).
> 
> (4) Reuse the previous search values only for the repeated search
>     `C-s C-s'.  This what the variable `isearch-last-case-fold-search'
>     is used for.  But why `isearch-last-case-fold-search' only?
>     Other search variables could be supported too.
> 
> This suggests two new customizable options: one to define which search
> variables to keep for the next search to be able to choose the behavior
> between (1) and (2) for every search variable, and another option for (4)
> to choose which search variables to keep for the repeated search `C-s C-s'
> (its default value could reuse the last case-fold only as it currently
> does).

As has often been the case, we think similarly (but differently too) here.
In my previous msg I mentioned a different way to combine both possibilities
(1: start with default vs 2: start from last)).  Not limited to Isearch,
and togglable between the two start-with behaviors.

When I get some time I'll throw something together and try it.  Maybe
let a (particular) prefix arg on the toggle command toggle also the current
start-with behavior.  E.g., a macro would define a user option and associated
variables, plus a toggle command usable for both start-with behaviors.

bug#11378: 24.1.50; Suggestion: Let M-i in isearch cycle `search-invisible'

[Juri Linkov]

> As has often been the case, we think similarly (but differently too) here.
> In my previous msg I mentioned a different way to combine both possibilities
> (1: start with default vs 2: start from last)).  Not limited to Isearch,
> and togglable between the two start-with behaviors.

Since a separate user option is necessary anyway, currently
I'm preparing a patch that adds it to customize the behavior of
case-fold and invisible.  Based on that, later you could provide
a prefix arg to toggle this user option, if you wish.

bug#11378: 24.1.50; Suggestion: Let M-i in isearch cycle `search-invisible'

[Juri Linkov]

> I'm preparing a patch that adds it to customize the behavior of
> case-fold and invisible.

This patch adds a new user option `isearch-keep-mode-variables'
in parallel to another new option `isearch-keep-stack-variables'
added in bug#12986.  It supports `isearch-case-fold-search' and
`isearch-invisible' variables, but `isearch-filter-predicates',
`isearch-lax-whitespace' could be added in the same way later.

=== modified file 'lisp/isearch.el'
--- lisp/isearch.el	2013-05-30 23:50:36 +0000
+++ lisp/isearch.el	2013-06-02 09:45:01 +0000
@@ -153,6 +153,20 @@ (defcustom isearch-hide-immediately t
   :type 'boolean
   :group 'isearch)
 
+(defcustom isearch-keep-mode-variables nil
+  "A set of search variables to keep between different searches.
+When a search variable is customized to exist in this set, then
+starting a new search doesn't reset the corresponding isearch variable
+to its default value, thus keeping the value from the previous search
+\(changed using toggling commands)."
+  :type '(set (const :tag "Case folding" isearch-case-fold-search)
+	      (const :tag "Invisible text" isearch-invisible)
+	      (const :tag "Filters" isearch-filter-predicates)
+	      (const :tag "Lax whitespace" isearch-lax-whitespace)
+	      (const :tag "Regexp lax whitespace" isearch-regexp-lax-whitespace))
+  :version "24.4"
+  :group 'isearch)
+
 (defcustom isearch-resume-in-command-history nil
   "If non-nil, `isearch-resume' commands are added to the command history.
 This allows you to resume earlier Isearch sessions through the
@@ -866,8 +895,6 @@ (defun isearch-mode (forward &optional r
 	isearch-word word
 	isearch-op-fun op-fun
 	isearch-last-case-fold-search isearch-case-fold-search
-	isearch-case-fold-search case-fold-search
-	isearch-invisible search-invisible
 	isearch-string ""
 	isearch-message ""
 	isearch-cmds nil
@@ -898,6 +927,11 @@ (defun isearch-mode (forward &optional r
 	isearch-original-minibuffer-message-timeout minibuffer-message-timeout
 	minibuffer-message-timeout nil)
 
+  (unless (memq 'isearch-case-fold-search isearch-keep-mode-variables)
+    (setq isearch-case-fold-search case-fold-search))
+  (unless (memq 'isearch-invisible isearch-keep-mode-variables)
+    (setq isearch-invisible search-invisible))
+
   ;; We must bypass input method while reading key.  When a user type
   ;; printable character, appropriate input method is turned on in
   ;; minibuffer to read multibyte characters.

bug#11378: 24.1.50; Suggestion: Let M-i in isearch cycle `search-invisible'

[Juri Linkov]

>>> I was thinking that maybe a cycling behavior would be better than
>>> toggling for these: off / foo-only / foo-excluded.
>>
>> Maybe something like this:
>
> Sounds about right, thank you,

Now that text proprieties on the search string can be saved in the desktop file,
I tried to save the isearch-filter-predicate with advice-functions, and
set it back to the search string, but this fails.

An example of the search string saved in the search ring in the desktop file:

#("string" 0 6 (isearch-filter-predicate #1=#[128 #2="\300\301\2\"\205\13\0\300\302\2\"\207"
              [apply isearch-filter-textual #[128 #3="\301\302\300!\2\"\207"
              [isearch-filter-predicate apply default-value] 4 #4="\n\n(fn &rest ARGS)"]
              #5=((isearch-message-prefix . "textual "))] 4 nil]))

After it's restored by desktop.el using `load' and set from the restored string
with (setq isearch-filter-predicate ...) it goes into an infinite loop:

Debugger entered--Lisp error: (error "Lisp nesting exceeds ‘max-lisp-eval-depth’")
  ...
  apply(#f(compiled-function (&rest args) #<bytecode 0x2958aa5>) (12237 12244))
  #f(advice-wrapper :before-while #f(compiled-function (&rest args) #<bytecode 0x2958aa5>) isearch-filter-nontextual ((isearch-message-prefix . "nontextual ")))(12237 12244)
  apply(#f(advice-wrapper :before-while #f(compiled-function
  #f(compiled-function (&rest args) #<bytecode 0x2958aa5>)(12237 12244)
  apply(#f(compiled-function (&rest args) #<bytecode 0x2958aa5>) (12237 12244))
  #f(advice-wrapper :before-while #f(compiled-function (&rest args) #<bytecode 0x2958aa5>) isearch-filter-nontextual ((isearch-message-prefix . "nontextual ")))(12237 12244)
  apply(#f(advice-wrapper :before-while #f(compiled-function
  #f(compiled-function (&rest args) #<bytecode 0x2958aa5>)(12237 12244)
  apply(#f(compiled-function (&rest args) #<bytecode 0x2958aa5>) (12237 12244))
  #f(advice-wrapper :before-while #f(compiled-function (&rest args) #<bytecode 0x2958aa5>) isearch-filter-nontextual ((isearch-message-prefix . "nontextual ")))(12237 12244)
  funcall(#f(advice-wrapper :before-while #f(compiled-function
  (not (funcall isearch-filter-predicate (nth 0 real-match-data) (nth 1 real-match-data)))

If it's impossible to restore its value, then maybe better to save only
function names, and on restoring explicitly call add-function with restored
function names as symbols?

A complete patch is here:

diff --git a/lisp/isearch.el b/lisp/isearch.el
index 5cbb4c9..70e3e44 100644
--- a/lisp/isearch.el
+++ b/lisp/isearch.el
@@ -1136,7 +1141,8 @@ isearch-update-ring
 (defun isearch-string-propertize (string &optional properties)
   "Add isearch properties to the isearch string."
   (unless properties
-    (setq properties `(isearch-case-fold-search ,isearch-case-fold-search))
+    (setq properties `(isearch-case-fold-search ,isearch-case-fold-search
+                       isearch-filter-predicate ,isearch-filter-predicate))
     (unless isearch-regexp
       (setq properties (append properties `(isearch-regexp-function ,isearch-regexp-function)))))
   (apply 'propertize string properties))
@@ -1146,6 +1152,9 @@ isearch-update-from-string-properties
   (when (plist-member (text-properties-at 0 string) 'isearch-case-fold-search)
     (setq isearch-case-fold-search
 	  (get-text-property 0 'isearch-case-fold-search string)))
+  (when (plist-member (text-properties-at 0 string) 'isearch-filter-predicate)
+    (setq isearch-filter-predicate
+	  (get-text-property 0 'isearch-filter-predicate string)))
   (when (plist-member (text-properties-at 0 string) 'isearch-regexp-function)
     (setq isearch-regexp-function
 	  (get-text-property 0 'isearch-regexp-function string))))
@@ -1640,6 +1648,38 @@ isearch--momentary-message
        "match invisible text"
      "match visible text")))
 
+(isearch-define-mode-toggle textual "ft" nil
+  "This determines whether to search inside or outside textual parts.
+Toggles the variable `isearch-filter-predicate' between three states:
+searching inside strings or comments, searching outside of strings or
+comments, the third state removes the filtering altogether."
+  (cond
+   ((advice-function-member-p #'isearch-filter-textual
+                              isearch-filter-predicate)
+    (remove-function (local 'isearch-filter-predicate)
+                     #'isearch-filter-textual)
+    (add-function :before-while (local 'isearch-filter-predicate)
+                  #'isearch-filter-nontextual
+                  '((isearch-message-prefix . "nontextual "))))
+   ((advice-function-member-p #'isearch-filter-nontextual
+                              isearch-filter-predicate)
+    (remove-function (local 'isearch-filter-predicate)
+                     #'isearch-filter-nontextual))
+   (t
+    (add-function :before-while (local 'isearch-filter-predicate)
+                  #'isearch-filter-textual
+                  '((isearch-message-prefix . "textual "))))))
+
+(defun isearch-filter-textual (_beg _end)
+  "Test whether the current search hit inside a string or comment."
+  (save-match-data
+    (or (nth 3 (syntax-ppss))
+        (nth 4 (syntax-ppss)))))
+
+(defun isearch-filter-nontextual (beg end)
+  "Test whether the current search hit outside a string or comment."
+  (not (isearch-filter-textual beg end)))
+
 \f
 ;; Word search
 
@@ -3179,6 +3219,7 @@ isearch-lazy-highlight-window-group
 (defvar isearch-lazy-highlight-window-start nil)
 (defvar isearch-lazy-highlight-window-end nil)
 (defvar isearch-lazy-highlight-case-fold-search nil)
+(defvar isearch-lazy-highlight-filter-predicate nil)
 (defvar isearch-lazy-highlight-regexp nil)
 (defvar isearch-lazy-highlight-lax-whitespace nil)
 (defvar isearch-lazy-highlight-regexp-lax-whitespace nil)
@@ -3218,6 +3259,8 @@ isearch-lazy-highlight-new-loop
                             isearch-lazy-highlight-window-group))
 		 (not (eq isearch-lazy-highlight-case-fold-search
 			  isearch-case-fold-search))
+		 (not (equal isearch-lazy-highlight-filter-predicate
+			     isearch-filter-predicate))
 		 (not (eq isearch-lazy-highlight-regexp
 			  isearch-regexp))
 		 (not (eq isearch-lazy-highlight-regexp-function
@@ -3259,6 +3302,7 @@ isearch-lazy-highlight-new-loop
 	  isearch-lazy-highlight-wrapped      nil
 	  isearch-lazy-highlight-last-string  isearch-string
 	  isearch-lazy-highlight-case-fold-search isearch-case-fold-search
+	  isearch-lazy-highlight-filter-predicate isearch-filter-predicate
 	  isearch-lazy-highlight-regexp       isearch-regexp
 	  isearch-lazy-highlight-lax-whitespace   isearch-lax-whitespace
 	  isearch-lazy-highlight-regexp-lax-whitespace isearch-regexp-lax-whitespace
diff --git a/lisp/replace.el b/lisp/replace.el
index 3503b65..2f297b1 100644
--- a/lisp/replace.el
+++ b/lisp/replace.el
@@ -2383,7 +2383,10 @@ perform-replace
          ;; If non-nil, it is marker saying where in the buffer to stop.
          (limit nil)
          ;; Use local binding in add-function below.
-         (isearch-filter-predicate isearch-filter-predicate)
+         (isearch-filter-predicate
+          (or (and (plist-member (text-properties-at 0 from-string) 'isearch-filter-predicate)
+                   (get-text-property 0 'isearch-filter-predicate from-string))
+              isearch-filter-predicate))
          (region-bounds nil)
 
          ;; Data for the next match.  If a cons, it has the same format as
@@ -2395,6 +2398,15 @@ perform-replace
               (apply 'propertize
                      (concat "Query replacing "
                              (if backward "backward " "")
+                             (if (not case-fold-search)
+                                 "case-sensitive ")
+                             (let ((prefix ""))
+                               (advice-function-mapc
+                                (lambda (_ props)
+                                  (let ((np (cdr (assq 'isearch-message-prefix props))))
+                                    (if np (setq prefix (concat np prefix)))))
+                                isearch-filter-predicate)
+                               prefix)
                              (if delimited-flag
                                  (or (and (symbolp delimited-flag)
                                           (get delimited-flag

bug#11378: 24.1.50; Suggestion: Let M-i in isearch cycle `search-invisible'

[Stefan Kangas]

Juri Linkov <juri@jurta.org> writes:

>> I'm preparing a patch that adds it to customize the behavior of
>> case-fold and invisible.
>
> This patch adds a new user option `isearch-keep-mode-variables'
> in parallel to another new option `isearch-keep-stack-variables'
> added in bug#12986.  It supports `isearch-case-fold-search' and
> `isearch-invisible' variables, but `isearch-filter-predicates',
> `isearch-lax-whitespace' could be added in the same way later.

The below patch from 6 years ago was sent but never applied at the
time.  Is it still relevant?

Best regards,
Stefan Kangas


> === modified file 'lisp/isearch.el'
> --- lisp/isearch.el	2013-05-30 23:50:36 +0000
> +++ lisp/isearch.el	2013-06-02 09:45:01 +0000
> @@ -153,6 +153,20 @@ (defcustom isearch-hide-immediately t
>    :type 'boolean
>    :group 'isearch)
>  
> +(defcustom isearch-keep-mode-variables nil
> +  "A set of search variables to keep between different searches.
> +When a search variable is customized to exist in this set, then
> +starting a new search doesn't reset the corresponding isearch variable
> +to its default value, thus keeping the value from the previous search
> +\(changed using toggling commands)."
> +  :type '(set (const :tag "Case folding" isearch-case-fold-search)
> +	      (const :tag "Invisible text" isearch-invisible)
> +	      (const :tag "Filters" isearch-filter-predicates)
> +	      (const :tag "Lax whitespace" isearch-lax-whitespace)
> +	      (const :tag "Regexp lax whitespace" isearch-regexp-lax-whitespace))
> +  :version "24.4"
> +  :group 'isearch)
> +
>  (defcustom isearch-resume-in-command-history nil
>    "If non-nil, `isearch-resume' commands are added to the command history.
>  This allows you to resume earlier Isearch sessions through the
> @@ -866,8 +895,6 @@ (defun isearch-mode (forward &optional r
>  	isearch-word word
>  	isearch-op-fun op-fun
>  	isearch-last-case-fold-search isearch-case-fold-search
> -	isearch-case-fold-search case-fold-search
> -	isearch-invisible search-invisible
>  	isearch-string ""
>  	isearch-message ""
>  	isearch-cmds nil
> @@ -898,6 +927,11 @@ (defun isearch-mode (forward &optional r
>  	isearch-original-minibuffer-message-timeout minibuffer-message-timeout
>  	minibuffer-message-timeout nil)
>  
> +  (unless (memq 'isearch-case-fold-search isearch-keep-mode-variables)
> +    (setq isearch-case-fold-search case-fold-search))
> +  (unless (memq 'isearch-invisible isearch-keep-mode-variables)
> +    (setq isearch-invisible search-invisible))
> +
>    ;; We must bypass input method while reading key.  When a user type
>    ;; printable character, appropriate input method is turned on in
>    ;; minibuffer to read multibyte characters.

bug#11378: 24.1.50; Suggestion: Let M-i in isearch cycle `search-invisible'

[Stefan Kangas]

Juri Linkov <juri@jurta.org> writes:

>>>> I was thinking that maybe a cycling behavior would be better than
>>>> toggling for these: off / foo-only / foo-excluded.
>>>
>>> Maybe something like this:
>>
>> Sounds about right, thank you,
>
> Now that text proprieties on the search string can be saved in the desktop file,
> I tried to save the isearch-filter-predicate with advice-functions, and
> set it back to the search string, but this fails.
>
> An example of the search string saved in the search ring in the desktop file:
>
> #("string" 0 6 (isearch-filter-predicate #1=#[128 #2="\300\301\2\"\205\13\0\300\302\2\"\207"
>               [apply isearch-filter-textual #[128 #3="\301\302\300!\2\"\207"
>               [isearch-filter-predicate apply default-value] 4 #4="\n\n(fn &rest ARGS)"]
>               #5=((isearch-message-prefix . "textual "))] 4 nil]))
>
> After it's restored by desktop.el using `load' and set from the restored string
> with (setq isearch-filter-predicate ...) it goes into an infinite loop:
>
> Debugger entered--Lisp error: (error "Lisp nesting exceeds ‘max-lisp-eval-depth’")
>   ...
>   apply(#f(compiled-function (&rest args) #<bytecode 0x2958aa5>) (12237 12244))
>   #f(advice-wrapper :before-while #f(compiled-function (&rest args) #<bytecode 0x2958aa5>) isearch-filter-nontextual ((isearch-message-prefix . "nontextual ")))(12237 12244)
>   apply(#f(advice-wrapper :before-while #f(compiled-function
>   #f(compiled-function (&rest args) #<bytecode 0x2958aa5>)(12237 12244)
>   apply(#f(compiled-function (&rest args) #<bytecode 0x2958aa5>) (12237 12244))
>   #f(advice-wrapper :before-while #f(compiled-function (&rest args) #<bytecode 0x2958aa5>) isearch-filter-nontextual ((isearch-message-prefix . "nontextual ")))(12237 12244)
>   apply(#f(advice-wrapper :before-while #f(compiled-function
>   #f(compiled-function (&rest args) #<bytecode 0x2958aa5>)(12237 12244)
>   apply(#f(compiled-function (&rest args) #<bytecode 0x2958aa5>) (12237 12244))
>   #f(advice-wrapper :before-while #f(compiled-function (&rest args) #<bytecode 0x2958aa5>) isearch-filter-nontextual ((isearch-message-prefix . "nontextual ")))(12237 12244)
>   funcall(#f(advice-wrapper :before-while #f(compiled-function
>   (not (funcall isearch-filter-predicate (nth 0 real-match-data) (nth 1 real-match-data)))
>
> If it's impossible to restore its value, then maybe better to save only
> function names, and on restoring explicitly call add-function with restored
> function names as symbols?

Was the below patch ever applied?  Is it still relevant?

Best regards,
Stefan Kangas


> A complete patch is here:
>
> diff --git a/lisp/isearch.el b/lisp/isearch.el
> index 5cbb4c9..70e3e44 100644
> --- a/lisp/isearch.el
> +++ b/lisp/isearch.el
> @@ -1136,7 +1141,8 @@ isearch-update-ring
>  (defun isearch-string-propertize (string &optional properties)
>    "Add isearch properties to the isearch string."
>    (unless properties
> -    (setq properties `(isearch-case-fold-search ,isearch-case-fold-search))
> +    (setq properties `(isearch-case-fold-search ,isearch-case-fold-search
> +                       isearch-filter-predicate ,isearch-filter-predicate))
>      (unless isearch-regexp
>        (setq properties (append properties `(isearch-regexp-function ,isearch-regexp-function)))))
>    (apply 'propertize string properties))
> @@ -1146,6 +1152,9 @@ isearch-update-from-string-properties
>    (when (plist-member (text-properties-at 0 string) 'isearch-case-fold-search)
>      (setq isearch-case-fold-search
>  	  (get-text-property 0 'isearch-case-fold-search string)))
> +  (when (plist-member (text-properties-at 0 string) 'isearch-filter-predicate)
> +    (setq isearch-filter-predicate
> +	  (get-text-property 0 'isearch-filter-predicate string)))
>    (when (plist-member (text-properties-at 0 string) 'isearch-regexp-function)
>      (setq isearch-regexp-function
>  	  (get-text-property 0 'isearch-regexp-function string))))
> @@ -1640,6 +1648,38 @@ isearch--momentary-message
>         "match invisible text"
>       "match visible text")))
>  
> +(isearch-define-mode-toggle textual "ft" nil
> +  "This determines whether to search inside or outside textual parts.
> +Toggles the variable `isearch-filter-predicate' between three states:
> +searching inside strings or comments, searching outside of strings or
> +comments, the third state removes the filtering altogether."
> +  (cond
> +   ((advice-function-member-p #'isearch-filter-textual
> +                              isearch-filter-predicate)
> +    (remove-function (local 'isearch-filter-predicate)
> +                     #'isearch-filter-textual)
> +    (add-function :before-while (local 'isearch-filter-predicate)
> +                  #'isearch-filter-nontextual
> +                  '((isearch-message-prefix . "nontextual "))))
> +   ((advice-function-member-p #'isearch-filter-nontextual
> +                              isearch-filter-predicate)
> +    (remove-function (local 'isearch-filter-predicate)
> +                     #'isearch-filter-nontextual))
> +   (t
> +    (add-function :before-while (local 'isearch-filter-predicate)
> +                  #'isearch-filter-textual
> +                  '((isearch-message-prefix . "textual "))))))
> +
> +(defun isearch-filter-textual (_beg _end)
> +  "Test whether the current search hit inside a string or comment."
> +  (save-match-data
> +    (or (nth 3 (syntax-ppss))
> +        (nth 4 (syntax-ppss)))))
> +
> +(defun isearch-filter-nontextual (beg end)
> +  "Test whether the current search hit outside a string or comment."
> +  (not (isearch-filter-textual beg end)))
> +
>  \f
>  ;; Word search
>  
> @@ -3179,6 +3219,7 @@ isearch-lazy-highlight-window-group
>  (defvar isearch-lazy-highlight-window-start nil)
>  (defvar isearch-lazy-highlight-window-end nil)
>  (defvar isearch-lazy-highlight-case-fold-search nil)
> +(defvar isearch-lazy-highlight-filter-predicate nil)
>  (defvar isearch-lazy-highlight-regexp nil)
>  (defvar isearch-lazy-highlight-lax-whitespace nil)
>  (defvar isearch-lazy-highlight-regexp-lax-whitespace nil)
> @@ -3218,6 +3259,8 @@ isearch-lazy-highlight-new-loop
>                              isearch-lazy-highlight-window-group))
>  		 (not (eq isearch-lazy-highlight-case-fold-search
>  			  isearch-case-fold-search))
> +		 (not (equal isearch-lazy-highlight-filter-predicate
> +			     isearch-filter-predicate))
>  		 (not (eq isearch-lazy-highlight-regexp
>  			  isearch-regexp))
>  		 (not (eq isearch-lazy-highlight-regexp-function
> @@ -3259,6 +3302,7 @@ isearch-lazy-highlight-new-loop
>  	  isearch-lazy-highlight-wrapped      nil
>  	  isearch-lazy-highlight-last-string  isearch-string
>  	  isearch-lazy-highlight-case-fold-search isearch-case-fold-search
> +	  isearch-lazy-highlight-filter-predicate isearch-filter-predicate
>  	  isearch-lazy-highlight-regexp       isearch-regexp
>  	  isearch-lazy-highlight-lax-whitespace   isearch-lax-whitespace
>  	  isearch-lazy-highlight-regexp-lax-whitespace isearch-regexp-lax-whitespace
> diff --git a/lisp/replace.el b/lisp/replace.el
> index 3503b65..2f297b1 100644
> --- a/lisp/replace.el
> +++ b/lisp/replace.el
> @@ -2383,7 +2383,10 @@ perform-replace
>           ;; If non-nil, it is marker saying where in the buffer to stop.
>           (limit nil)
>           ;; Use local binding in add-function below.
> -         (isearch-filter-predicate isearch-filter-predicate)
> +         (isearch-filter-predicate
> +          (or (and (plist-member (text-properties-at 0 from-string) 'isearch-filter-predicate)
> +                   (get-text-property 0 'isearch-filter-predicate from-string))
> +              isearch-filter-predicate))
>           (region-bounds nil)
>  
>           ;; Data for the next match.  If a cons, it has the same format as
> @@ -2395,6 +2398,15 @@ perform-replace
>                (apply 'propertize
>                       (concat "Query replacing "
>                               (if backward "backward " "")
> +                             (if (not case-fold-search)
> +                                 "case-sensitive ")
> +                             (let ((prefix ""))
> +                               (advice-function-mapc
> +                                (lambda (_ props)
> +                                  (let ((np (cdr (assq 'isearch-message-prefix props))))
> +                                    (if np (setq prefix (concat np prefix)))))
> +                                isearch-filter-predicate)
> +                               prefix)
>                               (if delimited-flag
>                                   (or (and (symbolp delimited-flag)
>                                            (get delimited-flag

bug#11378: 24.1.50; Suggestion: Let M-i in isearch cycle `search-invisible'

[Michael Heerdegen]

Stefan Kangas <stefan@marxist.se> writes:

> Was the below patch ever applied?  Is it still relevant?

I don't know.

At least my original suggestion seems to have been implemented now: M-s
i toggles searching inside invisible text (see the line

  (isearch-define-mode-toggle invisible "i" nil

and following in "isearch.el").


Michael.