Re: [PATCH] Add abbrev suggestions

[Robert Pluim]

>>>>> On Tue, 15 Sep 2020 00:04:05 +0200, Mathias Dahl <mathias.dahl@gmail.com> 
>>>>> said:

    Mathias> Hi Eli,
    Mathias> Finally! I sat down and documented this, updated NEWS as well as 
tried
    Mathias> to format the commit message in the correct manner. Let me know how
    Mathias> the new patch looks like.

    Mathias> Keeping my fingers crossed...

Nitpicking below. Nothing major, it looks to be a useful feature.

    Mathias> From b0b1bb04098a11e7f6c5fffbcbf7cd30be4679e0 Mon Sep 17 00:00:00 
2001
    Mathias> From: Mathias Dahl <mathias.dahl@gmail.com>
    Mathias> Date: Mon, 14 Sep 2020 17:05:11 -0400
    Mathias> Subject: [PATCH] Add abbrev suggestions

    Mathias>     Add abbrev suggestions

Youʼre allowed to be a little bit more verbose in the commit messages
:-) Tell us briefly what this does.

    Mathias>     * lisp/abbrev.el (abbrev-suggest): New defcustom.
    Mathias>     (abbrev-suggest-hint-threshold): New defcustom.
    Mathias>     (abbrev--suggest-get-active-tables-including-parents): New 
defun.
    Mathias>     (abbrev--suggest-get-active-abbrev-expansions): New defun.
    Mathias>     (abbrev--suggest-count-words): New defun.
    Mathias>     (abbrev--suggest-get-previous-words): New defun.
    Mathias>     (abbrev--suggest-above-threshold): New defun.
    Mathias>     (abbrev--suggest-saved-recommendations): New defvar.
    Mathias>     (abbrev--suggest-inform-user): New defun.
    Mathias>     (abbrev--suggest-shortest-abbrev): New defun.
    Mathias>     (abbrev--suggest-maybe-suggest): New defun.
    Mathias>     (abbrev--suggest-get-totals): New defun.
    Mathias>     (abbrev-suggest-show-report): New defun.
    Mathias>     (expand-abbrev): If the previous word was not an abbrev, maybe
    Mathias>     suggest an abbrev to the user.
    Mathias>     * doc/emacs/abbrevs.texi (Abbrev suggestions): New section.
    Mathias>     * etc/NEWS: Announce abbrev suggestions.
    Mathias> ---
    Mathias>  doc/emacs/abbrevs.texi |  33 ++++++++++++
    Mathias>  etc/NEWS               |   8 +++
    Mathias>  lisp/abbrev.el         | 143 
++++++++++++++++++++++++++++++++++++++++++++++++-
    Mathias>  3 files changed, 183 insertions(+), 1 deletion(-)

    Mathias> diff --git a/doc/emacs/abbrevs.texi b/doc/emacs/abbrevs.texi
    Mathias> index 21bf8c5..fbe7117 100644
    Mathias> --- a/doc/emacs/abbrevs.texi
    Mathias> +++ b/doc/emacs/abbrevs.texi
    Mathias> @@ -28,6 +28,7 @@ Abbrevs
    Mathias>  * Abbrev Concepts::   Fundamentals of defined abbrevs.
    Mathias>  * Defining Abbrevs::  Defining an abbrev, so it will expand when 
typed.
    Mathias>  * Expanding Abbrevs:: Controlling expansion: prefixes, canceling 
expansion.
    Mathias> +* Abbrevs Suggestions:: Get suggestions about defined abbrevs.
    Mathias>  * Editing Abbrevs::   Viewing or editing the entire list of 
defined abbrevs.
    Mathias>  * Saving Abbrevs::    Saving the entire list of abbrevs for 
another session.
    Mathias>  * Dynamic Abbrevs::   Abbreviations for words already in the 
buffer.
    Mathias> @@ -223,6 +224,38 @@ Expanding Abbrevs
    Mathias>  the abbrev expansion.  @xref{Abbrev Expansion,,, elisp, The Emacs 
Lisp
    Mathias>  Reference Manual}.
 
    Mathias> +@node Abbrev Suggestions
    Mathias> +@section Abbrev Suggestions
    Mathias> +
    Mathias> +  You can get abbrev suggestions when you manually type text for 
which
    Mathias> +there is currently an active defined abbrev.  For example, if 
