unofficial mirror of bug-gnu-emacs@gnu.org 
 help / color / mirror / code / Atom feed
* bug#26672: 25.2; Flyspell overlay conflicts with table.el
@ 2017-04-26 22:00 Allen Li
  2017-04-28  9:10 ` Eli Zaretskii
  0 siblings, 1 reply; 6+ messages in thread
From: Allen Li @ 2017-04-26 22:00 UTC (permalink / raw)
  To: 26672

Flyspell's overlay for misspelled words conflicts with table.el

table.el adds its keymap as a text property to the text in table cells.
When Flyspell detects a misspelled word, it adds an overlay with a
keymap binding mouse2 to ‘flyspell-correct-word’.  Apparently, this
overlay keymap overrides table.el’s ‘keymap’ text property.

The effect of this is that pressing TAB to move between table cells will
instead insert a literal tab character if your cursor happens to be on a
misspelled word.  This is extremely annoying.

More generally, I’m not sure that an overlay keymap replacing the
‘keymap’ text property is desired behavior.  At the very least, there
should be an escape hatch option on the overlay keymap that defers to
the ‘keymap’ text property for cases like Flyspell where replacing the
‘keymap’ text property is not desired behavior.

I am aware that there’s an option ‘flyspell-highlight-properties’ to
prevent Flyspell adding its overlay if the text has properties, but
that’s not really useful since most modes other than perhaps fundamental or text
will add various properties to text.





^ permalink raw reply	[flat|nested] 6+ messages in thread

* bug#26672: 25.2; Flyspell overlay conflicts with table.el
  2017-04-26 22:00 bug#26672: 25.2; Flyspell overlay conflicts with table.el Allen Li
@ 2017-04-28  9:10 ` Eli Zaretskii
  2017-04-29  4:19   ` Allen Li
  0 siblings, 1 reply; 6+ messages in thread
From: Eli Zaretskii @ 2017-04-28  9:10 UTC (permalink / raw)
  To: Allen Li; +Cc: 26672

> From: Allen Li <vianchielfaura@gmail.com>
> Date: Wed, 26 Apr 2017 15:00:05 -0700
> 
> Flyspell's overlay for misspelled words conflicts with table.el
> 
> table.el adds its keymap as a text property to the text in table cells.
> When Flyspell detects a misspelled word, it adds an overlay with a
> keymap binding mouse2 to ‘flyspell-correct-word’.  Apparently, this
> overlay keymap overrides table.el’s ‘keymap’ text property.
> 
> The effect of this is that pressing TAB to move between table cells will
> instead insert a literal tab character if your cursor happens to be on a
> misspelled word.  This is extremely annoying.

Thank you for your report.

Could you please provide a complete recipe for reproducing the
problem, starting with "emacs -Q", and loading all the necessary
packages and visiting files if needed?  I think I know how to fix
this, but I need a clear-cut test case, and I don't use table.el to
easily know how to do that.

Also, is the problem only with TAB, or are there other keys which
conflict with the Flyspell overlay keymap?

> More generally, I’m not sure that an overlay keymap replacing the
> ‘keymap’ text property is desired behavior.  At the very least, there
> should be an escape hatch option on the overlay keymap that defers to
> the ‘keymap’ text property for cases like Flyspell where replacing the
> ‘keymap’ text property is not desired behavior.

I think we do have the necessary infrastructure in Emacs to achieve
the effect you want, it's just a matter of using it.  Whether to use
it in any given case is a decision that should be made on a case by
case basis, since the user and/or application could want one or the
other.





^ permalink raw reply	[flat|nested] 6+ messages in thread

* bug#26672: 25.2; Flyspell overlay conflicts with table.el
  2017-04-28  9:10 ` Eli Zaretskii
@ 2017-04-29  4:19   ` Allen Li
  2017-04-29  9:09     ` Eli Zaretskii
  0 siblings, 1 reply; 6+ messages in thread
From: Allen Li @ 2017-04-29  4:19 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 26672

