Re: Abbrev tables in elisp with some extra stuff

[Stefan Monnier]

>       - :case-preserve non-nil means that abbreviations are lookedup without
>       case-folding, and the expansion is not capitalized/upcased.

> It seems like a mistake to make this a per-table decision.
> In every abbrev table, abbrevs should preserve case by default.
> To define an abbrev that is only detected in a particular case
> is an exception, so each abbrev needs to be marked if it is
> to work that way.

In my experience, this exception usually holds for groups of abbreviations
or even for all abbrevs defined in a mode (typically for skeleton-abbrevs),
so it seems convenient to set it once and for all for the whole group.
Since we can have many abbrev-tables active at the same time, this is not
a limitation.

>       - :syntax-table holds the syntax table to use to find the relevant word.
> Why do we want this?

So that we can define abbrevs which include "-" (for example) without
changing the syntax of "-" in the normal syntax-table.  Currently python.el
uses an ugly pre-abbrev-expand-hook to cobble up some way to simulate
this feature.  mailabbrev.el also needs this.

>       - :abbrev-before-point-function holds a function to use to find the
>       abbrev at point.
> Why do we want this?

Mostly to provide other rules than "abbrev name = preceding word".

E.g. there's a sample function in the code I sent to place on this hook
which instead of looking at the preceding word just cycles through all the
abbrevs in the table and checks if it matches text before point, so as to
completely eliminate the "abbrev=word" limitation.  Multi-word abbrevs are
regularly requested on gnu.emacs.help (usually not very loudly, admittedly).

>       - :enable-function can be set to a function of no argument which returns
>       non-nil iff the abbrevs in this table should be used for this instance
>       of `expand-abbrev'.  Useful to disable skeleton-abbrevs in strings and
>       comments.

> That feature is useful, but shouldn't it be per-abbrev, not per-table?

Same as :case-preserve, it tends to apply to groups of abbrevs.

> If we have some abbrevs that are from skeletons, and some abbrevs that
> are not, we don't want to have to put them in different abbrev tables.

Why not?  We can also combine those tables into a single one via
inheritance, if desired.

>     - allow local-abbrev-table to hold a list of abbrev tables so minor
>     modes can add their own abbrev tables as well (useful for
>     mailabbrev.el).
> The right way to do this is to have minor-mode-abbrev-table-alist
> which would work like minor-mode-map-alist.

We could do that as well, but it didn't seem to be much better and makes for
more complex processing in expand-abbrev which is likely to be more often
executed than enabling/disabling minor modes.  As long as the minor mode is
enabled/disabled via the function, we can provide the same behavior in
either case.

>     +(defvar abbrev-auto-activated-tables t
>     +  ;; Could be expanded to be a predicate.
>     +  "List of abbrev tables that can be used when `expand-abbrev' is called 
> implicitly.
>     +If t, use all installed tables.")

> Is this the best way to design the feature so that `mail-abbrevs-only'
> can use it?  Ideally we want some hook function to test
> `mail-abbrevs-only' and DTRT, so that setting or binding
> `mail-abbrevs-only' in any fashion has the right effect.  That is the
> case with the current code in mailabbrev.el.  I don't want to take
> a step backwards.

I'm not 100% satisfied with abbrev-auto-activated-tables either indeed.
The problem is that the hook function would need to run *around* the
expand-abbrev code (the current mailabbrev.el uses an big ugly hack to be
able to wrap itself around expand-abbrev).  I started writing
abbrev-with-wrapper-hook for this purpose, actually, so that might be
a better solution.

>     +(defmacro abbrev-with-wrapper-hook (var &rest body)
>     +  "Run BODY wrapped with the VAR hook.
>     +VAR is a special hook: its functions are called with one argument which
>     +is the \"original\" code, so the hook function can wrap the original 
> function,

> What does "the original function" mean?  There is no function
> in the arguments.
> I think it means the body.

You're right, it means the body (potentially wrapped by other functions
on the hook).

>     +  `(labels ((loop (--abrev-funs-- --abbrev-global--)

> Redefining `loop' is really confusing.

It's only locally (lexically) bound, but I guess you're right that I should
use another name.

> Using `labels' in this way also requires an explanation
> of why it is written this way.

It's just so as to use lexical scoping, which is much cleaner than going
through the gensym rigmarole.

> Did you try to write it in a more straightforward way, without
> `labels'?  If so, what was the problem with that approach?

lexical scoping *is* the straightforward way.

>     +(defvar abbrev-minor-mode-tables nil
>     +  "List of additional abbrev tables.")

> "Additional" does not explain clearly the difference
> between these and others.

This variable is a remnant from before local-abbrev-table could hold a list
of tables.  I'd remove it (or replace it with something else).

>     +(defcustom abbrev-all-caps nil
>     +  "Set non-nil means expand multi-word abbrevs all caps if abbrev was 
> so."
> Please delete "Set".

Sure (this docstring is copied straight from abbrev.c ;-)

>     +        (dolist (table (prog1 tables (setq tables nil)) tables)
>     +          (if (abbrev-set-member table abbrev-auto-activated-tables)
>     +              (push table tables))))))))

> That always returns nil.

No.  But if this code confuses you, I'd better rewrite it as its equivalent

       (dolist (table (prog1 tables (setq tables nil)))
         (if (abbrev-set-member table abbrev-auto-activated-tables)
             (push table tables)))
       tables)))))

>     +(defun write--abbrev (sym)
> This name should start with `internal-'.
>     +(defun describe--abbrev (sym)
> Likewise.

I like the "--" convention used in a few other packages, mostly because it
still sticks to the normal prefix.  But if you insist, I can use the longer
and prefix-less-clean "internal-" thingy.


        Stefan