there is
    Mathias> +an abbrev @samp{foo} with the expansion @samp{find outer otter}, 
and
    Mathias> +you manually type @samp{find outer otter}, the abbrev suggestion
    Mathias> +feature will notice this and show a hint in the echo when you have
    Mathias> +stop typing.

Either "when you stop" or "when you have stopped"

    Mathias> +
    Mathias> +@vindex abbrev-suggest
    Mathias> +  Enable the abbrev suggestion feature by setting the variable

You donʼt need "the variable" here.

    Mathias> +@code{abbrev-suggest} to @code{t}.
    Mathias> +
    Mathias> +@vindex abbrev-suggest-hint-threshold
    Mathias> +  The variable @code{abbrev-suggest-hint-threshold} controls when 
to

Nor here. Or call it a "user option" (I assume itʼs customizable)

    Mathias> +suggest an abbrev to the user.  The variable defines the number of
    Mathias> +characters that the user must save in order to get a suggestion.  
For
    Mathias> +example, if the user types @samp{foo bar} (seven characters) and 
there
    Mathias> +is an abbrev @samp{fubar} defined (five characters), the user 
will not
    Mathias> +get any suggestion unless the threshold is set to the number 2 or
    Mathias> +lower.  With the default value 3, the user would not get any
    Mathias> +suggestion, because the savings in using the abbrev are too 
small, not

Iʼd drop the "too small,"

    Mathias> +above the threshold.  If you always want to get abbrev 
suggestions,
    Mathias> +set this variable to 0.
    Mathias> +
    Mathias> +@findex abbrev-suggest-show-report
    Mathias> +  The command @code{abbrev-suggest-show-report} can be used to 
show a
    Mathias> +buffer with all abbrev suggestions from the current editing 
session.
    Mathias> +This can be useful if you get several abbrev suggestions and don't
    Mathias> +remember them all.
    Mathias> +
    Mathias>  @node Editing Abbrevs
    Mathias>  @section Examining and Editing Abbrevs
 
    Mathias> diff --git a/etc/NEWS b/etc/NEWS
    Mathias> index 4076630..b4dc52a 100644
    Mathias> --- a/etc/NEWS
    Mathias> +++ b/etc/NEWS
    Mathias> @@ -1158,6 +1158,14 @@ messages, contain the error name of that 
message now.  They can be
    Mathias>  made visible by setting user variable 'dbus-show-dbus-errors' to
    Mathias>  non-nil, even if protected by 'dbus-ignore-errors' otherwise.
 
    Mathias> +** Abbrev mode
    Mathias> +
    Mathias> ++++
    Mathias> +*** A new user option, 'abbrev-suggest', enables the new abbrev
    Mathias> +suggestion feature.  When enabled, if a user manually type a 
piece of
    Mathias> +text that could have been written by using an abbrev, a hint will 
be
    Mathias> +displayed mentioning the abbrev that could have been used instead.
    Mathias> +

The first line should be a complete sentence, with details below, so:

"Abbrev can now suggest pre-existing abbreviations based on typed text."

    Mathias>
    Mathias>  * New Modes and Packages in Emacs 28.1
 
    Mathias> diff --git a/lisp/abbrev.el b/lisp/abbrev.el
    Mathias> index be6f9ee..a249dc0 100644
    Mathias> --- a/lisp/abbrev.el
    Mathias> +++ b/lisp/abbrev.el
    Mathias> @@ -824,6 +824,145 @@ abbrev-expand-function
    Mathias>    "Function that `expand-abbrev' uses to perform abbrev expansion.
    Mathias>  Takes no argument and should return the abbrev symbol if 
expansion took place.")
 
    Mathias> +(defcustom abbrev-suggest nil
    Mathias> +  "Non-nil means suggest abbrevs to the user.
    Mathias> +By enabling this option, if abbrev mode is enabled and if the
    Mathias> +user has typed some text that exists as an abbrev, suggest to the
    Mathias> +user to use the abbrev instead."

How is the suggestion displayed? Echo area, popup frame....?

    Mathias> +    :type 'boolean
    Mathias> +    :group 'abbrev-mode
    Mathias> +    :version "28.0")