1. emacs -Q
2. M-x flyspell-mode
3. M-x table-insert RET RET RET ... (the defaults are fine)
4. apple M-b TAB (apple is spelled correctly, TAB works)
5. asdf M-b TAB (wait for flyspell to mark the misspelling, TAB doesn't work)

On Fri, Apr 28, 2017 at 2:10 AM, Eli Zaretskii <eliz@gnu.org> wrote:
>> From: Allen Li <vianchielfaura@gmail.com>
>> Date: Wed, 26 Apr 2017 15:00:05 -0700
>>
>> Flyspell's overlay for misspelled words conflicts with table.el
>>
>> table.el adds its keymap as a text property to the text in table cells.
>> When Flyspell detects a misspelled word, it adds an overlay with a
>> keymap binding mouse2 to ‘flyspell-correct-word’.  Apparently, this
>> overlay keymap overrides table.el’s ‘keymap’ text property.
>>
>> The effect of this is that pressing TAB to move between table cells will
>> instead insert a literal tab character if your cursor happens to be on a
>> misspelled word.  This is extremely annoying.
>
> Thank you for your report.
>
> Could you please provide a complete recipe for reproducing the
> problem, starting with "emacs -Q", and loading all the necessary
> packages and visiting files if needed?  I think I know how to fix
> this, but I need a clear-cut test case, and I don't use table.el to
> easily know how to do that.
>
> Also, is the problem only with TAB, or are there other keys which
> conflict with the Flyspell overlay keymap?
>
>> More generally, I’m not sure that an overlay keymap replacing the
>> ‘keymap’ text property is desired behavior.  At the very least, there
>> should be an escape hatch option on the overlay keymap that defers to
>> the ‘keymap’ text property for cases like Flyspell where replacing the
>> ‘keymap’ text property is not desired behavior.
>
> I think we do have the necessary infrastructure in Emacs to achieve
> the effect you want, it's just a matter of using it.  Whether to use
> it in any given case is a decision that should be made on a case by
> case basis, since the user and/or application could want one or the
> other.





^ permalink raw reply	[flat|nested] 6+ messages in thread

* bug#26672: 25.2; Flyspell overlay conflicts with table.el
  2017-04-29  4:19   ` Allen Li
@ 2017-04-29  9:09     ` Eli Zaretskii
  2017-04-29 21:42       ` Allen Li
  0 siblings, 1 reply; 6+ messages in thread
From: Eli Zaretskii @ 2017-04-29  9:09 UTC (permalink / raw)
  To: Allen Li; +Cc: 26672

> From: Allen Li <vianchielfaura@gmail.com>
> Date: Fri, 28 Apr 2017 21:19:38 -0700
> Cc: 26672@debbugs.gnu.org
> 
> 1. emacs -Q
> 2. M-x flyspell-mode
> 3. M-x table-insert RET RET RET ... (the defaults are fine)
> 4. apple M-b TAB (apple is spelled correctly, TAB works)
> 5. asdf M-b TAB (wait for flyspell to mark the misspelling, TAB doesn't work)

Thanks.  Does the change below give good results?

diff --git a/lisp/textmodes/flyspell.el b/lisp/textmodes/flyspell.el
index 0edf9b1..ecf729d 100644
--- a/lisp/textmodes/flyspell.el
+++ b/lisp/textmodes/flyspell.el
@@ -447,7 +447,9 @@ flyspell-prog-mode
 ;;*    The minor mode declaration.                                      */
 ;;*---------------------------------------------------------------------*/
 (defvar flyspell-mouse-map
-  (make-sparse-keymap)
+  (let ((map (make-sparse-keymap)))
+    (define-key map [mouse-2] 'flyspell-correct-word)
+    map)
   "Keymap for Flyspell to put on erroneous words.")
 
 (defvar flyspell-mode-map
@@ -1759,6 +1761,9 @@ make-flyspell-overlay
     (overlay-put overlay 'flyspell-overlay t)
     (overlay-put overlay 'evaporate t)
     (overlay-put overlay 'help-echo "mouse-2: correct word at point")
+    ;; If misspelled text has a 'keymap' property, let that remain in
+    ;; effect for the bindings that flyspell-mouse-map doesn't override.
+    (set-keymap-parent flyspell-mouse-map (get-char-property beg 'keymap))
     (overlay-put overlay 'keymap flyspell-mouse-map)
     (when (eq face 'flyspell-incorrect)
       (and (stringp flyspell-before-incorrect-word-string)





^ permalink raw reply related	[flat|nested] 6+ messages in thread

* bug#26672: 25.2; Flyspell overlay conflicts with table.el
  2017-04-29  9:09     ` Eli Zaretskii
@ 2017-04-29 21:42       ` Allen Li
  2017-04-30 18:57         ` Eli Zaretskii
  0 siblings, 1 reply; 6+ messages in thread
From: Allen Li @ 2017-04-29 21:42 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 26672

Works, thanks, although I had to rebase the patch on emacs-25 branch
as I couldn't get the master branch to compile.

On Sat, Apr 29, 2017 at 2:09 AM, Eli Zaretskii <eliz@gnu.org> wrote:
>> From: Allen Li <vianchielfaura@gmail.com>
>> Date: Fri, 28 Apr 2017 21:19:38 -0700
>> Cc: 26672@debbugs.gnu.org
>>
>> 1. emacs -Q
>> 2. M-x flyspell-mode
>> 3. M-x table-insert RET RET RET ... (the defaults are fine)
>> 4. apple M-b TAB (apple is spelled correctly, TAB works)
>> 5. asdf M-b TAB (wait for flyspell to mark the misspelling, TAB doesn't work)
>
> Thanks.  Does the change below give good results?
>
> diff --git a/lisp/textmodes/flyspell.el b/lisp/textmodes/flyspell.el
> index 0edf9b1..ecf729d 100644
> --- a/lisp/textmodes/flyspell.el
> +++ b/lisp/textmodes/flyspell.el
> @@ -447,7 +447,9 @@ flyspell-prog-mode
>  ;;*    The minor mode declaration.                                      */
>  ;;*---------------------------------------------------------------------*/
>  (defvar flyspell-mouse-map
> -  (make-sparse-keymap)
> +  (let ((map (make-sparse-keymap)))
> +    (define-key map [mouse-2] 'flyspell-correct-word)
> +    map)
>    "Keymap for Flyspell to put on erroneous words.")
>
>  (defvar flyspell-mode-map
> @@ -1759,6 +1761,9 @@ make-flyspell-overlay
>      (overlay-put overlay 'flyspell-overlay t)
>      (overlay-put overlay 'evaporate t)
>      (overlay-put overlay 'help-echo "mouse-2: correct word at point")
> +    ;; If misspelled text has a 'keymap' property, let that remain in
> +    ;; effect for the bindings that flyspell-mouse-map doesn't override.
> +    (set-keymap-parent flyspell-mouse-map (get-char-property beg 'keymap))
>      (overlay-put overlay 'keymap flyspell-mouse-map)
>      (when (eq face 'flyspell-incorrect)
>        (and (stringp flyspell-before-incorrect-word-string)





^ permalink raw reply	[flat|nested] 6+ messages in thread

* bug#26672: 25.2; Flyspell overlay conflicts with table.el
  2017-04-29 21:42       ` Allen Li
@ 2017-04-30 18:57         ` Eli Zaretskii
  0 siblings, 0 replies; 6+ messages in thread
From: Eli Zaretskii @ 2017-04-30 18:57 UTC (permalink / raw)
  To: Allen Li; +Cc: 26672-done

> From: Allen Li <vianchielfaura@gmail.com>
> Date: Sat, 29 Apr 2017 14:42:15 -0700
> Cc: 26672@debbugs.gnu.org
> 
> Works, thanks, although I had to rebase the patch on emacs-25 branch
> as I couldn't get the master branch to compile.

Thanks, pushed to the master branch.





^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2017-04-30 18:57 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-04-26 22:00 bug#26672: 25.2; Flyspell overlay conflicts with table.el Allen Li
2017-04-28  9:10 ` Eli Zaretskii
2017-04-29  4:19   ` Allen Li
2017-04-29  9:09     ` Eli Zaretskii
2017-04-29 21:42       ` Allen Li
2017-04-30 18:57         ` Eli Zaretskii

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).