unofficial mirror of emacs-devel@gnu.org 
 help / color / mirror / code / Atom feed
From: Michal Nazarewicz <mina86@mina86.com>
To: Stefan Monnier <monnier@iro.umontreal.ca>
Cc: emacs-devel@gnu.org
Subject: [PATCHv2] textmodes/flyspell.el: Don't check pre-word if buffer was switched.
Date: Wed, 27 Mar 2013 13:47:51 +0100	[thread overview]
Message-ID: <xa1twqstggaw.fsf@mina86.com> (raw)
In-Reply-To: <jwvy5d9fy47.fsf-monnier+emacs@gnu.org>

If command changed the buffer, the decision may be made based on the
current buffer even though it should based on the previous one.  This
may lead to false positives and more importantly to errors since
`flyspell-pre-point' is buffer local so it may have unsanitised value
(such as nil) in previous buffer.

To be honest, I'm not sure how this can happen since
`flyspell-pre-point' is set in previous buffer, but nonetheless, I've
been encountering the error for quite some time and finally decided to
fix it.  Interestingly, line making `flyspell-pre-point'
a buffer-local variable has a very revealing "Why?? --Stef" comment.

To avoid the problem, change flyspell-check-pre-word-p so that it does
not allow checking of pre-word if command changed buffer
(ie. `flyspell-pre-buffer' is not current buffer).
---
 lisp/ChangeLog             | 11 +++++++++++
 lisp/textmodes/flyspell.el | 25 +++++++++++--------------
 2 files changed, 22 insertions(+), 14 deletions(-)

 On Wed, Mar 27 2013, Stefan Monnier <monnier@iro.umontreal.ca> wrote:
 > Can't we just say that if current-buffer is different from the previous
 > one, then don't do anything?

 Sure.  I don't mind either way.

diff --git a/lisp/ChangeLog b/lisp/ChangeLog
index 5b24164..e310e35 100644
--- a/lisp/ChangeLog
+++ b/lisp/ChangeLog
@@ -1,3 +1,14 @@
+2013-03-27  Michal Nazarewicz  <mina86@mina86.com>
+
+	* textmodes/flyspell.el (flyspell-check-pre-word-p): Return nil if
+	command changed buffer (ie. `flyspell-pre-buffer' is not current
+	buffer), which prevents making decisions based on invalid value of
+	`flyspell-pre-point' in the wrong buffer.  Most notably, this used to
+	cause an error when `flyspell-pre-point' was nil after switching
+	buffers
+	(flyspell-post-command-hook): No longer needs to change buffers when
+	checking pre-word.  While at it remove unnecessary progn.
+
 2013-03-26  Stefan Monnier  <monnier@iro.umontreal.ca>
 
 	* desktop.el (desktop--v2s): Rename from desktop-internal-v2s.
diff --git a/lisp/textmodes/flyspell.el b/lisp/textmodes/flyspell.el
index 6ab3e3d..81f17c8 100644
--- a/lisp/textmodes/flyspell.el
+++ b/lisp/textmodes/flyspell.el
@@ -738,7 +738,7 @@ before the current command."
   (let ((ispell-otherchars (ispell-get-otherchars)))
     (cond
    ((not (and (numberp flyspell-pre-point)
-              (buffer-live-p flyspell-pre-buffer)))
+              (eq flyspell-pre-buffer (current-buffer))))
       nil)
      ((and (eq flyspell-pre-pre-point flyspell-pre-point)
 	   (eq flyspell-pre-pre-buffer flyspell-pre-buffer))
@@ -956,11 +956,10 @@ Mostly we check word delimiters."
             ;; Prevent anything we do from affecting the mark.
             deactivate-mark)
         (if (flyspell-check-pre-word-p)
-            (with-current-buffer flyspell-pre-buffer
+            (save-excursion
               '(flyspell-debug-signal-pre-word-checked)
-              (save-excursion
-                (goto-char flyspell-pre-point)
-                (flyspell-word))))
+              (goto-char flyspell-pre-point)
+              (flyspell-word)))
         (if (flyspell-check-word-p)
             (progn
               '(flyspell-debug-signal-word-checked)
@@ -974,16 +973,14 @@ Mostly we check word delimiters."
               ;; FLYSPELL-CHECK-PRE-WORD-P
               (setq flyspell-pre-pre-buffer (current-buffer))
               (setq flyspell-pre-pre-point  (point)))
-          (progn
-            (setq flyspell-pre-pre-buffer nil)
-            (setq flyspell-pre-pre-point  nil)
-            ;; when a word is not checked because of a delayed command
-            ;; we do not disable the ispell cache.
-            (if (and (symbolp this-command)
+          (setq flyspell-pre-pre-buffer nil)
+          (setq flyspell-pre-pre-point  nil)
+          ;; when a word is not checked because of a delayed command
+          ;; we do not disable the ispell cache.
+          (when (and (symbolp this-command)
                      (get this-command 'flyspell-delayed))
-                (progn
-                  (setq flyspell-word-cache-end -1)
-                  (setq flyspell-word-cache-result '_)))))
+            (setq flyspell-word-cache-end -1)
+            (setq flyspell-word-cache-result '_)))
         (while (and (not (input-pending-p)) (consp flyspell-changes))
           (let ((start (car (car flyspell-changes)))
                 (stop  (cdr (car flyspell-changes))))



  reply	other threads:[~2013-03-27 12:47 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-03-26 18:09 [PATCH] textmodes/flyspell.el (flyspell-post-command-hook): Decided whether to check previous word while in correct buffer Michal Nazarewicz
2013-03-27  1:09 ` Stefan Monnier
2013-03-27 12:47   ` Michal Nazarewicz [this message]
2013-04-20  4:53     ` [PATCHv2] textmodes/flyspell.el: Don't check pre-word if buffer was switched Agustin Martin
2013-04-20 15:46       ` Michal Nazarewicz

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: https://www.gnu.org/software/emacs/

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=xa1twqstggaw.fsf@mina86.com \
    --to=mina86@mina86.com \
    --cc=emacs-devel@gnu.org \
    --cc=monnier@iro.umontreal.ca \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).