"28.1"

    Mathias> +
    Mathias> +(defcustom abbrev-suggest-hint-threshold 3
    Mathias> +  "Threshold for when to inform the user that there is an abbrev.
    Mathias> +The threshold is the number of characters that differ between the
    Mathias> +length of the abbrev and the length of the expansion.  The
    Mathias> +thinking is that if the expansion is only one or a few characters
    Mathias> +longer than the abbrev, the benefit of informing the user is not
    Mathias> +that big.  If you always want to be informed, set this value to
    Mathias> +`0' or less.  This setting only applies if `abbrev-suggest' is
    Mathias> +non-nil."
    Mathias> +    :type 'number
    Mathias> +    :group 'abbrev-mode
    Mathias> +    :version "28.0")

And again. Plus the :group is derived from the last seen 'defgroup',
so itʼs optional here.

    Mathias> +
    Mathias> +(defun abbrev--suggest-get-active-tables-including-parents ()
    Mathias> +  "Return a list of all active abbrev tables, including parent 
tables."
    Mathias> +  (let* ((tables (abbrev--active-tables))
    Mathias> +   (all tables))

You've got indent-tabs-mode set to t, even though our .dir-locals.el
sets it to nil for elisp.

    Mathias> +    (dolist (table tables)
    Mathias> +      (setq all (append (abbrev-table-get table :parents) all)))
    Mathias> +    all))
    Mathias> +
    Mathias> +(defun abbrev--suggest-get-active-abbrev-expansions ()
    Mathias> +  "Return a list of all the active abbrev expansions.
    Mathias> +Includes expansions from parent abbrev tables."
    Mathias> +    (let (expansions)
    Mathias> +      (dolist (table 
(abbrev--suggest-get-active-tables-including-parents))
    Mathias> +  (mapatoms (lambda (e)
    Mathias> +              (let ((value (symbol-value (abbrev--symbol e 
table))))
    Mathias> +                (when value
    Mathias> +                        (push (cons value (symbol-name e)) 
expansions))))
    Mathias> +            table))
    Mathias> +      expansions))
    Mathias> +
    Mathias> +(defun abbrev--suggest-count-words (expansion)
    Mathias> +  "Return the number of words in EXPANSION.
    Mathias> +Expansion is a string of one or more words."
    Mathias> +    (length (split-string expansion " " t)))
    Mathias> +
    Mathias> +(defun abbrev--suggest-get-previous-words (n)
    Mathias> +  "Return the N words before point, spaces included.
    Mathias> +Changes newlines into spaces."

Well, anything with whitespace syntax. Iʼm not sure you need to
mention that.

    Mathias> +    (let ((end (point)))
    Mathias> +      (save-excursion
    Mathias> +  (backward-word n)
    Mathias> +  (replace-regexp-in-string
    Mathias> +   "\\s " " "
    Mathias> +   (buffer-substring-no-properties (point) end)))))
    Mathias> +
    Mathias> +(defun abbrev--suggest-above-threshold (expansion)
    Mathias> +  "Return non-nil if an abbrev in EXPANSION provides significant 
savings.

"the abbrev", no? I think EXPANSION only contains one abbrev.

    Mathias> +A significant saving, here, is the difference in length between
    Mathias> +the abbrev and the abbrev expansion.  EXPANSION is a cons cell
    Mathias> +where the car is the expansion and the cdr is the abbrev."
    Mathias> +    (>= (- (length (car expansion))
    Mathias> +     (length (cdr expansion)))
    Mathias> +  abbrev-suggest-hint-threshold))
    Mathias> +
    Mathias> +(defvar abbrev--suggest-saved-recommendations nil
    Mathias> +    "Keeps a list of expansions that have abbrevs defined.
    Mathias> +The user can show this list by calling
    Mathias> +`abbrev-suggest-show-report'.")
    Mathias> +
    Mathias> +(defun abbrev--suggest-inform-user (expansion)
    Mathias> +    "Display a message to the user about the existing abbrev.
    Mathias> +EXPANSION is a cons cell where the `car' is the expansion and the
    Mathias> +`cdr' is the abbrev."
    Mathias> +    (run-with-idle-timer
    Mathias> +     1 nil
    Mathias> +     (lambda ()
    Mathias> +       (message "You can write `%s' using the abbrev `%s'."
    Mathias> +                                   (car expansion) (cdr 
expansion))))

