Re: bug#75327: 31.0.50; ERC 5.6.1-git: M-TAB not autocorrecting with erc-spelling module and flyspell

[J.P.]

Trevor Arjeski <tmarjeski@gmail.com> writes:

> When using the erc-spelling module, pressing M-TAB to autocorrect a word
> does not trigger autocorrection, but using C-. or C-; does, even when
> `flyspell-use-meta-tab' is enabled.
>
> Minimal config:
>
> (use-package erc
>   :config
>   (setopt erc-modules '(spelling))
>   (erc-spelling-mode))
>
> 1. Open an erc buffer
> 2. Type 'thier' and hit M-TAB
> 3. Notice it passes through to ispell - perhaps due to the command
> passthrough in `flyspell-auto-correct-word'.

Regarding this "passthrough" behavior with "M-TAB" vs "C-.", I think
what's happening is that when you type "C-.", the call to

  (let (flyspell-mode) (key-binding (kbd "C-.")))

in `flyspell-auto-correct-word' returns nil because there's no global or
`erc-mode' binding for that key, so `flyspell-word' ultimately runs
further down in the function body. And so too does the predicate in
question, `erc-spelling-flyspell-verify', with `flyspell-word' bound as
a dynamic variable. By contrast,

  (let (flyspell-mode) (key-binding (kbd "C-M-i"))) ; or "M-TAB"

yields a non-nil result, namely `ispell-complete-word', because that's
the binding in `erc-mode-map'. And if you were to remove that with
`local-unset-key', you'd observe `flyspell-auto-correct-word' deferring
to `complete-symbol', which appeals to CAPF and `pcomplete'-related
machinery in ERC buffers.

> Setting `flyspell-generic-check-word-predicate' to nil in the erc buffer
> resolves the issue, but we lose verification that is done in
> `erc-spelling-flyspell-verify'.

Right, that basically nullifies the integration.

> Here is a patch to resolve this issue:
>
> From 3d32aea96cc6ec020e35901d35f1d02994912215 Mon Sep 17 00:00:00 2001
> From: Trevor Arjeski <tmarjeski@gmail.com>
> Date: Sat, 4 Jan 2025 08:45:29 +0300
> Subject: [PATCH] erc: bug#75327 fix flyspell verify
>
> Slightly refactored `erc-spelling-flyspell-verify' to use
> `flyspell-get-word', which in turn, allows M-TAB to call
> `flyspell-auto-correct-word'.

In the future, please use the GNU ChangeLog style for commit messages,
as described in CONTRIBUTE.

> ---
>  lisp/erc/erc-spelling.el | 29 +++++++++++++----------------
>  1 file changed, 13 insertions(+), 16 deletions(-)
>
> diff --git a/lisp/erc/erc-spelling.el b/lisp/erc/erc-spelling.el
> index 01e587af368..0bfb20fce2e 100644
> --- a/lisp/erc/erc-spelling.el
> +++ b/lisp/erc/erc-spelling.el
> @@ -92,22 +92,19 @@ erc-spelling-unhighlight-word
>  
>  (defun erc-spelling-flyspell-verify ()
>    "Flyspell only the input line, nothing else."
[...]
> +  (when-let* (((>= (point) erc-input-marker))
> +              (word-data (flyspell-get-word)))

While calling `flyspell-get-word' definitely seems to work and is
probably harmless, I'm not sure Flyspell's authors intended these
predicates to know about the current candidate, much less modify
existing overlays based on that info (both preexisting transgressions
and obviously not your doing).

There's also the small matter of `flyspell-get-word' running twice in
quick succession during the course of a normal `flyspell-word' call.
Again, probably harmless, but if there's a more idiomatic way to get at
this, we should probably abide (see attached), just in case future
flyspell.el hackers decide to assume all "consumers" of its API are well
behaved citizens.

> +    (cond
> +     ;; don't spell-check names of users
> +     ((and erc-channel-users
> +           (erc-get-channel-user (car word-data)))
> +      (erc-spelling-unhighlight-word word-data)
> +      nil)
> +     ;; if '/' occurs before the word, don't spell-check it
> +     ((eq (char-before (nth 1 word-data)) ?/)
> +      (erc-spelling-unhighlight-word word-data)
> +      nil)
> +     (t t))))
>  
>  (put 'erc-mode
>       'flyspell-mode-predicate

Please see the attached iteration, which has a few more changes than
your v1 patch. If we go with something similar, that would, by my
uninformed figuring, put you right at the copyright-exempt limit of
12-ish nontrivial lines (though we may be pushing the envelope a tad).

Thanks.

P.S. Have you already (or will you) put in for a copyright assignment?

0000-v1-v2.diff
Description: Text Data

0001-5.6.1-Improve-use-of-flyspell-s-API-in-erc-spelling.patch
Description: Text Data

0002-5.6.1-Declare-unused-functions-in-erc-spelling-obsol.patch
Description: Text Data