Re: [PATCH] Add abbrev suggestions

[Mathias Dahl <mathias.dahl@gmail.com>]

[-- Attachment #1.1: Type: text/plain, Size: 2767 bytes --]

Hi Eli,

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

Keeping my fingers crossed...

/Mathias


On Thu, Aug 13, 2020 at 4:29 PM Mathias Dahl <mathias.dahl@gmail.com> wrote:

> > > No, I'm not. In fact, it was not the default when I started
> > > working on this, but Stefan suggested that it might be a good
> > > default. Now we're me and you against him, I guess... :)
>
> > Let's start with having it opt-in.  We can later see if it is
> > popular enough to become the default.
>
> Fine by me.
>
> > How about the below?
>
> > Return non-nil if an abbrev in EXPANSION provides significant
> > savings.
>
> Hey, that's cheating! :)
>
> I prepared a new version myself too. Will see which one I select when
> I send a new patch...
>
> > > Should I include those changes in the same patch and resend
> > > that when done?
>
> > Yes, please.
>
> Alrighty then!
>
> Thanks!
>
> /Mathias
>
>
> On Thu, Aug 13, 2020 at 3:59 PM Eli Zaretskii <eliz@gnu.org> wrote:
>
>> > From: Mathias Dahl <mathias.dahl@gmail.com>
>> > Date: Wed, 12 Aug 2020 00:16:33 +0200
>> > Cc: emacs-devel@gnu.org
>> >
>> > > Are you sure it is a good idea to make this non-nil by default?
>> > > Wouldn't some users consider these suggestions an annoyance?
>> >
>> > No, I'm not. In fact, it was not the default when I started working on
>> > this, but Stefan suggested that it might be a good default. Now we're
>> > me and you against him, I guess... :)
>>
>> Let's start with having it opt-in.  We can later see if it is popular
>> enough to become the default.
>>
>> > > > +(defun abbrev--suggest-above-threshold (expansion)
>> > > > +    "Return t if we are above the threshold.
>> > >
>> > > Who is "we" in this context?  This should be explained.
>> >
>> > I know, I was not happy when I wrote that. "we", here, is something
>> > like "the difference in length between what the user typed and the
>> > abbrev that we found." I guess I could not find a good way to keep the
>> > first sentence of the docstring short, so I opted for the fuzzy "we"
>> > expression...
>>
>> How about the below?
>>
>>   Return non-nil if an abbrev in EXPANSION provides significant savings.
>>
>> > > > +EXPANSION is a cons cell where the car is the expansion and the
>> > > > +cdr is the abbrev."
>> > >
>> > > Our style is to include the arguments in the first sentence of the doc
>> > > string.
>> >
>> > I know. Frankly I don't know if I can come up with a suggestion that
>> > combines that together with having a relatively short first
>> > sentence...
>>
>> See above.
>>
>> > Should I include those changes in the same patch and resend
>> > that when done?
>>
>> Yes, please.
>>
>

[-- Attachment #1.2: Type: text/html, Size: 3994 bytes --]

[-- Attachment #2: 0001-Add-abbrev-suggestions-take-two.patch --]
[-- Type: application/octet-stream, Size: 10710 bytes --]

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

    Add abbrev suggestions

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

diff --git a/doc/emacs/abbrevs.texi b/doc/emacs/abbrevs.texi
index 21bf8c5..fbe7117 100644
--- a/doc/emacs/abbrevs.texi
+++ b/doc/emacs/abbrevs.texi
@@ -28,6 +28,7 @@ Abbrevs
 * Abbrev Concepts::   Fundamentals of defined abbrevs.
 * Defining Abbrevs::  Defining an abbrev, so it will expand when typed.
 * Expanding Abbrevs:: Controlling expansion: prefixes, canceling expansion.
+* Abbrevs Suggestions:: Get suggestions about defined abbrevs.
 * Editing Abbrevs::   Viewing or editing the entire list of defined abbrevs.
 * Saving Abbrevs::    Saving the entire list of abbrevs for another session.
 * Dynamic Abbrevs::   Abbreviations for words already in the buffer.
@@ -223,6 +224,38 @@ Expanding Abbrevs
 the abbrev expansion.  @xref{Abbrev Expansion,,, elisp, The Emacs Lisp
 Reference Manual}.
 
+@node Abbrev Suggestions
+@section Abbrev Suggestions
+
+  You can get abbrev suggestions when you manually type text for which
+there is currently an active defined abbrev.  For example, if there is
+an abbrev @samp{foo} with the expansion @samp{find outer otter}, and
+you manually type @samp{find outer otter}, the abbrev suggestion
+feature will notice this and show a hint in the echo when you have
+stop typing.
+
+@vindex abbrev-suggest
+  Enable the abbrev suggestion feature by setting the variable
+@code{abbrev-suggest} to @code{t}.
+
+@vindex abbrev-suggest-hint-threshold
+  The variable @code{abbrev-suggest-hint-threshold} controls when to
+suggest an abbrev to the user.  The variable defines the number of
+characters that the user must save in order to get a suggestion.  For
+example, if the user types @samp{foo bar} (seven characters) and there
+is an abbrev @samp{fubar} defined (five characters), the user will not
+get any suggestion unless the threshold is set to the number 2 or
+lower.  With the default value 3, the user would not get any
+suggestion, because the savings in using the abbrev are too small, not
+above the threshold.  If you always want to get abbrev suggestions,
+set this variable to 0.
+
+@findex abbrev-suggest-show-report
+  The command @code{abbrev-suggest-show-report} can be used to show a
+buffer with all abbrev suggestions from the current editing session.
+This can be useful if you get several abbrev suggestions and don't
+remember them all.
+
 @node Editing Abbrevs
 @section Examining and Editing Abbrevs
 
diff --git a/etc/NEWS b/etc/NEWS
index 4076630..b4dc52a 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -1158,6 +1158,14 @@ messages, contain the error name of that message now.  They can be
 made visible by setting user variable 'dbus-show-dbus-errors' to
 non-nil, even if protected by 'dbus-ignore-errors' otherwise.
 
+** Abbrev mode
+
++++
+*** A new user option, 'abbrev-suggest', enables the new abbrev
+suggestion feature.  When enabled, if a user manually type a piece of
+text that could have been written by using an abbrev, a hint will be
+displayed mentioning the abbrev that could have been used instead.
+
 \f
 * New Modes and Packages in Emacs 28.1
 
diff --git a/lisp/abbrev.el b/lisp/abbrev.el
index be6f9ee..a249dc0 100644
--- a/lisp/abbrev.el
+++ b/lisp/abbrev.el
@@ -824,6 +824,145 @@ abbrev-expand-function
   "Function that `expand-abbrev' uses to perform abbrev expansion.
 Takes no argument and should return the abbrev symbol if expansion took place.")
 
+(defcustom abbrev-suggest nil
+  "Non-nil means suggest abbrevs to the user.
+By enabling this option, if abbrev mode is enabled and if the
+user has typed some text that exists as an abbrev, suggest to the
+user to use the abbrev instead."
+    :type 'boolean
+    :group 'abbrev-mode
+    :version "28.0")
+
+(defcustom abbrev-suggest-hint-threshold 3
+  "Threshold for when to inform the user that there is an abbrev.
+The threshold is the number of characters that differ between the
+length of the abbrev and the length of the expansion.  The
+thinking is that if the expansion is only one or a few characters
+longer than the abbrev, the benefit of informing the user is not
+that big.  If you always want to be informed, set this value to
+`0' or less.  This setting only applies if `abbrev-suggest' is
+non-nil."
+    :type 'number
+    :group 'abbrev-mode
+    :version "28.0")
+
+(defun abbrev--suggest-get-active-tables-including-parents ()
+  "Return a list of all active abbrev tables, including parent tables."
+  (let* ((tables (abbrev--active-tables))
+	 (all tables))
+    (dolist (table tables)
+      (setq all (append (abbrev-table-get table :parents) all)))
+    all))
+
+(defun abbrev--suggest-get-active-abbrev-expansions ()
+  "Return a list of all the active abbrev expansions.
+Includes expansions from parent abbrev tables."
+    (let (expansions)
+      (dolist (table (abbrev--suggest-get-active-tables-including-parents))
+	(mapatoms (lambda (e)
+		    (let ((value (symbol-value (abbrev--symbol e table))))
+		      (when value
+                        (push (cons value (symbol-name e)) expansions))))
+		  table))
+      expansions))
+
+(defun abbrev--suggest-count-words (expansion)
+  "Return the number of words in EXPANSION.
+Expansion is a string of one or more words."
+    (length (split-string expansion " " t)))
+
+(defun abbrev--suggest-get-previous-words (n)
+  "Return the N words before point, spaces included.
+Changes newlines into spaces."
+    (let ((end (point)))
+      (save-excursion
+	(backward-word n)
+	(replace-regexp-in-string
+	 "\\s " " "
+	 (buffer-substring-no-properties (point) end)))))
+
+(defun abbrev--suggest-above-threshold (expansion)
+  "Return non-nil if an abbrev in EXPANSION provides significant savings.
+A significant saving, here, is the difference in length between
+the abbrev and the abbrev expansion.  EXPANSION is a cons cell
+where the car is the expansion and the cdr is the abbrev."
+    (>= (- (length (car expansion))
+	   (length (cdr expansion)))
+	abbrev-suggest-hint-threshold))
+
+(defvar abbrev--suggest-saved-recommendations nil
+    "Keeps a list of expansions that have abbrevs defined.
+The user can show this list by calling
+`abbrev-suggest-show-report'.")
+
+(defun abbrev--suggest-inform-user (expansion)
+    "Display a message to the user about the existing abbrev.
+EXPANSION is a cons cell where the `car' is the expansion and the
+`cdr' is the abbrev."
+    (run-with-idle-timer
+     1 nil
+     (lambda ()
+       (message "You can write `%s' using the abbrev `%s'."
+                                   (car expansion) (cdr expansion))))
+    (push expansion abbrev--suggest-saved-recommendations))
+
+(defun abbrev--suggest-shortest-abbrev (new current)
+    "Return the shortest abbrev of NEW and CURRENT.
+NEW and CURRENT are cons cells where the `car' is the expansion
+and the `cdr' is the abbrev."
+    (if (not current)
+	new
+      (if (< (length (cdr new))
+	     (length (cdr current)))
+	  new
+	current)))
+
+(defun abbrev--suggest-maybe-suggest ()
+    "Suggest an abbrev to the user based on the word(s) before point.
+Uses `abbrev-suggest-hint-threshold' to find out if the user should be
+informed about the existing abbrev."
+    (let (words abbrev-found word-count)
+      (dolist (expansion (abbrev--suggest-get-active-abbrev-expansions))
+	(setq word-count (abbrev--suggest-count-words (car expansion))
+	      words (abbrev--suggest-get-previous-words word-count))
+	(let ((case-fold-search t))
+	  (when (and (> word-count 0)
+		     (string-match (car expansion) words)
+		     (abbrev--suggest-above-threshold expansion))
+	    (setq abbrev-found (abbrev--suggest-shortest-abbrev
+				expansion abbrev-found)))))
+      (when abbrev-found
+	(abbrev--suggest-inform-user abbrev-found))))
+
+(defun abbrev--suggest-get-totals ()
+    "Return a list of all expansions and how many times they were used.
+Each expansion is a cons cell where the `car' is the expansion
+and the `cdr' is the number of times the expansion has been
+typed."
+    (let (total cell)
+      (dolist (expansion abbrev--suggest-saved-recommendations)
+	(if (not (assoc (car expansion) total))
+	    (push (cons (car expansion) 1) total)
+	  (setq cell (assoc (car expansion) total))
+	  (setcdr cell (1+ (cdr cell)))))
+      total))
+
+(defun abbrev-suggest-show-report ()
+  "Show the user a report of abbrevs he could have used."
+  (interactive)
+  (let ((totals (abbrev--suggest-get-totals))
+	(buf (get-buffer-create "*abbrev-suggest*")))
+    (set-buffer buf)
+    (erase-buffer)
+        (insert "** Abbrev expansion usage **
+
+Below is a list of expansions for which abbrevs are defined, and
+the number of times the expansion was typed manually.  To display
+and edit all abbrevs, type `M-x edit-abbrevs RET'\n\n")
+	(dolist (expansion totals)
+	  (insert (format " %s: %d\n" (car expansion) (cdr expansion))))
+	(display-buffer buf)))
+
 (defun expand-abbrev ()
   "Expand the abbrev before point, if there is an abbrev there.
 Effective when explicitly called even when `abbrev-mode' is nil.
@@ -831,7 +970,9 @@ expand-abbrev
 the work, and returns whatever it does.  (That return value should
 be the abbrev symbol if expansion occurred, else nil.)"
   (interactive)
-  (funcall abbrev-expand-function))
+  (or (funcall abbrev-expand-function)
+      (if abbrev-suggest
+          (abbrev--suggest-maybe-suggest))))
 
 (defun abbrev--default-expand ()
   "Default function to use for `abbrev-expand-function'.
-- 
1.9.1