23.0.60; isearch for M-: incorrect in Info

23.0.60; isearch for M-: incorrect in Info

[Drew Adams]

emacs -Q
Go to node Glossary in Emacs manual.
C-s M-:
 
There are false hits here. It mistakenly finds M- and C-M- when they
are the text of links.
 

In GNU Emacs 23.0.60.1 (i386-mingw-nt5.1.2600)
 of 2008-04-04 on LENNART-69DE564
Windowing system distributor `Microsoft Corp.', version 5.1.2600
configured using `configure --with-gcc (3.4) --no-opt --cflags -Ic:/g/include
-fno-crossjumping'

Re: 23.0.60; isearch for M-: incorrect in Info

[Juri Linkov]

> emacs -Q
> Go to node Glossary in Emacs manual.
> C-s M-:
>
> There are false hits here. It mistakenly finds M- and C-M- when they
> are the text of links.

When you type `C-s M - :' isearch finds parts of Info cross references

     Control-Meta.  *Note C-M-: User Input.

that are displayed as

     Control-Meta.  See C-M-.

Since this text is not visible for the user, I think we should hide it
from isearch as well.

-- 
Juri Linkov
http://www.jurta.org/emacs/

RE: 23.0.60; isearch for M-: incorrect in Info

[Drew Adams]

> > emacs -Q
> > Go to node Glossary in Emacs manual.
> > C-s M-:
> > There are false hits here. It mistakenly finds M- and C-M- when they
> > are the text of links.
> 
> When you type `C-s M - :' isearch finds parts of Info cross references
> 
>      Control-Meta.  *Note C-M-: User Input.
> 
> that are displayed as
> 
>      Control-Meta.  See C-M-.
> 
> Since this text is not visible for the user, I think we should hide it
> from isearch as well.

Exactly.

Re: 23.0.60; isearch for M-: incorrect in Info

[Juri Linkov]

