* More problems with flyspell @ 2006-01-05 13:37 Piet van Oostrum 2006-01-05 21:52 ` Stefan Monnier 2006-01-06 0:11 ` Agustin Martin 0 siblings, 2 replies; 10+ messages in thread From: Piet van Oostrum @ 2006-01-05 13:37 UTC (permalink / raw) Some recent changes in flyspell.el (flyspell-accept-buffer-local-defs) have caused me a problem. It hit me when I updated my CVS emacs this week: I want my email and news messages to be flyspell-checked automatically, preferably with the right language. So in mail-mode-hook, message-mode-hook and vm-mail-mode-hook I run a function that installs a first-change-hook. In the latter I do some regular expression grepping to guess what language the message is in. This only works when replying to a message, for an new message the default is "Dutch". When I am going to enter a new message in English, I manually run a command to reset it to English. Each time I call (flyspell-mode 1). This used to work, but stopped with the recent version. The reason is the optimization in flyspell-accept-buffer-local-defs. It won't restart ispell because there was no buffer change. So I think when flyspell-accept-buffer-local-defs is called from flyspell-mode-on it should do its work unconditionally. This can be done by setting flyspell-last-buffer to nil in flyspell-mode-on. Or maybe another solution is to give flyspell-accept-buffer-local-defs a parameter that tells whether to do the optimization and only do it in time-critical situations. -- Piet van Oostrum <piet@cs.uu.nl> URL: http://www.cs.uu.nl/~piet [PGP 8DAE142BE17999C4] Private email: piet@vanoostrum.org ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: More problems with flyspell 2006-01-05 13:37 More problems with flyspell Piet van Oostrum @ 2006-01-05 21:52 ` Stefan Monnier 2006-01-06 0:11 ` Agustin Martin 1 sibling, 0 replies; 10+ messages in thread From: Stefan Monnier @ 2006-01-05 21:52 UTC (permalink / raw) Cc: emacs-devel > Some recent changes in flyspell.el (flyspell-accept-buffer-local-defs) have > caused me a problem. It hit me when I updated my CVS emacs this week: [...] > Or maybe another solution is to give flyspell-accept-buffer-local-defs a > parameter that tells whether to do the optimization and only do it in > time-critical situations. That's what I've done. Thank you. It should be fixed now. Stefan ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: More problems with flyspell 2006-01-05 13:37 More problems with flyspell Piet van Oostrum 2006-01-05 21:52 ` Stefan Monnier @ 2006-01-06 0:11 ` Agustin Martin 2006-01-08 14:47 ` Richard M. Stallman 1 sibling, 1 reply; 10+ messages in thread From: Agustin Martin @ 2006-01-06 0:11 UTC (permalink / raw) On Thu, Jan 05, 2006 at 02:37:42PM +0100, Piet van Oostrum wrote: > Some recent changes in flyspell.el (flyspell-accept-buffer-local-defs) have > caused me a problem. It hit me when I updated my CVS emacs this week: > > I want my email and news messages to be flyspell-checked automatically, > preferably with the right language. So in mail-mode-hook, message-mode-hook > and vm-mail-mode-hook I run a function that installs a first-change-hook. > In the latter I do some regular expression grepping to guess what language > the message is in. This only works when replying to a message, for an new > message the default is "Dutch". When I am going to enter a new message in > English, I manually run a command to reset it to English. Each time I call > (flyspell-mode 1). This used to work, but stopped with the recent version. > The reason is the optimization in flyspell-accept-buffer-local-defs. It > won't restart ispell because there was no buffer change. > So I think when > flyspell-accept-buffer-local-defs is called from flyspell-mode-on it should > do its work unconditionally. This can be done by setting > flyspell-last-buffer to nil in flyspell-mode-on. > > Or maybe another solution is to give flyspell-accept-buffer-local-defs a > parameter that tells whether to do the optimization and only do it in > time-critical situations. While we are with this, I have noticed that flyspell-accept-buffer-local-defs is called from flyspell-large-region, but not from flyspell-small-region. By the way, since none of both is supposed to be called directly, might be better to move that call to flyspell-region. that is the interactive function that will internally call flyspell-{large,small}-region. Since I would expect this to be affected by the same problem the preferred fix from those you propose above should also be applied here. -- Agustin ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: More problems with flyspell 2006-01-06 0:11 ` Agustin Martin @ 2006-01-08 14:47 ` Richard M. Stallman 2006-01-09 12:53 ` Agustin Martin 0 siblings, 1 reply; 10+ messages in thread From: Richard M. Stallman @ 2006-01-08 14:47 UTC (permalink / raw) Cc: emacs-devel While we are with this, I have noticed that flyspell-accept-buffer-local-defs is called from flyspell-large-region, but not from flyspell-small-region. flyspell-small-region works by calling flyspell-word. I thought that flyspell-word arranged to do flyspell-accept-buffer-local-defs, or the equivalent. Doesn't it do that? However, if calling flyspell-accept-buffer-local-defs twice does no harm, it would be fine to move that call into flyspell-region. ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: More problems with flyspell 2006-01-08 14:47 ` Richard M. Stallman @ 2006-01-09 12:53 ` Agustin Martin 2006-01-09 18:26 ` Agustin Martin 0 siblings, 1 reply; 10+ messages in thread From: Agustin Martin @ 2006-01-09 12:53 UTC (permalink / raw) On Sun, Jan 08, 2006 at 09:47:58AM -0500, Richard M. Stallman wrote: > While we are with this, I have noticed that > flyspell-accept-buffer-local-defs is called from flyspell-large-region, but > not from flyspell-small-region. > > flyspell-small-region works by calling flyspell-word. I thought that > flyspell-word arranged to do flyspell-accept-buffer-local-defs, or > the equivalent. Doesn't it do that? Not always with the optimization Piet mentioned, e.g. in next sequence, with flyspell-region operating over a region that will call flyspell-small-region a) Load file and run flyspell-region on a small region. O.K. => Starting new Ispell process [francais0] ... => Spell Checking completed. b) Notice you used the wrong dict, ispell-change-dictionary to the new one => Loading cl-macs...done => Ispell process killed => Local Ispell dictionary set to francais c) Run again flyspell-region on the same region => if: Buffer french-LIZEZMOI has no process > However, if calling flyspell-accept-buffer-local-defs twice does no > harm, it would be fine to move that call into flyspell-region. However, you are right, (flyspell-word) is the key here and if instead of (c) you run flyspell-word over a word the same error appears. I think we can handle this from ispell.el as part of the ispell-change-dictionary+double-call problem making sure flyspell-last-buffer is nil'ed on dict change if flyspell is loaded. this way first call to flyspell-word would re-run flyspell-accept-buffer-local-defs and skip that for further calls from the same buffer. No need to add an explicit call in flyspell-small-region or flyspell-region. This might also fix the original problem Piet described, but in a different way (compatible with the current fix, no need for changes there). -- Agustin ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: More problems with flyspell 2006-01-09 12:53 ` Agustin Martin @ 2006-01-09 18:26 ` Agustin Martin 2006-01-09 21:53 ` Stefan Monnier 0 siblings, 1 reply; 10+ messages in thread From: Agustin Martin @ 2006-01-09 18:26 UTC (permalink / raw) [-- Attachment #1: Type: text/plain, Size: 1939 bytes --] On Mon, Jan 09, 2006 at 01:53:13PM +0100, Agustin Martin wrote: > c) Run again flyspell-region on the same region > => if: Buffer french-LIZEZMOI has no process Just noticed that this becomes evident when ispell patch for "flyspell needs ispell-change-dictionary twice" is applied. When using current plain ispell.el from CVS this error message does not appear, but things are also buggy; although silently, dict is not changed. > > > However, if calling flyspell-accept-buffer-local-defs twice does no > > harm, it would be fine to move that call into flyspell-region. > > However, you are right, (flyspell-word) is the key here and if instead of > (c) you run flyspell-word over a word the same error appears. I think we can > handle this from ispell.el as part of the > ispell-change-dictionary+double-call problem making sure > flyspell-last-buffer is nil'ed on dict change if flyspell is loaded. this > way first call to flyspell-word would re-run > flyspell-accept-buffer-local-defs and skip that for further calls from the > same buffer. No need to add an explicit call in flyspell-small-region or > flyspell-region. > > This might also fix the original problem Piet described, but in a different > way (that looks compatible with the current fix, no need for changes there). I am attaching a possible patch for that. It is done on top of the (not yet installed) "flyspell needs ispell-change-dictionary twice" patch. It is the incremental one, [ispell.el.ispell-internal-change-dictionary+incremental.diff] (ispell-internal-change-dictionary) - Clear out flyspell-last-buffer if flyspell is loaded and dict is changed, so (flyspell-accept-buffer-local-defs) is re-run for the new values. I am also attaching the full patch for current ispell.el from CVS, so it can be tested better if previous patch is not yet installed when reading this mail. [ispell.el.ispell-internal-change-dictionary+full.diff] -- Agustin [-- Attachment #2: ispell.el.ispell-internal-change-dictionary+incremental.diff --] [-- Type: text/plain, Size: 700 bytes --] --- ispell.el.0 2006-01-09 12:49:13.000000000 +0100 +++ ispell.el 2006-01-09 13:22:27.000000000 +0100 @@ -2641,10 +2641,11 @@ (ispell-kill-ispell t) (setq ispell-current-dictionary dict) ;; If needed, start ispell process and clear out flyspell word cache - (when (and (featurep 'flyspell) - flyspell-mode) - (ispell-init-process) - (setq flyspell-word-cache-word nil))))) + (when (featurep 'flyspell) + (setq flyspell-last-buffer nil) ;; re-read buffer-local-defs + (when flyspell-mode + (ispell-init-process) + (setq flyspell-word-cache-word nil)))))) ;;; Spelling of comments are checked when ispell-check-comments is non-nil. [-- Attachment #3: ispell.el.ispell-internal-change-dictionary+full.diff --] [-- Type: text/plain, Size: 1734 bytes --] --- ispell.el.orig0 2005-12-20 15:06:29.000000000 +0100 +++ ispell.el 2006-01-09 13:22:27.000000000 +0100 @@ -2504,7 +2504,8 @@ (setq ispell-filter nil ispell-filter-continue nil) ;; may need to restart to select new personal dictionary. (ispell-kill-ispell t) - (message "Starting new Ispell process...") + (message "Starting new Ispell process [%s] ..." + (or ispell-local-dictionary ispell-dictionary "default")) (sit-for 0) (setq ispell-library-directory (ispell-check-version) ispell-process-directory default-directory @@ -2619,6 +2620,14 @@ (setq ispell-local-dictionary dict) (setq ispell-local-dictionary-overridden t)) (error "Undefined dictionary: %s" dict)) + ;; For global setting clear out flyspell word cache when needed + (when (and arg + (featurep 'flyspell)) + (dolist (buf (buffer-list)) + (with-current-buffer buf + (when flyspell-mode + (setq flyspell-word-cache-word nil))))) + (ispell-internal-change-dictionary) (message "%s Ispell dictionary set to %s" (if arg "Global" "Local") dict)))) @@ -2630,8 +2639,13 @@ (let ((dict (or ispell-local-dictionary ispell-dictionary))) (unless (equal ispell-current-dictionary dict) (ispell-kill-ispell t) - (setq ispell-current-dictionary dict)))) - + (setq ispell-current-dictionary dict) + ;; If needed, start ispell process and clear out flyspell word cache + (when (featurep 'flyspell) + (setq flyspell-last-buffer nil) ;; re-read buffer-local-defs + (when flyspell-mode + (ispell-init-process) + (setq flyspell-word-cache-word nil)))))) ;;; Spelling of comments are checked when ispell-check-comments is non-nil. [-- Attachment #4: 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] 10+ messages in thread
* Re: More problems with flyspell 2006-01-09 18:26 ` Agustin Martin @ 2006-01-09 21:53 ` Stefan Monnier 2006-01-09 22:56 ` Agustin Martin 0 siblings, 1 reply; 10+ messages in thread From: Stefan Monnier @ 2006-01-09 21:53 UTC (permalink / raw) Cc: emacs-devel > @@ -2630,8 +2639,13 @@ > (let ((dict (or ispell-local-dictionary ispell-dictionary))) > (unless (equal ispell-current-dictionary dict) > (ispell-kill-ispell t) > - (setq ispell-current-dictionary dict)))) > - > + (setq ispell-current-dictionary dict) > + ;; If needed, start ispell process and clear out flyspell word cache > + (when (featurep 'flyspell) > + (setq flyspell-last-buffer nil) ;; re-read buffer-local-defs > + (when flyspell-mode > + (ispell-init-process) > + (setq flyspell-word-cache-word nil)))))) Looking at this patch, I feel like my flyspell-last-buffer should probably be moved from flyspell.el to ispell.el (and renamed ispell-last-buffer). If someone could do that for me, that'd be swell, Stefan PS: Maybe the same should be done with flyspell-word-cache-word, of course, but I'm not familiar enough with it. ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: More problems with flyspell 2006-01-09 21:53 ` Stefan Monnier @ 2006-01-09 22:56 ` Agustin Martin 2006-01-10 4:16 ` Stefan Monnier 0 siblings, 1 reply; 10+ messages in thread From: Agustin Martin @ 2006-01-09 22:56 UTC (permalink / raw) On Mon, Jan 09, 2006 at 04:53:31PM -0500, Stefan Monnier wrote: > Looking at this patch, I feel like my flyspell-last-buffer should probably > be moved from flyspell.el to ispell.el (and renamed ispell-last-buffer). If I am not wrong nothing in ispell.el really needs it, is (flyspell-word) in a loop what needs it. If patch is correct, it will only be set if flyspell.el is loaded, so flyspell.el seems not that a bad place. And it indeed shows name of last buffer accessed by a flyspell operation. Its presence in ispell.el is a bit surprising, but harmless when flyspell.el is not loaded. > PS: Maybe the same should be done with flyspell-word-cache-word, of course, > but I'm not familiar enough with it. That is flyspell only, stores last word checked by (flyspell-word), so flyspell-word is not called again unless cursor goes to a different word. However is good to have it cleared when dict is changed to force re-checking of current word, and the dict change is done from ispell.el. I think this one should not be moved. -- Agustin ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: More problems with flyspell 2006-01-09 22:56 ` Agustin Martin @ 2006-01-10 4:16 ` Stefan Monnier 2006-01-10 14:46 ` Agustin Martin 0 siblings, 1 reply; 10+ messages in thread From: Stefan Monnier @ 2006-01-10 4:16 UTC (permalink / raw) Cc: emacs-devel >> Looking at this patch, I feel like my flyspell-last-buffer should probably >> be moved from flyspell.el to ispell.el (and renamed ispell-last-buffer). > If I am not wrong nothing in ispell.el really needs it, is (flyspell-word) > in a loop what needs it. If patch is correct, it will only be set if > flyspell.el is loaded, so flyspell.el seems not that a bad place. And it > indeed shows name of last buffer accessed by a flyspell operation. > Its presence in ispell.el is a bit surprising, but harmless when > flyspell.el is not loaded. ispell.el is not just an end-user package: it's also a library that provides services for flyspell. The flyspell-last-buffer variable was introduced to optimize a particular operation because flyspell needs that optimization, but the right place to put that optimization is in the ispell.el library, even if the ispell.el end-user commands don't directly benefit from it. >> PS: Maybe the same should be done with flyspell-word-cache-word, of course, >> but I'm not familiar enough with it. > That is flyspell only, stores last word checked by (flyspell-word), so > flyspell-word is not called again unless cursor goes to a different word. > However is good to have it cleared when dict is changed to force re-checking > of current word, and the dict change is done from ispell.el. I think this > one should not be moved. Then maybe rather than move flyspell-last-buffer and flyspell-word-cache-word to ispell.el, ispell.el should provide an ispell-start-process-hook or ispell-change-dict-hook that flyspell could use to reset those vars. Doing a (featurep 'flyspell) test is just really ugly. Stefan ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: More problems with flyspell 2006-01-10 4:16 ` Stefan Monnier @ 2006-01-10 14:46 ` Agustin Martin 0 siblings, 0 replies; 10+ messages in thread From: Agustin Martin @ 2006-01-10 14:46 UTC (permalink / raw) On Mon, Jan 09, 2006 at 11:16:05PM -0500, Stefan Monnier wrote: > > ispell.el is not just an end-user package: it's also a library that provides > services for flyspell. The flyspell-last-buffer variable was introduced to > optimize a particular operation because flyspell needs that optimization, > but the right place to put that optimization is in the ispell.el library, > even if the ispell.el end-user commands don't directly benefit from it. > > >> PS: Maybe the same should be done with flyspell-word-cache-word, of course, > >> but I'm not familiar enough with it. > > I wrote: > > > That is flyspell only, stores last word checked by (flyspell-word), so > > flyspell-word is not called again unless cursor goes to a different word. > > However is good to have it cleared when dict is changed to force re-checking > > of current word, and the dict change is done from ispell.el. I think this > > one should not be moved. > > Then maybe rather than move flyspell-last-buffer and > flyspell-word-cache-word to ispell.el, ispell.el should provide an > ispell-start-process-hook or ispell-change-dict-hook that flyspell could use > to reset those vars. This sounds good to me. What others think? > Doing a (featurep 'flyspell) test is just really ugly. I admit that even visually, seeing explicit "flyspell" strings and code in ispell.el is something I dislike too. -- Agustin ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2006-01-10 14:46 UTC | newest] Thread overview: 10+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2006-01-05 13:37 More problems with flyspell Piet van Oostrum 2006-01-05 21:52 ` Stefan Monnier 2006-01-06 0:11 ` Agustin Martin 2006-01-08 14:47 ` Richard M. Stallman 2006-01-09 12:53 ` Agustin Martin 2006-01-09 18:26 ` Agustin Martin 2006-01-09 21:53 ` Stefan Monnier 2006-01-09 22:56 ` Agustin Martin 2006-01-10 4:16 ` Stefan Monnier 2006-01-10 14:46 ` 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).