From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.io!.POSTED.ciao.gmane.io!not-for-mail From: Mathias Dahl Newsgroups: gmane.emacs.devel Subject: Re: Abbrev suggestions - feedback appreciated Date: Mon, 11 May 2020 23:37:26 +0200 Message-ID: References: <871smeoalc.fsf@gnu.org> <87fuatmw71.fsf@gnu.org> Mime-Version: 1.0 Content-Type: multipart/alternative; boundary="000000000000e2c97005a566279b" Injection-Info: ciao.gmane.io; posting-host="ciao.gmane.io:159.69.161.202"; logging-data="5825"; mail-complaints-to="usenet@ciao.gmane.io" To: monnier@iro.umontreal.ca, emacs-devel@gnu.org Original-X-From: emacs-devel-bounces+ged-emacs-devel=m.gmane-mx.org@gnu.org Mon May 11 23:38:37 2020 Return-path: Envelope-to: ged-emacs-devel@m.gmane-mx.org Original-Received: from lists.gnu.org ([209.51.188.17]) by ciao.gmane.io with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1jYG8D-0001PK-Cr for ged-emacs-devel@m.gmane-mx.org; Mon, 11 May 2020 23:38:37 +0200 Original-Received: from localhost ([::1]:51942 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1jYG8C-0007VF-F4 for ged-emacs-devel@m.gmane-mx.org; Mon, 11 May 2020 17:38:36 -0400 Original-Received: from eggs.gnu.org ([2001:470:142:3::10]:43202) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1jYG7J-0006AI-V2 for emacs-devel@gnu.org; Mon, 11 May 2020 17:37:41 -0400 Original-Received: from mail-ua1-x936.google.com ([2607:f8b0:4864:20::936]:43437) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1jYG7H-0003U7-S0 for emacs-devel@gnu.org; Mon, 11 May 2020 17:37:41 -0400 Original-Received: by mail-ua1-x936.google.com with SMTP id u12so3992680uau.10 for ; Mon, 11 May 2020 14:37:38 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to; bh=Sje5m8cAIhzYTsIXFisX7m1TBAm75OwFSg41XaHcBKI=; b=RB1bOh/C8o3k5qHtDlhQ0KcCY51xS7eZcaRvunRfqvIAMMSvqmwBL6yxz3ApunnIqJ iF4ROZUQWQcdwH1xmjwAQeBWVDBUACrNYb0/R9futPsQS40hpNEO792ahOZxqCM/2ExW vb7Qrdjc0TVTwfIuYl4ZWoSf6gr7kMIvFcNkv8y0CI0P42ogSKKBHPUhY8zuOgpuxNpL sb316CXQFR0MsWoJL5oGyZQHYGizmYnJjHbYENgZNg8GBB123/SqiqcQ2M+zgacUUt30 FpV7SkisfPQzheiuI6OwPmbnnbqQIDq472jIgRImS3WJY5njs+VaxJJ9jKYm8cn1ZcPl eBTw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to; bh=Sje5m8cAIhzYTsIXFisX7m1TBAm75OwFSg41XaHcBKI=; b=ByKyHeLl2o29KuXTvx135fIFfXCgMtxPcG+bB0cMo/6HQ1hzOzEW3N1PDpy30quuy4 XtElElpdBgHHfgfQpfzuH3HI9b5vQkRDrtcc5mpVsk8+iZk/cwBPnpLDcHpwS1JOjksr MBY8TAtuXD+18W1Na28J6IEasm0n4/wgFCYYxIwmr1+2KWl3SmnarpL5A0mFopBxaTwj VR5qoAu2zc6VMwvzYZhaNMdv8x7YwZdVQ518oqrZIY5hTnYdVPO7QIgaf3IfDNF06HT4 NoQbz1T1WtGPcTbEuCfdumss0y44WJgkQ9u5g4kFH77J1x4iM2aWWJgFu6Sm3YB3+FgQ dmuw== X-Gm-Message-State: AGi0PuaomtSJbOAX/Ca+BZxWdCAzEMfxDOi4phvZKQlolRZO6UOuZMB8 5R7IuKa4erCLG2VKq9tcfRy7s6mpIAmAgYdV4TIeYcKZlk4= X-Google-Smtp-Source: APiQypK2Yyaz8rWn9o6ncRYKulcQoYrAGeanb1O3HCmNVyuJLsD3l67qUaxg13lcTbJVCTncMmBZGJM1Rqxvw2RD1p8= X-Received: by 2002:ab0:20ce:: with SMTP id z14mr13938076ual.137.1589233057972; Mon, 11 May 2020 14:37:37 -0700 (PDT) In-Reply-To: Received-SPF: pass client-ip=2607:f8b0:4864:20::936; envelope-from=mathias.dahl@gmail.com; helo=mail-ua1-x936.google.com X-detected-operating-system: by eggs.gnu.org: No matching host in p0f cache. That's all we know. X-Spam_score_int: -20 X-Spam_score: -2.1 X-Spam_bar: -- X-Spam_report: (-2.1 / 5.0 requ) BAYES_00=-1.9, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, FREEMAIL_FROM=0.001, HTML_MESSAGE=0.001, RCVD_IN_DNSWL_NONE=-0.0001, SPF_PASS=-0.001, URIBL_BLOCKED=0.001 autolearn=_AUTOLEARN X-Spam_action: no action X-BeenThere: emacs-devel@gnu.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: "Emacs development discussions." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: emacs-devel-bounces+ged-emacs-devel=m.gmane-mx.org@gnu.org Original-Sender: "Emacs-devel" Xref: news.gmane.io gmane.emacs.devel:249896 Archived-At: --000000000000e2c97005a566279b Content-Type: text/plain; charset="UTF-8" Hi Stefan, Thanks for your feedback. Now, one and a half year later I started working on this again :) Still think it is a great idea to have this as part of the abbrev concept. On Tue, Sep 18, 2018 at 4:06 AM Stefan Monnier wrote: > [ Note: your patch seems to have its TABs converted into SPCs, messing > up indentation. I tried to fix those when quoting your code, but > I may have mishandled some parts. ] > I'll have a look at that at my next attempt. Are you saying there should be TABs in Emacs' .el files? > +(defcustom abbrev-suggest nil > > + "Non-nil means we should 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 > > + :group 'convenience) > > Does it really need to be in `convenience` as well? > Looking at the other defcustoms in the same file, probably not. I guess I added that when I first wrote absug.el, thinking it was a convenience thing. To be honest I think abbrevs are a convenience thing, but I will not fight for tagging it as such... > What would be the disadvantages/risks of enabling it by default? > Of the top of my head, some people might be annoyed by the constant "nagging" about using abbrevs. They could of course turn it off, if we enable it by default. Since I personally like it I would not bother keeping it on and perhaps new users would like it too. The other disadvantage could be performance, but we have discussed that already and perhaps it will never be a problem, not sure. By the way, here are the two top defcustoms in abbrev.el: (defcustom abbrev-file-name (locate-user-emacs-file "abbrev_defs" ".abbrev_defs") "Default name of file from which to read abbrevs." :initialize 'custom-initialize-delay :type 'file) (defcustom only-global-abbrevs nil "Non-nil means user plans to use global abbrevs only. This makes the commands that normally define mode-specific abbrevs define global abbrevs instead." :type 'boolean :group 'abbrev-mode :group 'convenience) The first does not have a group set and the second has convenience as group as well. I have never thought about if this is a normal or not. Is it? > > -(defvar abbrev-expand-function #'abbrev--default-expand > > - "Function that `expand-abbrev' uses to perform abbrev expansion. > > +(defvar abbrev-expand-function #'abbrev--try-expand-maybe-suggest > > + "Function that `expand-abbrev' uses to perform abbrev expansion. > [...] > > +(defun abbrev--try-expand-maybe-suggest () > > + "Try to expand abbrev, look for suggestions if enabled. > > +This is set as `abbrev-expand-function'. If no abbrev expansion > > +is found by `abbrev--default-expand', see if there is an abbrev > > +defined for the word before point, and suggest it to the user." > > + (or (abbrev--default-expand) > > + (if abbrev-suggest > > + (abbrev-suggest-maybe-suggest) > > + nil))) > > If you put the code directly into abbrev.el, then I think you may as > well modify expand-abbrev rather than hooking into abbrev-expand-function. > Yes, I was looking for other places to put this, but it felt better to me to have the suggestions come at the end of the existing code, when abbrevs was tried and not found. Also, I was worried to mess with what is said here: (defun abbrev--default-expand () "Default function to use for `abbrev-expand-function'. This also respects the obsolete wrapper hook `abbrev-expand-functions'. \(See `with-wrapper-hook' for details about wrapper hooks.) Calls `abbrev-insert' to insert any expansion, **and returns what it does.**" <==== (subr--with-wrapper-hook-no-warnings abbrev-expand-functions () (pcase-let ((`(,sym ,name ,wordstart ,wordend) (abbrev--before-point))) ... That is mentioned in other places too, that it should return what was inserted, if any. It did not feel right to mess with that, but perhaps it is okay... Here is an attempt: (defun expand-abbrev () "Expand the abbrev before point, if there is an abbrev there. Effective when explicitly called even when `abbrev-mode' is nil. Before doing anything else, runs `pre-abbrev-expand-hook'. Calls the value of `abbrev-expand-function' with no argument to do the work, and returns whatever it does. (That return value should be the abbrev symbol if expansion occurred, else nil.)" (interactive) (run-hooks 'pre-abbrev-expand-hook) (or (funcall abbrev-expand-function) (abbrev-suggest-maybe-suggest))) Is something like that what you had in mind? > The rest of the code defines abbrev--suggest-maybe-suggest rather than > abbrev-suggest-maybe-suggest. > Seems to be a search and replace-mistake :) > > +(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 differs 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." > > + :type 'number > > + :group 'abbrev-mode > > + :group 'convenience) > > Do we really need to add it to `convenience`? > Just as above, I'd recommend you just remove both :group keywords and > let the default behavior kick in (which will put it into `abbrev-mode`). > Sure, I'm fine with that. > > +(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 > > I think you'd be better served defining a dolist-style macro or > a mapc-style function to loop over all abbrevs without creating an > intermediate list. > Sorry, I don't understand what you are suggesting, and why (performance, or saving memory?). > > + (setq expansions > > + (cons (cons value (symbol-name e)) > > + expansions))))) > > Aka (push (cons value (symbol-name e)) expansions)))) > You learn something new every day :) I'll try that out. > > +(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))) > > Why does your code pay attention to words? > I'll think about that... Are you thinking that only characters matter? I think my focus is primarily using abbrevs for sentences. I also have some abbrevs that are only short forms for longer words, but I think I feel that I get more benefit from sentence-like abbrevs... Again, I will think about this and see if words are what I want to pay attention to. > +(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)))) > > Please don't quote your lambdas. `abbrev.el` uses lexical-binding, so > you can just write > > (run-with-idle-timer > 1 nil > (lambda () > (message "You can write `%s' using the abbrev `%s'." > (car expansion) (cdr expansion)))) > Got it! > > + (setq abbrev--suggest-saved-recommendations > > + (cons expansion abbrev--suggest-saved-recommendations))) > > Aka (push expansion abbrev--suggest-saved-recommendations)) > Changed it. > BTW, won't this list contain repetitions? > Yes it will, and it should. Or, at least I am doing that now to show the user a count of how many times an expansion was typed manually. Have a look at `abbrev--suggest-get-totals'. > Maybe you should use `add-to-list` or `cl-pushnew` instead? > > One more thing: I think it'd be even better to put abbrev objects (which > are implemented as symbols) in there, so you have easy accesss to > a more info than just the expansion. > I have access to the expansion as well as the abbrev, and this is now added to the optional report shown to the user. Not sure what I need more... > +(defun abbrev--suggest-shortest-abbrev (new current) > > + "Return the shortest abbrev. > > +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))) > > Maybe rather than the shortest abbrev, it would be better to choose the > abbrev with the largest difference between abbrev and expansion. > It sounds like an interesting idea. I will try it out. > > +(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)) > > Why not use something like > > (buffer-substring (- (point) (length (car expansion))) (point))? > > and when it's equal to (car expansion), then maybe check that there's > a word boundary? > I'll experiment with not thinking about words. I think I cannot just compare things like that because of newlines, but that can be handled since I did that in my "word-based code". > > + (let ((case-fold-search t)) > > Some abbrevs are case-sensitive. > Okay... Will think about that too. Not sure if I added that because I actually needed it to solve a problem, or if I was proactive and did not need to be... > > + (when (and (> word-count 0) > > + (string-match (car expansion) words) > > (car expansion) is a string, not a regular expression, so you'd need to > regexp-quote it before passing it to string-match. > Okay, thanks! > > + (abbrev--suggest-above-threshold expansion)) > > + (setq abbrev-found (abbrev--suggest-shortest-abbrev > > + expansion abbrev-found))))) > > + (when abbrev-found > > + (abbrev--suggest-inform-user abbrev-found)))) > > I suspect that abbrev--suggest-above-threshold will eliminate a fairly > large number of abbrevs for some users (e.g. those using abbrevs to fix > recurring typos) and it's a cheap test which doesn't need to allocate > any memory, so I'd recommend performing it before the calls to > abbrev--suggest-count-words and abbrev--suggest-get-previous-words. > Will try that. > +(defun abbrev-suggest-try-expand-maybe-suggest () > > + "Try to expand abbrev, look for suggestions of no abbrev found. > > +This is the main entry to the abbrev suggest mechanism. This is > > +set as the default value for `abbrev-expand-function'. If no > > +abbrev expansion is found by `abbrev--default-expand', see if > > +there is an abbrev defined for the word before point, and suggest > > +it to the user." > > + (or (abbrev--default-expand) > > + (if abbrev-suggest > > + (abbrev-suggest-maybe-suggest)))) > > Looks identical to the earlier abbrev--try-expand-maybe-suggest > Yes, it was a leftover. I'll try to cook a new version now. Let's see if it takes another two years :) Thanks for all the suggestions and time spent on this. /Mathias --000000000000e2c97005a566279b Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable
Hi Stefan,

Thanks for your feedback. Now, one = and a half year later I started working on this again :) Still think it is = a great idea to have this as part of the abbrev concept.