>> > emacs -Q
>> > Go to node Glossary in Emacs manual.
>> > C-s M-:
>> > There are false hits here. It mistakenly finds M- and C-M- when they
>> > are the text of links.
>>
>> When you type `C-s M - :' isearch finds parts of Info cross references
>>
>>      Control-Meta.  *Note C-M-: User Input.
>>
>> that are displayed as
>>
>>      Control-Meta.  See C-M-.
>>
>> Since this text is not visible for the user, I think we should hide it
>> from isearch as well.
>
> Exactly.

This bug can be fixed with a simple patch that was postponed to install
after the 22.1 release.  It seems now is the right time to install it:

http://thread.gmane.org/gmane.emacs.devel/34884/focus=41845

-- 
Juri Linkov
http://www.jurta.org/emacs/

Re: 23.0.60; isearch for M-: incorrect in Info

[Stefan Monnier]

> This bug can be fixed with a simple patch that was postponed to install
> after the 22.1 release.  It seems now is the right time to install it:

> http://thread.gmane.org/gmane.emacs.devel/34884/focus=41845

Could you show us the patch?


        Stefan

Re: 23.0.60; isearch for M-: incorrect in Info

[Juri Linkov]

>> This bug can be fixed with a simple patch that was postponed to install
>> after the 22.1 release.  It seems now is the right time to install it:
>
>> http://thread.gmane.org/gmane.emacs.devel/34884/focus=41845
>
> Could you show us the patch?

The patch is below.  It adds a new predicate variable
`isearch-success-function' with the default behavior of
using `isearch-range-invisible' if it is nil.

info.el overrides it to let isearch skip invisible text, node header line
and Tag Table node.  It can't reuse `isearch-range-invisible' because
`isearch-range-invisible' doesn't skip the string if at least one its
character is visible.  But in Info the reverse is necessary (according
to Drew's bug report): skip the string when at least one its character
is invisible.

Index: lisp/isearch.el
===================================================================
RCS file: /sources/emacs/emacs/lisp/isearch.el,v
retrieving revision 1.316
diff -c -r1.316 isearch.el
*** lisp/isearch.el	18 Apr 2008 10:29:23 -0000	1.316
--- lisp/isearch.el	19 Apr 2008 22:45:01 -0000
***************
*** 178,183 ****
--- 178,188 ----
    "Function to save a function restoring the mode-specific isearch state
  to the search status stack.")
  
+ (defvar isearch-success-function nil
+   "Function to report whether the new search match is considered successful.
+ The function has two arguments: the positions of start and end of text
+ matched by search.")
+ 
  ;; Search ring.
  
  (defvar search-ring nil
***************
*** 2104,2110 ****
        (setq isearch-case-fold-search
  	    (isearch-no-upper-case-p isearch-string isearch-regexp)))
    (condition-case lossage
!       (let ((inhibit-point-motion-hooks search-invisible)
  	    (inhibit-quit nil)
  	    (case-fold-search isearch-case-fold-search)
  	    (search-spaces-regexp search-whitespace-regexp)
--- 2176,2183 ----
        (setq isearch-case-fold-search
  	    (isearch-no-upper-case-p isearch-string isearch-regexp)))
    (condition-case lossage
!       (let ((inhibit-point-motion-hooks
! 	     (and (not isearch-success-function) search-invisible))
  	    (inhibit-quit nil)
  	    (case-fold-search isearch-case-fold-search)
  	    (search-spaces-regexp search-whitespace-regexp)
***************
*** 2115,2126 ****
  		(isearch-search-string isearch-string nil t))
  	  ;; Clear RETRY unless we matched some invisible text
  	  ;; and we aren't supposed to do that.
! 	  (if (or (eq search-invisible t)
! 		  (not isearch-success)
  		  (bobp) (eobp)
  		  (= (match-beginning 0) (match-end 0))
! 		  (not (isearch-range-invisible
! 			(match-beginning 0) (match-end 0))))
  	      (setq retry nil)))
  	(setq isearch-just-started nil)
  	(if isearch-success
--- 2188,2202 ----
  		(isearch-search-string isearch-string nil t))
  	  ;; Clear RETRY unless we matched some invisible text
  	  ;; and we aren't supposed to do that.
! 	  (if (or (not isearch-success)
  		  (bobp) (eobp)
  		  (= (match-beginning 0) (match-end 0))
! 		  (if isearch-success-function
! 		      (funcall isearch-success-function
! 			       (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

Index: lisp/info.el
===================================================================
RCS file: /sources/emacs/emacs/lisp/info.el,v
retrieving revision 1.527
diff -c -r1.527 info.el
*** lisp/info.el	19 Apr 2008 03:33:13 -0000	1.527
--- lisp/info.el	19 Apr 2008 22:45:02 -0000
***************
*** 1625,1644 ****
  				(1- (point)))
  			      (point-max)))
  	  (while (and (not give-up)
! 		      (save-match-data
! 			(or (null found)
! 			    (if backward
! 				(isearch-range-invisible found beg-found)
! 			      (isearch-range-invisible beg-found found))
! 			    ;; Skip node header line
! 			    (and (save-excursion (forward-line -1)
! 						 (looking-at "\^_"))
! 				 (forward-line (if backward -1 1)))
! 			    ;; Skip Tag Table node
! 			    (save-excursion
! 			      (and (search-backward "\^_" nil t)
! 				   (looking-at
! 				    "\^_\n\\(Tag Table\\|Local Variables\\)"))))))
  	    (let ((search-spaces-regexp
  		   (if (or (not isearch-mode) isearch-regexp)
  		       Info-search-whitespace-regexp)))
--- 1628,1635 ----
  				(1- (point)))
  			      (point-max)))
  	  (while (and (not give-up)
! 		      (or (null found)
! 			  (not (funcall isearch-success-function beg-found found))))
  	    (let ((search-spaces-regexp
  		   (if (or (not isearch-mode) isearch-regexp)
  		       Info-search-whitespace-regexp)))
***************
*** 1717,1736 ****
  		(setq list (cdr list))
  		(setq give-up nil found nil)
  		(while (and (not give-up)
! 			    (save-match-data
! 			      (or (null found)
! 				  (if backward
! 				      (isearch-range-invisible found beg-found)
! 				    (isearch-range-invisible beg-found found))
! 				  ;; Skip node header line
! 				  (and (save-excursion (forward-line -1)
! 						       (looking-at "\^_"))
! 				       (forward-line (if backward -1 1)))
! 				  ;; Skip Tag Table node
! 				  (save-excursion
! 				    (and (search-backward "\^_" nil t)
! 					 (looking-at
! 					  "\^_\n\\(Tag Table\\|Local Variables\\)"))))))
  		  (let ((search-spaces-regexp
  			 (if (or (not isearch-mode) isearch-regexp)
  			     Info-search-whitespace-regexp)))
--- 1708,1715 ----
  		(setq list (cdr list))
  		(setq give-up nil found nil)
  		(while (and (not give-up)
! 			    (or (null found)
! 				(not (funcall isearch-success-function beg-found found))))
  		  (let ((search-spaces-regexp
  			 (if (or (not isearch-mode) isearch-regexp)
  			     Info-search-whitespace-regexp)))
***************
*** 1836,1841 ****
--- 1815,1842 ----
  
  (defun Info-isearch-start ()
    (setq Info-isearch-initial-node nil))
+ 
+ (defun Info-search-success-function (beg-found found)
+   "Skip invisible text, node header line and Tag Table node."
+   (save-match-data
+     (let ((backward (< found beg-found)))
+       (not
+        (or
+ 	(if backward
+ 	    (or (text-property-not-all found beg-found 'invisible nil)
+ 		(text-property-not-all found beg-found 'display nil))
+ 	  (or (text-property-not-all beg-found found 'invisible nil)
+ 	      (text-property-not-all beg-found found 'display nil)))
+ 	;; Skip node header line
+ 	(and (save-excursion (forward-line -1)
+ 			     (looking-at "\^_"))
+ 	     (forward-line (if backward -1 1)))
+ 	;; Skip Tag Table node
+ 	(save-excursion
+ 	  (and (search-backward "\^_" nil t)
+ 	       (looking-at
+ 		"\^_\n\\(Tag Table\\|Local Variables\\)"))))))))
+ 
  \f
  (defun Info-extract-pointer (name &optional errorname)
    "Extract the value of the node-pointer named NAME.
***************
*** 3457,3462 ****
--- 3458,3465 ----
         'Info-isearch-wrap)
    (set (make-local-variable 'isearch-push-state-function)
         'Info-isearch-push-state)
+   (set (make-local-variable 'isearch-success-function)
+        'Info-search-success-function)
    (set (make-local-variable 'search-whitespace-regexp)
         Info-search-whitespace-regexp)
    (set (make-local-variable 'revert-buffer-function)

-- 
Juri Linkov
http://www.jurta.org/emacs/

Re: 23.0.60; isearch for M-: incorrect in Info

[Stefan Monnier]

> + (defvar isearch-success-function nil
[...]
> ! 		  (if isearch-success-function
> ! 		      (funcall isearch-success-function
> ! 			       (match-beginning 0) (match-end 0))
> ! 		    (or (eq search-invisible t)
> ! 			(not (isearch-range-invisible
> ! 			      (match-beginning 0) (match-end 0))))))

Better just call isearch-success-function unconditionally and move the
default code to the default value of isearch-success-function.

I understand that may require many more changes to your code, tho, but
I think it's important to always make when you introduce such
a foo-function variable to make sure that it is possible to reproduce
the default value (since it's often desirable to do almost the same as
the default).


        Stefan

Re: 23.0.60; isearch for M-: incorrect in Info

[Juri Linkov]

>> + (defvar isearch-success-function nil
> [...]
>> ! 		  (if isearch-success-function
>> ! 		      (funcall isearch-success-function
>> ! 			       (match-beginning 0) (match-end 0))
>> ! 		    (or (eq search-invisible t)
>> ! 			(not (isearch-range-invisible
>> ! 			      (match-beginning 0) (match-end 0))))))
>
> Better just call isearch-success-function unconditionally and move the
> default code to the default value of isearch-success-function.
>
> I understand that may require many more changes to your code, tho, but
> I think it's important to always make when you introduce such
> a foo-function variable to make sure that it is possible to reproduce
> the default value (since it's often desirable to do almost the same as
> the default).

I can't invent a good name for the default function that describes exactly
what it does, so I named it simply `isearch-success-function-default'.
I've seen this naming convention in other places too.

