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

[Juri Linkov <juri@jurta.org>]

> 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)))