flyspell.el [1.90->1.91] flyspell-large-region-beg should be moved after good match

flyspell.el [1.90->1.91] flyspell-large-region-beg should be moved after good match

[Agustin Martin]

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

Hi,

Just noticed commit of flyspell.el 1.91 and a problem in it. In function
flyspell-external-point-words flyspell-large-region-beg is not moved
after successful match. That makes that for buffers like 

ispella
ispella
ispella

only first match is found and tagged as misspelled since all further
searches start from the same point.

I am attaching a possible patch for that,

-- 
Agustin

[-- Attachment #2: flyspell.el.fly-ext-poi-words.diff --]
[-- Type: text/plain, Size: 816 bytes --]

--- flyspell.el.orig	2005-12-16 12:10:52.000000000 +0100
+++ flyspell.el	2005-12-16 13:18:59.000000000 +0100
@@ -1334,7 +1334,8 @@
 				    flyspell-large-region-end t)
 		    (save-excursion
 		      (goto-char (- (point) 1))
-		      (let* ((flyword-prev-l (flyspell-get-word nil))
+		      (let* ((match-point (+ (point) 1))
+			     (flyword-prev-l (flyspell-get-word nil))
 			     (flyword-prev (car flyword-prev-l))
 			     (size-match (= (length flyword-prev) (length word))))
 			(when (or
@@ -1361,7 +1362,7 @@
 			  (setq keep nil)
 			  (flyspell-word)
 			  ;; Next search will begin from end of last match
-			  )))
+			  (setq flyspell-large-region-beg match-point))))
 		  ;; Record if misspelling is not found and try new one
 		  (add-to-list 'words-not-found
 			       (concat " -> " word " - "

[-- Attachment #3: Type: text/plain, Size: 142 bytes --]

_______________________________________________
Emacs-devel mailing list
Emacs-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/emacs-devel

Re: flyspell.el [1.90->1.91] flyspell-large-region-beg should be moved after good match

[Richard M. Stallman]

I think this fix is a cleaner one.  Does it work right?

*** flyspell.el	04 Dec 2005 12:42:38 -0500	1.91
--- flyspell.el	16 Dec 2005 19:58:07 -0500	
***************
*** 1311,1316 ****
--- 1311,1318 ----
  	(ispell-otherchars (ispell-get-otherchars)))
      (with-current-buffer flyspell-external-ispell-buffer
        (goto-char (point-min))
+       (with-current-buffer flyspell-large-region-buffer
+ 	(goto-char flyspell-large-region-beg))
        ;; Loop over incorrect words.
        (while (re-search-forward "\\([^\n]+\\)\n" (point-max) t)
  	;; Bind WORD to the next one.
***************
*** 1325,1331 ****
  		       (* 100 (/ (float (point)) (point-max)))
  		       word))
  	  (with-current-buffer flyspell-large-region-buffer
- 	    (goto-char flyspell-large-region-beg)
  	    (let ((keep t))
  	      ;; Iterate on string search until string is found as word,
  	      ;; not as substring
--- 1327,1332 ----

Re: flyspell.el [1.90->1.91] flyspell-large-region-beg should be moved after good match

[Agustin Martin]

On Fri, Dec 16, 2005 at 08:03:25PM -0500, Richard M. Stallman wrote:
> I think this fix is a cleaner one.  Does it work right?
> 
> *** flyspell.el	04 Dec 2005 12:42:38 -0500	1.91
> --- flyspell.el	16 Dec 2005 19:58:07 -0500	
> ***************
> *** 1311,1316 ****
> --- 1311,1318 ----
>   	(ispell-otherchars (ispell-get-otherchars)))
>       (with-current-buffer flyspell-external-ispell-buffer
>         (goto-char (point-min))
> +       (with-current-buffer flyspell-large-region-buffer
> + 	(goto-char flyspell-large-region-beg))
>         ;; Loop over incorrect words.
>         (while (re-search-forward "\\([^\n]+\\)\n" (point-max) t)
>   	;; Bind WORD to the next one.


I have tested that change in Debian sarge emacs21.4 with the french
Hydro Quebec ispell dict and the same french ispell dictionary
(with the old boundary-chars mistmatches) and that seems to cause
problems with forward jumps over regions.

These problems are probably related to the boundary-chars mismatch,
but since this can appear from user .emacs files in a rather obscure
mode I would rather leave patch the other way.

I will try testing this in a current emacs when I can. I still do not
understand where and why is the jump produced in that file, but as mentioned
above seems related to words tagged as not found because contain chars not
in casechars or boundary-chars, but actually found in the search loop thus
moving point.

-- 
Agustin

Re: flyspell.el [1.90->1.91] flyspell-large-region-beg should be moved after good match

[Agustin Martin]

On Mon, Dec 19, 2005 at 01:41:15AM +0100, Agustin Martin wrote:
> On Fri, Dec 16, 2005 at 08:03:25PM -0500, Richard M. Stallman wrote:
> > I think this fix is a cleaner one.  Does it work right?
> I will try testing this in a current emacs when I can. I still do not
> understand where and why is the jump produced in that file, but as mentioned
> above seems related to words tagged as not found because contain chars not
> in casechars or boundary-chars, but actually found in the search loop thus
> moving point.

Tested a bit more, and indeed related to boundary chars mismatch. I am
considering a better approach for this kind of mismatches, but is still
untested.

For it I think we can do better starting from your last committed
changes (1.91), so please do *not* apply my patch. I will send a new
one once I have tested things better.

-- 
Agustin

Re: flyspell.el [1.90->1.91] flyspell-large-region-beg should be moved after good match

[Agustin Martin]

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

On Mon, Dec 19, 2005 at 02:28:17AM +0100, Agustin Martin wrote:
> On Mon, Dec 19, 2005 at 01:41:15AM +0100, Agustin Martin wrote:
> > On Fri, Dec 16, 2005 at 08:03:25PM -0500, Richard M. Stallman wrote:
> > > I think this fix is a cleaner one.  Does it work right?
> > I will try testing this in a current emacs when I can. I still do not
> > understand where and why is the jump produced in that file, but as mentioned
> > above seems related to words tagged as not found because contain chars not
> > in casechars or boundary-chars, but actually found in the search loop thus
> > moving point.
> 
> Tested a bit more, and indeed related to boundary chars mismatch. I am
> considering a better approach for this kind of mismatches, but is still
> untested.

Testing things in emacs-snapshot, so we get real up-to-date results.

The matter here is if search for next misspelling should start where
searches involving last misspell ended or only where last validated match
was. I am strongly in favour of the second option and that is what my patch
proposed. The reason for this is that boundary-char mismatches between
ispell and ispell.el are a potential source for problems that are not easy
to debug. Having updated ispell-dictionary-alist entries is not enough,
because this is still very subjected to user settings in the ~/.emacs file
or to changes in the ispell aff file (this last should not be a problem for
aspell, where ispell.el detects boundary-chars, but users settings might
still be).

Since this is hard to debug for a normal user I am strongly in favour of
the conservative option, moving search start only after a validated match.

I was testing previous flyspell.el version with your last non-installed patch
in a test-system where dot is declared as boundary char in ispell francais
dict but not in ispell.el (this is fixed in current ispell.el, but some
~/.emacs might put it wrong) with the contents

francais.aff
anothermisspell
francais.aff
francais.aff
francais.aff

flyspell do not think francais.aff is a word, so it is not validated and is
re-searched and finally marked as not found, but point is then at the last
francais.aff appearance, so next misspelling, "anothermisspell" is also not
found, as well as any other misspelling in the middle. Bad forward unsync.

There is something else in the validation code that can be improved, based
on Piet van Oostrum suggestion in the old "flyspell bug" thread, validate
if misspelling length is higher than length of what flyspell considers a
word. In the above example, "francais.aff" would be the misspelling, but
considering where point is, flyspell would say that current word is "aff",
and since length("francais.aff") > length("aff") match would be validated
and the above example work even in a per-search point move.

Unfortunately this might not work with more ellaborated mismatches, so I
think we should combine both things, validating also as above and be
safer starting searches from last validated match.

I am attaching a more ellaborated patch for consideration

-- 
Agustin

[-- Attachment #2: flyspell.el.flyspell-external-point-words.diff --]
[-- Type: text/plain, Size: 2160 bytes --]

--- flyspell.el.orig	2005-12-22 12:23:55.000000000 +0100
+++ flyspell.el	2005-12-22 12:49:28.000000000 +0100
@@ -1325,6 +1325,9 @@
 		       (* 100 (/ (float (point)) (point-max)))
 		       word))
 	  (with-current-buffer flyspell-large-region-buffer
+	    ;; Make sure search starts from last validated match (or from
+	    ;; beginning of region if the first time). This intends to avoid
+	    ;; erroneous forward jumps that might cause fatal forward unsyncs.
 	    (goto-char flyspell-large-region-beg)
 	    (let ((keep t))
 	      ;; Iterate on string search until string is found as word,
@@ -1334,15 +1337,21 @@
 				    flyspell-large-region-end t)
 		    (save-excursion
 		      (goto-char (- (point) 1))
-		      (let* ((flyword-prev-l (flyspell-get-word nil))
+		      (let* ((match-point (+ (point) 1))
+			     (flyword-prev-l (flyspell-get-word nil))
 			     (flyword-prev (car flyword-prev-l))
-			     (size-match (= (length flyword-prev) (length word))))
+			     (flyword-length (length flyword-prev))
+			     (misspell-length (length word)))
 			(when (or
-			       ;; size matches, we are done
-			       size-match
+			       ;; Size matches, we are done
+			       (= flyword-length misspell-length)
 			       ;; Matches as part of a boundary-char separated word
 			       (member word
 				       (split-string flyword-prev ispell-otherchars))
+			       ;; Misspelling has higher length than what flyspell
+			       ;; considers the word. Caused by boundary-chars mismatch.
+			       ;; Validating seems safe.
+			       (< flyword-length misspell-length)
 			       ;; ispell treats beginning of some TeX
 			       ;; commands as nroff control sequences
 			       ;; and strips them in the list of
@@ -1360,8 +1369,9 @@
 					nil))))
 			  (setq keep nil)
 			  (flyspell-word)
-			  ;; Next search will begin from end of last match
-			  )))
+			  ;; Search for next misspelled word will begin from
+			  ;; end of last validated match.
+			  (setq flyspell-large-region-beg match-point))))
 		  ;; Record if misspelling is not found and try new one
 		  (add-to-list 'words-not-found
 			       (concat " -> " word " - "

[-- Attachment #3: Type: text/plain, Size: 142 bytes --]

_______________________________________________
Emacs-devel mailing list
Emacs-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/emacs-devel