* flyspell.el [1.90->1.91] flyspell-large-region-beg should be moved after good match @ 2005-12-16 12:42 Agustin Martin 2005-12-17 1:03 ` Richard M. Stallman 0 siblings, 1 reply; 5+ messages in thread From: Agustin Martin @ 2005-12-16 12:42 UTC (permalink / raw) [-- 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 ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: flyspell.el [1.90->1.91] flyspell-large-region-beg should be moved after good match 2005-12-16 12:42 flyspell.el [1.90->1.91] flyspell-large-region-beg should be moved after good match Agustin Martin @ 2005-12-17 1:03 ` Richard M. Stallman 2005-12-19 0:41 ` Agustin Martin 0 siblings, 1 reply; 5+ messages in thread From: Richard M. Stallman @ 2005-12-17 1:03 UTC (permalink / raw) Cc: emacs-devel 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 ---- ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: flyspell.el [1.90->1.91] flyspell-large-region-beg should be moved after good match 2005-12-17 1:03 ` Richard M. Stallman @ 2005-12-19 0:41 ` Agustin Martin 2005-12-19 1:28 ` Agustin Martin 0 siblings, 1 reply; 5+ messages in thread From: Agustin Martin @ 2005-12-19 0:41 UTC (permalink / raw) 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 ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: flyspell.el [1.90->1.91] flyspell-large-region-beg should be moved after good match 2005-12-19 0:41 ` Agustin Martin @ 2005-12-19 1:28 ` Agustin Martin 2005-12-22 13:02 ` Agustin Martin 0 siblings, 1 reply; 5+ messages in thread From: Agustin Martin @ 2005-12-19 1:28 UTC (permalink / raw) 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 ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: flyspell.el [1.90->1.91] flyspell-large-region-beg should be moved after good match 2005-12-19 1:28 ` Agustin Martin @ 2005-12-22 13:02 ` Agustin Martin 0 siblings, 0 replies; 5+ messages in thread From: Agustin Martin @ 2005-12-22 13:02 UTC (permalink / raw) [-- 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 ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2005-12-22 13:02 UTC | newest] Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2005-12-16 12:42 flyspell.el [1.90->1.91] flyspell-large-region-beg should be moved after good match Agustin Martin 2005-12-17 1:03 ` Richard M. Stallman 2005-12-19 0:41 ` Agustin Martin 2005-12-19 1:28 ` Agustin Martin 2005-12-22 13:02 ` Agustin Martin
Code repositories for project(s) associated with this public inbox https://git.savannah.gnu.org/cgit/emacs.git This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).