On Tue, Sep 18, 2018 at 4:06 AM Stefan Mo= nnier <mon= nier@iro.umontreal.ca> wrote:
[ Note: your patch seems to have its TABs converted in= to SPCs, messing
=C2=A0 up indentation.=C2=A0 I tried to fix those when quoting your code, b= ut
=C2=A0 I may have mishandled some parts.=C2=A0 ]

<= /div>
I'll have a look at that at my=C2=A0next attempt. Are you say= ing there should be TABs in Emacs' .el files?

> +(defcustom abbrev-suggest nil
> +=C2=A0 "Non-nil means we should 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."
> +=C2=A0 :type 'boolean
> +=C2=A0 :group 'abbrev-mode
> +=C2=A0 :group 'convenience)

Does it really need to be in `convenience` as well?
Looking at the other defcustoms in the same file, probably not= . I guess I added that when I first wrote absug.el, thinking it was a conve= nience thing. To be honest I think abbrevs are a convenience thing, but I w= ill not fight for tagging it as such...
=C2=A0
What would be the disadvantages/risks of enabling it by default?

Of the top of my head, some people might be annoy= ed by the constant "nagging" about using abbrevs. They could of c= ourse turn it off, if we enable it by default. Since I personally like it I= would not bother keeping it on and perhaps new users would like it too. Th= e other disadvantage could be performance, but we have discussed that alrea= dy and perhaps it will never be a problem, not sure.

By the way, here are the two top defcustoms in abbrev.el:

=
(defcustom abbrev-file-name
=C2=A0 (locate-user-e= macs-file "abbrev_defs" ".abbrev_defs")
=C2= =A0 "Default name of file from which to read abbrevs."
= =C2=A0 :initialize 'custom-initialize-delay
=C2=A0 :type '= ;file)

(defcustom only-global-abbrevs nil
=C2=A0 "Non-nil means user plans to use global abbrevs only.
This makes the commands that normally define mode-specific abbrevs
=
define global abbrevs instead."
=C2=A0 :type 'boole= an
=C2=A0 :group 'abbrev-mode
=C2=A0 :group 'co= nvenience)

The first does not have a group s= et and the second has convenience as group as well. I have never thought ab= out if this is a normal or not. Is it?
=C2=A0
> -(defvar abbrev-expand-function #'abbrev--default-expand
> -=C2=A0 "Function that `expand-abbrev' uses to perform abbrev= expansion.
> +(defvar abbrev-expand-function #'abbrev--try-expand-maybe-suggest=
> +=C2=A0 =C2=A0 "Function that `expand-abbrev' uses to perform= abbrev expansion.
[...]
> +(defun abbrev--try-expand-maybe-suggest ()
> +=C2=A0 "Try to expand abbrev, look for suggestions if enabled. > +This is set as `abbrev-expand-function'.=C2=A0 If no abbrev expan= sion
> +is found by `abbrev--default-expand', see if there is an abbrev > +defined for the word before point, and suggest it to the user."<= br> > +=C2=A0 (or (abbrev--default-expand)
> +=C2=A0 =C2=A0 =C2=A0 (if abbrev-suggest
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 (abbrev-suggest-maybe-suggest)
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 nil)))

If you put the code directly into abbrev.el, then I think you may as
well modify expand-abbrev rather than hooking into abbrev-expand-function.<= br>

Yes, I was looking for other places to = put this, but it felt better to me to have the suggestions come at the end = of the existing code, when abbrevs was tried and not found. Also, I was wor= ried to mess with what is said here:

(defun a= bbrev--default-expand ()
=C2=A0 "Default function to use for= `abbrev-expand-function'.
This also respects the obsolete wr= apper hook `abbrev-expand-functions'.
\(See `with-wrapper-hoo= k' for details about wrapper hooks.)
Calls `abbrev-insert'= ; to insert any expansion, *and returns what it does.*" <=3D= =3D=3D=3D
=C2=A0 (subr--with-wrapper-hook-no-warnings abbrev-expa= nd-functions ()
=C2=A0 =C2=A0 (pcase-let ((`(,sym ,name ,wordstar= t ,wordend) (abbrev--before-point)))
=C2=A0 =C2=A0 ...

That is mentioned in other places too, that it should = return what was inserted, if any. It did not feel right to mess with that, = but perhaps it is okay... Here is an attempt:

(defun expand-abbrev ()
=C2=A0 "Expand the abbrev before po= int, if there is an abbrev there.
Effective when explicitly calle= d even when `abbrev-mode' is nil.
Before doing anything else,= runs `pre-abbrev-expand-hook'.
Calls the value of `abbrev-ex= pand-function' with no argument to do
the work, and returns w= hatever it does.=C2=A0 (That return value should
be the abbrev sy= mbol if expansion occurred, else nil.)"
=C2=A0 (interactive)=
=C2=A0 (run-hooks 'pre-abbrev-expand-hook)
=C2=A0 = (or (funcall abbrev-expand-function)
=C2=A0 =C2=A0 =C2=A0 (abbrev= -suggest-maybe-suggest)))

Is something like = that what you had in mind?
=C2=A0
The rest of the code defines abbrev--suggest-maybe-suggest rather than
abbrev-suggest-maybe-suggest.

Seems to = be a search and replace-mistake :)
=C2=A0
> +(defcustom abbrev-suggest-hint-threshold 3
> +=C2=A0 "Threshold for when to inform the user that there is an a= bbrev.
> +The threshold is the number of characters that differs between
> +the length of the abbrev and the length of the expansion.=C2=A0 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.=C2=A0 If you always want to be informed, set this value to<= br> > +`0' or less."
> +=C2=A0 :type 'number
> +=C2=A0 :group 'abbrev-mode
> +=C2=A0 :group 'convenience)

Do we really need to add it to `convenience`?
Just as above, I'd recommend you just remove both :group keywords and let the default behavior kick in (which will put it into `abbrev-mode`).

Sure, I'm fine with that.
= =C2=A0
> +(defun abbrev--suggest-get-active-abbrev-expansions ()
> +=C2=A0 "Return a list of all the active abbrev expansions.
> +Includes expansions from parent abbrev tables."
> +=C2=A0 (let (expansions)
> +=C2=A0 =C2=A0 (dolist (table (abbrev--suggest-get-active-tables-inclu= ding-parents))
> +=C2=A0 =C2=A0 =C2=A0 (mapatoms (lambda (e)
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 (let (= (value (symbol-value (abbrev--symbol e table))))
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= (when value

I think you'd be better served defining a dolist-style macro or
a mapc-style function to loop over all abbrevs without creating an
intermediate list.

Sorry, I don't u= nderstand what you are suggesting, and why (performance, or saving memory?)= .
=C2=A0
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0 (setq expansions
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0 =C2=A0 =C2=A0 =C2=A0 (cons (cons value (symbol-name e))
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 expansions)))))

Aka=C2=A0 =C2=A0(push (cons value (symbol-name e)) expansions))))

You learn something new every day :) I'll tr= y that out.
=C2=A0
> +(defun abbrev--suggest-count-words (expansion)
> +=C2=A0 =C2=A0 "Return the number of words in EXPANSION.
> +Expansion is a string of one or more words."
> +=C2=A0 =C2=A0 (length (split-string expansion " " t)))

Why does your code pay attention to words?

<= div>I'll think about that... Are you thinking that only characters matt= er? I think my focus is primarily using abbrevs for sentences. I also have = some abbrevs that are only short forms for longer words, but I think I feel= that I get more benefit from sentence-like abbrevs... Again, I will think = about this and see if words are what I want to pay attention to.
=
> +(defun abbrev--suggest-inform-user (expansion)
> +=C2=A0 "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."
> +=C2=A0 (run-with-idle-timer
> +=C2=A0 =C2=A01 nil
> +=C2=A0 =C2=A0`(lambda ()
> +=C2=A0 =C2=A0 =C2=A0 (message "You can write `%s' using the = abbrev `%s'."
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0,(car expansio= n) ,(cdr expansion))))

Please don't quote your lambdas.=C2=A0 `abbrev.el` uses lexical-binding= , so
you can just write

=C2=A0 =C2=A0 =C2=A0 =C2=A0(run-with-idle-timer
=C2=A0 =C2=A0 =C2=A0 =C2=A0 1 nil
=C2=A0 =C2=A0 =C2=A0 =C2=A0 (lambda ()
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 (message "You can write `%s' us= ing the abbrev `%s'."
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0(car e= xpansion) (cdr expansion))))

Got it!
=C2=A0
> +=C2=A0 =C2=A0 (setq abbrev--suggest-saved-recommendations
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 (cons expansion abbrev--suggest-sa= ved-recommendations)))

Aka=C2=A0 (push expansion abbrev--suggest-saved-recommendations))

Changed it.=C2=A0
=C2=A0
BTW, won't this list contain repetitions?

Yes it will, and it should. Or, at least I am doing that now to show= the user a count of how many times an expansion was typed manually. Have a= look at `abbrev--suggest-get-totals'.
=C2=A0
Maybe you should use `add-to-list` or `cl-pushnew` instead?

One more thing: I think it'd be even better to put abbrev objects (whic= h
are implemented as symbols) in there, so you have easy accesss to
a more info than just the expansion.

I = have access to the expansion as well as the abbrev, and this is now added t= o the optional report shown to the user. Not sure what I need more...
=

> +(defun abbrev--suggest-shortest-abbrev (new current)
> +=C2=A0 =C2=A0 "Return the shortest abbrev.
> +NEW and CURRENT are cons cells where the `car' is the expansion > +and the `cdr' is the abbrev."
> +=C2=A0 =C2=A0 (if (not current)
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 new
> +=C2=A0 =C2=A0 =C2=A0 (if (< (length (cdr new))
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0(length (cdr current)= ))
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 new
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 current)))

Maybe rather than the shortest abbrev, it would be better to choose the
abbrev with the largest difference between abbrev and expansion.

It sounds like an interesting idea. I will try it= out.
=C2=A0
> +(defun abbrev--suggest-maybe-suggest ()
> +=C2=A0 "Suggest an abbrev to the user based on the word(s) befor= e point.
> +Uses `abbrev-suggest-hint-threshold' to find out if the user shou= ld be
> +informed about the existing abbrev."
> +=C2=A0 (let (words abbrev-found word-count)
> +=C2=A0 =C2=A0 (dolist (expansion (abbrev--suggest-get-active-abbrev-e= xpansions))
> +=C2=A0 =C2=A0 =C2=A0 (setq word-count (abbrev--suggest-count-words (c= ar expansion))
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 words (abbrev--suggest-get-= previous-words word-count))

Why not use something like

=C2=A0 =C2=A0 (buffer-substring (- (point) (length (car expansion))) (point= ))?

and when it's equal to (car expansion), then maybe check that there'= ;s
a word boundary?

I'll experiment wi= th not thinking about words. I think I cannot just compare things like that= because of newlines, but that can be handled since I did that in my "= word-based code".
=C2=A0
> +=C2=A0 =C2=A0 =C2=A0 (let ((case-fold-search t))

Some abbrevs are case-sensitive.

Okay..= . Will think about that too. Not sure if I added that because I actually ne= eded it to solve a problem, or if I was proactive and did not need to be...=
=C2=A0
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 (when (and (> word-count 0)
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= (string-match (car expansion) words)

(car expansion) is a string, not a regular expression, so you'd need to=
regexp-quote it before passing it to string-match.
Okay, thanks!
=C2=A0
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= (abbrev--suggest-above-threshold expansion))
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 (setq abbrev-found (abbrev--sugges= t-shortest-abbrev
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 expansion abbrev-found)))))
> +=C2=A0 =C2=A0 (when abbrev-found
> +=C2=A0 =C2=A0 =C2=A0 (abbrev--suggest-inform-user abbrev-found))))
I suspect that abbrev--suggest-above-threshold will eliminate a fairly
large number of abbrevs for some users (e.g. those using abbrevs to fix
recurring typos) and it's a cheap test which doesn't need to alloca= te
any memory, so I'd recommend performing it before the calls to
abbrev--suggest-count-words and abbrev--suggest-get-previous-words.

Will try that.

> +(defun abbrev-suggest-try-expand-maybe-suggest ()
> +=C2=A0 "Try to expand abbrev, look for suggestions of no abbrev = found.
> +This is the main entry to the abbrev suggest mechanism.=C2=A0 This is=
> +set as the default value for `abbrev-expand-function'.=C2=A0 If n= o
> +abbrev expansion is found by `abbrev--default-expand', see if
> +there is an abbrev defined for the word before point, and suggest
> +it to the user."
> +=C2=A0 (or (abbrev--default-expand)
> +=C2=A0 =C2=A0 =C2=A0 (if abbrev-suggest
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 (abbrev-suggest-maybe-suggest))))<= br>
Looks identical to the earlier abbrev--try-expand-maybe-suggest

Yes, it was a leftover.

I= 'll try to cook a new version now. Let's see if it takes another tw= o years :)

Thanks for all the suggestions and time= spent on this.

/Mathias

--000000000000e2c97005a566279b--