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

From: Stefan Monnier <[email-masked]>
Date: Tue, 29 May 2012 14:22:27 -0400

> 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