Index: lisp/isearch.el
===================================================================
RCS file: /sources/emacs/emacs/lisp/isearch.el,v
retrieving revision 1.316
diff -c -r1.316 isearch.el
*** lisp/isearch.el	18 Apr 2008 10:29:23 -0000	1.316
--- lisp/isearch.el	20 Apr 2008 23:51:28 -0000
***************
*** 178,183 ****
--- 178,188 ----
    "Function to save a function restoring the mode-specific isearch state
  to the search status stack.")
  
+ (defvar isearch-success-function 'isearch-success-function-default
+   "Function to report whether the new search match is considered successful.
+ The function has two arguments: the positions of start and end of text
+ matched by search.")
+ 
  ;; Search ring.
  
  (defvar search-ring nil
***************
*** 2104,2110 ****
        (setq isearch-case-fold-search
  	    (isearch-no-upper-case-p isearch-string isearch-regexp)))
    (condition-case lossage
!       (let ((inhibit-point-motion-hooks search-invisible)
  	    (inhibit-quit nil)
  	    (case-fold-search isearch-case-fold-search)
  	    (search-spaces-regexp search-whitespace-regexp)
--- 2174,2182 ----
        (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-success-function 'isearch-success-function-default)
! 		  search-invisible))
  	    (inhibit-quit nil)
  	    (case-fold-search isearch-case-fold-search)
  	    (search-spaces-regexp search-whitespace-regexp)
***************
*** 2115,2126 ****
  		(isearch-search-string isearch-string nil t))
  	  ;; Clear RETRY unless we matched some invisible text
  	  ;; and we aren't supposed to do that.
! 	  (if (or (eq search-invisible t)
! 		  (not isearch-success)
  		  (bobp) (eobp)
  		  (= (match-beginning 0) (match-end 0))
! 		  (not (isearch-range-invisible
! 			(match-beginning 0) (match-end 0))))
  	      (setq retry nil)))
  	(setq isearch-just-started nil)
  	(if isearch-success
--- 2187,2197 ----
  		(isearch-search-string isearch-string nil t))
  	  ;; Clear RETRY unless we matched some invisible text
  	  ;; and we aren't supposed to do that.
! 	  (if (or (not isearch-success)
  		  (bobp) (eobp)
  		  (= (match-beginning 0) (match-end 0))
! 		  (funcall isearch-success-function
! 			   (match-beginning 0) (match-end 0)))
  	      (setq retry nil)))
  	(setq isearch-just-started nil)
  	(if isearch-success
***************
*** 2298,2303 ****
--- 2369,2381 ----
  		  nil)
  	      (setq isearch-hidden t)))))))
  
+ (defun isearch-success-function-default (beg end)
+   "Default function to report if the new search match is successful.
+ Returns t if search can match hidden text, or otherwise checks if some
+ text from BEG to END is visible."
+   (or (eq search-invisible t)
+       (not (isearch-range-invisible beg end))))
+ 
  \f
  ;; General utilities
  
-- 
Juri Linkov
http://www.jurta.org/emacs/

Re: 23.0.60; isearch for M-: incorrect in Info

[Stefan Monnier]

> I can't invent a good name for the default function that describes exactly
> what it does, so I named it simply `isearch-success-function-default'.
> I've seen this naming convention in other places too.

Looks good, please install,


        Stefan