[ELPA] New package: Auto Correct Mode

[ELPA] New package: Auto Correct Mode

[Ian Dunn]

[-- Attachment #1: Type: text/plain, Size: 452 bytes --]


I'd like to submit auto-correct.el as a single-file package to ELPA.  It's a small package that integrates with ispell and abbrev to provide auto-correct functionality in Emacs.  It uses a separate abbrev table, which is only enabled in auto-correct-mode, and only auto-corrects if a buffer-local predicate returns true.

As with the last two I've submitted today, if no one has any objections, I'll update the copyright and push it to the git repo.


[-- Attachment #2: auto-correct.el --]
[-- Type: application/emacs-lisp, Size: 3722 bytes --]

[-- Attachment #3: Type: text/plain, Size: 14 bytes --]


-- 
Ian Dunn

Re: [ELPA] New package: Auto Correct Mode

[Eli Zaretskii]

> From: Ian Dunn <dunni@gnu.org>
> Date: Sun, 27 Aug 2017 12:06:06 -0400
> 
> I'd like to submit auto-correct.el as a single-file package to ELPA.  It's a small package that integrates with ispell and abbrev to provide auto-correct functionality in Emacs.  It uses a separate abbrev table, which is only enabled in auto-correct-mode, and only auto-corrects if a buffer-local predicate returns true.
> 
> As with the last two I've submitted today, if no one has any objections, I'll update the copyright and push it to the git repo.

Thanks.  Please allow me a few stylistic comments:

> ;;; Commentary:
> 
> ;;; Code:

I suggest to say at least a few words about the package and what it
does (or doesn't do) in the Commentary section.  "Auto-correct" sounds
very promising, so users might have their expectations too high...

> (defgroup auto-correct nil
>   "*Auto correction support."

We no longer use the leading asterisk in doc strings.

> (defun auto-correct-fix-and-add (p)
>   "Call `ispell-word', then create an abbrev for it.
> With prefix P, create local abbrev. Otherwise it will
> be global.

Our usual style is

  "With a non-nil argument P (interactively, the prefix argument),
  create ..."

Also, we use the US English convention of leaving 2 spaces between
sentences.

> Originally pulled from endless parenthesis."

???

>       ;; correction was entered by hand.

Comments should preferably be full sentences, so start with a capital
letter.

> ;;;###autoload
> (defun auto-correct-scan-buffer ()
>   "Scans current buffer for misspelled words.

Our usual style is to say "Scan", not "Scans".

> When a misspelled word is found, offers to correct and add."
                                   ^^^^^^
Resp. "offer".  Also, I'd elaborate a bit wrt "and add", as it
otherwise sounds mysterious.

> (defun auto-correct-scan-region (start end)
>   (interactive "r")

Interactive functions should have a doc string.

> (defun auto-correct-scan ()
>   "Scans the buffer or region."

This doc string sounds too minimal...

Thanks again for working on this.

Re: [ELPA] New package: Auto Correct Mode

[Ian Dunn]

[-- Attachment #1: Type: text/plain, Size: 129 bytes --]


    EZ> Thanks.  Please allow me a few stylistic comments:

I made the fixes you suggested, and ran checkdoc for good measure.


[-- Attachment #2: auto-correct.el --]
[-- Type: application/emacs-lisp, Size: 5535 bytes --]

[-- Attachment #3: Type: text/plain, Size: 14 bytes --]


-- 
Ian Dunn

Re: [ELPA] New package: Auto Correct Mode

[Eli Zaretskii]

> From: Ian Dunn <dunni@gnu.org>
> Date: Sun, 27 Aug 2017 14:23:26 -0400
> Cc: emacs-devel@gnu.org
> 
>     EZ> Thanks.  Please allow me a few stylistic comments:
> 
> I made the fixes you suggested, and ran checkdoc for good measure.

Thanks.

Re: [ELPA] New package: Auto Correct Mode

[John Wiegley]

>>>>> "ID" == Ian Dunn <dunni@gnu.org> writes:

ID> I'd like to submit auto-correct.el as a single-file package to ELPA. It's
ID> a small package that integrates with ispell and abbrev to provide
ID> auto-correct functionality in Emacs. It uses a separate abbrev table,
ID> which is only enabled in auto-correct-mode, and only auto-corrects if a
ID> buffer-local predicate returns true.

How does this compare to flyspell-auto-correct-word, or a setting such as:

  (add-hook 'flyspell-incorrect-hook 'flyspell-maybe-correct-transposition)

Maybe what you're offering is already possible, or can be integrated directly
with flyspell using a customization?

-- 
John Wiegley                  GPG fingerprint = 4710 CF98 AF9B 327B B80F
http://newartisans.com                          60E1 46C4 BD1A 7AC1 4BA2

Re: [ELPA] New package: Auto Correct Mode

[Ian Dunn]

    JW> How does this compare to flyspell-auto-correct-word, or a setting such
    JW> as:

    JW>   (add-hook 'flyspell-incorrect-hook
    JW> 'flyspell-maybe-correct-transposition)

    JW> Maybe what you're offering is already possible, or can be integrated
    JW> directly with flyspell using a customization?

I don't believe so.

It's similar to repeatedly using flyspell-auto-correct-word and setting flyspell-abbrev-p to t, but with two differences (that I can see):

1. auto-correct-mode uses its own abbrev table, enabled using `abbrev-minor-mode-table-alist'.  This means it doesn't clutter the user's global abbrev table, and auto-correct can be disabled by disabling the minor mode.  Flyspell only allows the use of the global or local abbrev tables.

2. auto-correct gives a predicate that can provide further control over when to expand auto-corrections.  My favorite example is my abbrev that turns "i" to "I".  I keep auto-correct enabled in all modes, but prog-mode has this predicate set to only auto-correct when inside a comment or string.  If "i" turned to I every time I typed it in C++ (which, so far as I can tell, would happen with flyspell's corrections), I'd go mad.  However, it has proved nothing but useful for this correction to happen when I'm writing prose or documentation.

So as I said, the same functionality doesn't appear to be possible with flyspell at the moment.

-- 
Ian Dunn

Re: [ELPA] New package: Auto Correct Mode

[Ian Dunn]

[-- Attachment #1: Type: text/plain, Size: 1181 bytes --]

>>>>> "JW" == John Wiegley <jwiegley@gmail.com> writes:

    JW> Maybe what you're offering is already possible, or can be
    JW> integrated directly with flyspell using a customization?

Upon reviewing flyspell again, I realized that it is possible to
integrate with it, which I think makes much more sense than having a
package that stands entirely on its own.

What I've done is modified auto-correct to make it more of an extension
to ispell and flyspell, rather than its own interface.  Corrections made
through either of these packages will be added as automatic corrections.

Further, I also added the ability to add support for handling
corrections from any other package.  I explain how to do this in the
commentary.

Summary of New Design:

- Takes a correction from another package and stores it in an abbrev table
- The abbrev table is only active in auto-correct-mode, to avoid cluttering the global abbrev table
- The abbrev table is limited by auto-correct-predicate, which allows for better control
- Integrates with Ispell and flyspell to add their corrections without having to use another correction interface
- Can integrate with any other correction package


[-- Attachment #2: auto-correct.el --]
[-- Type: application/emacs-lisp, Size: 13286 bytes --]

[-- Attachment #3: Type: text/plain, Size: 14 bytes --]


-- 
Ian Dunn

Re: [ELPA] New package: Auto Correct Mode

[John Wiegley]

>>>>> "ID" == Ian Dunn <dunni@gnu.org> writes:

ID> Upon reviewing flyspell again, I realized that it is possible to integrate
ID> with it, which I think makes much more sense than having a package that
ID> stands entirely on its own.

This sounds great, Ian.  I went to install it into my Emacs just now, and upon
evaluating the buffer I get:

auto-correct--add-support: Symbol’s value as variable is void:
auto-correct-mode

It's coming from the minor-mode definition:

  :lighter " Auto-Correct"
  (if auto-correct-mode
      (run-hooks 'auto-correct-activate-functions)
    (run-hooks 'auto-correct-deactivate-functions)))

I see you have a (defvar auto-correct-mode) up above, but that doesn't seem to
help.

To recap, I'm just opening the file and typing M-x eval-buffer.

-- 
John Wiegley                  GPG fingerprint = 4710 CF98 AF9B 327B B80F
http://newartisans.com                          60E1 46C4 BD1A 7AC1 4BA2

Re: [ELPA] New package: Auto Correct Mode

[Ian Dunn]

[-- Attachment #1: Type: text/plain, Size: 1103 bytes --]


    ID> Upon reviewing flyspell again, I realized that it is possible to
    ID> integrate with it, which I think makes much more sense than having a
    ID> package that stands entirely on its own.

    JW> This sounds great, Ian.  I went to install it into my Emacs just now,
    JW> and upon evaluating the buffer I get:

    JW> auto-correct--add-support: Symbol’s value as variable is void:
    JW> auto-correct-mode

    JW> It's coming from the minor-mode definition:

    JW>   :lighter " Auto-Correct" (if auto-correct-mode (run-hooks
    JW> 'auto-correct-activate-functions) (run-hooks
    JW> 'auto-correct-deactivate-functions)))

    JW> I see you have a (defvar auto-correct-mode) up above, but that doesn't
    JW> seem to help.

    JW> To recap, I'm just opening the file and typing M-x eval-buffer.

    JW> -- John Wiegley GPG fingerprint = 4710 CF98 AF9B 327B B80F
    JW> http://newartisans.com 60E1 46C4 BD1A 7AC1 4BA2

Running in a fresh Emacs process and running M-x eval-buffer, I see the
same thing.  I rearranged the declarations to avoid the error.  It
should work now.


[-- Attachment #2: auto-correct.el --]
[-- Type: application/emacs-lisp, Size: 13256 bytes --]

[-- Attachment #3: Type: text/plain, Size: 14 bytes --]



--
Ian Dunn

Re: [ELPA] New package: Auto Correct Mode

[Stefan Monnier]

>   (if auto-correct-mode
>       (run-hooks 'auto-correct-activate-functions)
>     (run-hooks 'auto-correct-deactivate-functions)))

BTW, these are regular hooks (i.e. called with no arguments and only
for their side-effects), so their name should preferably end in
"-hook" than in "-functions".

Also, note that `auto-correct-mode` already runs (thanks to
`define-minor-mode`) `auto-correct-mode-hook` every time the mode is
changed (enabled or disabled), so the above two hooks
aren't indispensable.

E.g. you could use the patch below or simplify further by changing
auto-correct--add/remove-support to only take a single argument.  Or by
dropping this altogether since the third party could simply add itself
to auto-correct-hook directly, since that's a standard interface for
minor modes.

[ While I'm here, I'll wonder aloud why you used "--" names for functions
  which are meant to be used by third party packages, since we normally
  use "--" to mean that this is an internal function/var.  ]

The patch below also changes the flyspell part of the code to use
add-function so as not to make any assumption about the value of
flyspell-insert-function.


        Stefan


diff --git a/packages/auto-correct/auto-correct.el b/packages/auto-correct/auto-correct.el
index 42f84de1f..286b6678d 100644
--- a/packages/auto-correct/auto-correct.el
+++ b/packages/auto-correct/auto-correct.el
@@ -115,18 +115,6 @@ locally.  If nil, the correction will be made whenever
     (write-abbrev-file)
     (message "\"%s\" now expands to \"%s\"" bef aft)))
 
-;; Extension Support
-
-(defvar auto-correct-activate-functions nil
-  "Functions to run to activate auto-correct support in various packages.
-
-These are called by `auto-correct-mode' when it is enabled.")
-
-(defvar auto-correct-deactivate-functions nil
-  "Functions to run to deactivate auto-correct support in various packages.
-
-These are called by `auto-correct-mode' when it is disabled.")
-
 ;; The mode
 
 ;;;###autoload
@@ -152,9 +140,7 @@ the command `auto-correct-toggle-ispell-local'.
   :global t
   :init-value nil
   :lighter " Auto-Correct"
-  (if auto-correct-mode
-      (run-hooks 'auto-correct-activate-functions)
-    (run-hooks 'auto-correct-deactivate-functions)))
+  )
 
 ;; Only enable the abbrev list when auto-correct-mode is active.
 (add-to-list 'abbrev-minor-mode-table-alist
@@ -166,8 +152,8 @@ the command `auto-correct-toggle-ispell-local'.
   "Add auto-correct support for a spelling package.
 
 Support will be activated by ACTIVATE-FUN and deactivated by DEACTIVATE-FUN."
-  (add-hook 'auto-correct-activate-functions activate-fun)
-  (add-hook 'auto-correct-deactivate-functions deactivate-fun)
+  (add-hook 'auto-correct-hook
+            (lambda () (funcall (if auto-correct-mode activate-fun deactivate-fun))))
   ;; If `auto-correct-mode' is enabled, activate this package's support.
   (when auto-correct-mode
     (funcall activate-fun)))
@@ -176,8 +162,9 @@ Support will be activated by ACTIVATE-FUN and deactivated by DEACTIVATE-FUN."
   "Remove support for a spelling package.
 
 Support will be activated by ACTIVATE-FUN and deactivated by DEACTIVATE-FUN."
-  (remove-hook 'auto-correct-activate-functions activate-fun)
-  (remove-hook 'auto-correct-deactivate-functions deactivate-fun)
+  (remove-hook 'auto-correct-hook
+               (lambda ()
+                 (funcall (if auto-correct-mode activate-fun deactivate-fun))))
   ;; If `auto-correct-mode' is enabled, deactivate this package's support.
   (when auto-correct-mode
     (funcall deactivate-fun)))
@@ -209,19 +196,19 @@ When `auto-correct-mode' is enabled, this function is set as
   (let ((old-word flyspell-auto-correct-word)
         (new-word word)
         (local (not flyspell-use-global-abbrev-table-p)))
-    (auto-correct--add-or-update-correction old-word new-word local)
-    (insert word)))
+    (auto-correct--add-or-update-correction old-word new-word local)))
 
 (defun auto-correct--flyspell-activate ()
   "Activate flyspell auto-correct support.
 
 Sets `flyspell-insert-function' to `auto-correct-flyspell-insert'."
   ;; Add flyspell corrections as auto-corrections
-  (setq flyspell-insert-function 'auto-correct-flyspell-insert))
+  (add-function :before flyspell-insert-function
+                #'auto-correct-flyspell-insert))
 
 (defun auto-correct--flyspell-deactivate ()
   "Deactivate flyspell auto-correct support."
-  (setq flyspell-insert-function 'insert))
+  (remove-function flyspell-insert-function #'auto-correct-flyspell-insert))
 
 (defcustom auto-correct-enable-flyspell-support t
   "Whether to automatically correct corrections made in flyspell."

Re: [ELPA] New package: Auto Correct Mode

[Ian Dunn]

>>>>> "Stefan" == Stefan Monnier <monnier@iro.umontreal.ca> writes:

    >> (if auto-correct-mode (run-hooks
    >> 'auto-correct-activate-functions) (run-hooks
    >> 'auto-correct-deactivate-functions)))

    Stefan> BTW, these are regular hooks (i.e. called with no arguments
    Stefan> and only for their side-effects), so their name should
    Stefan> preferably end in "-hook" than in "-functions".

    Stefan> Also, note that `auto-correct-mode` already runs (thanks to
    Stefan> `define-minor-mode`) `auto-correct-mode-hook` every time the
    Stefan> mode is changed (enabled or disabled), so the above two
    Stefan> hooks aren't indispensable.

    Stefan> E.g. you could use the patch below or simplify further by
    Stefan> changing auto-correct--add/remove-support to only take a
    Stefan> single argument.  Or by dropping this altogether since the
    Stefan> third party could simply add itself to auto-correct-hook
    Stefan> directly, since that's a standard interface for minor modes.

I took it one step further and collapsed add/remove-support into just
handle-support.  I also changed it to take a function of one argument that indicates whether to activate or deactivate support.

I don't want to remove the support function entirely, since it's also got the virtue of handling activation/deactivation when it gets called.  Thus, flyspell support will be immediately deactivated when I set the customization variable to nil, or when some other package calls it for support.

Thanks for your input.

-- 
Ian Dunn