Iʼd put the abbrev first, and the expansion second. "abbrev `%s' can
be used to write `%s'."

    Mathias> +    (push expansion abbrev--suggest-saved-recommendations))
    Mathias> +
    Mathias> +(defun abbrev--suggest-shortest-abbrev (new current)
    Mathias> +    "Return the shortest abbrev of NEW and CURRENT.
    Mathias> +NEW and CURRENT are cons cells where the `car' is the expansion
    Mathias> +and the `cdr' is the abbrev."
    Mathias> +    (if (not current)
    Mathias> +  new
    Mathias> +      (if (< (length (cdr new))
    Mathias> +       (length (cdr current)))
    Mathias> +    new
    Mathias> +  current)))
    Mathias> +
    Mathias> +(defun abbrev--suggest-maybe-suggest ()
    Mathias> +    "Suggest an abbrev to the user based on the word(s) before 
point.
    Mathias> +Uses `abbrev-suggest-hint-threshold' to find out if the user 
should be
    Mathias> +informed about the existing abbrev."
    Mathias> +    (let (words abbrev-found word-count)
    Mathias> +      (dolist (expansion 
(abbrev--suggest-get-active-abbrev-expansions))
    Mathias> +  (setq word-count (abbrev--suggest-count-words (car expansion))
    Mathias> +        words (abbrev--suggest-get-previous-words word-count))
    Mathias> +  (let ((case-fold-search t))
    Mathias> +    (when (and (> word-count 0)

Personally Iʼm allergic to "(when (and", but opinions differ :-)

    Mathias> +               (string-match (car expansion) words)
    Mathias> +               (abbrev--suggest-above-threshold expansion))
    Mathias> +      (setq abbrev-found (abbrev--suggest-shortest-abbrev
    Mathias> +                          expansion abbrev-found)))))
    Mathias> +      (when abbrev-found
    Mathias> +  (abbrev--suggest-inform-user abbrev-found))))
    Mathias> +
    Mathias> +(defun abbrev--suggest-get-totals ()
    Mathias> +    "Return a list of all expansions and how many times they were 
used.
    Mathias> +Each expansion is a cons cell where the `car' is the expansion
    Mathias> +and the `cdr' is the number of times the expansion has been
    Mathias> +typed."
    Mathias> +    (let (total cell)
    Mathias> +      (dolist (expansion abbrev--suggest-saved-recommendations)
    Mathias> +  (if (not (assoc (car expansion) total))
    Mathias> +      (push (cons (car expansion) 1) total)
    Mathias> +    (setq cell (assoc (car expansion) total))
    Mathias> +    (setcdr cell (1+ (cdr cell)))))
    Mathias> +      total))
    Mathias> +
    Mathias> +(defun abbrev-suggest-show-report ()
    Mathias> +  "Show the user a report of abbrevs he could have used."
    Mathias> +  (interactive)
    Mathias> +  (let ((totals (abbrev--suggest-get-totals))
    Mathias> +  (buf (get-buffer-create "*abbrev-suggest*")))
    Mathias> +    (set-buffer buf)
    Mathias> +    (erase-buffer)
    Mathias> +        (insert "** Abbrev expansion usage **
    Mathias> +
    Mathias> +Below is a list of expansions for which abbrevs are defined, and
    Mathias> +the number of times the expansion was typed manually.  To display
    Mathias> +and edit all abbrevs, type `M-x edit-abbrevs RET'\n\n")
    Mathias> +  (dolist (expansion totals)
    Mathias> +    (insert (format " %s: %d\n" (car expansion) (cdr expansion))))
    Mathias> +  (display-buffer buf)))

`pop-to-buffer' rather than `set-buffer' + `display-buffer'?

    Mathias> +
    Mathias>  (defun expand-abbrev ()
    Mathias>    "Expand the abbrev before point, if there is an abbrev there.
    Mathias>  Effective when explicitly called even when `abbrev-mode' is nil.
    Mathias> @@ -831,7 +970,9 @@ expand-abbrev
    Mathias>  the work, and returns whatever it does.  (That return value should
    Mathias>  be the abbrev symbol if expansion occurred, else nil.)"
    Mathias>    (interactive)
    Mathias> -  (funcall abbrev-expand-function))
    Mathias> +  (or (funcall abbrev-expand-function)
    Mathias> +      (if abbrev-suggest
    Mathias> +          (abbrev--suggest-maybe-suggest))))
 
    Mathias>  (defun abbrev--default-expand ()
    Mathias>    "Default function to use for `abbrev-expand-function'.
    Mathias> -- 
    Mathias> 1.9.1