Re: [Emacs-diffs] scratch/so-long 7273fb2: Add so-long library

[Stefan Monnier <monnier@IRO.UMontreal.CA>]

> If I define a global mode, then there's another variable which is
> ostensibly tracking the 'enabled' state of the functionality, which
> seemed ugly.

I don't see why.  If you don't want to get rid of the so-long-enabled
variable, then just make it a defvaralias and you're done.

> that I could de-sync the mode and that variable (I forget how, but
> possibly by calling `so-long-disable' which I need to keep for
> backwards-compatibility),

Redefine so-long-enable as an obsolete function that just calls
(so-long-auto-mode 1).

Side note: I mistakenly thought so-long was pretty much a brand new
package.  How far back does it go?

> and then it just struck me that I was making more work and a bit of
> a headache for myself in an effort to allow people to add
> '(global-so-long-mode)' in their init files instead of
> '(so-long-enable)' and I immediately lost all interest in spending
> more time on achieving that.

The issue is consistency (a minor mode is a standard interface and UI to
enable&disable&test a feature) and ease of use (a global minor mode can
be enabled via Customize).

> `so-long-enable' is autoloaded, so I could drop the test for
> (require 'so-long nil :noerror) in the Commentary.  That's a hold-
> over from the earlier releases, and it seemed harmless to keep,

Users will copy it and wonder why Emacs has to be so complicated, and it
will then fail the day the file is renamed.

> but I'm happy to remove it, as it's not needed for either 27 or ELPA.

That would be good.

>> On the design side, I think you should merge `so-long` and
>> `so-long-mode` into a single function and make that function
>> a (buffer-local) minor-mode (i.e. not have any major mode, just use
>> fundamental-mode instead).  Making it a minor mode rather than
>> a major-mode will also make it easier to remember the previous
>> major-mode without any need for a change-major-mode-hook hack.
>
> These changes are too significant, so I don't wish to do any of that.

I think they're just some minor shuffling of code.

> There's backwards-compatibility with the earlier releases to consider
> (so-long-mode has always been a major mode);

Then use another name for the minor mode and make so-long-mode be the
combination of fundamental-mode + so-long-minor-mode.

> and I also see a benefit to retaining a major mode option (I have
> a use-case for using it as a file-local 'mode' on one project, and
> I've had cause to suggest the same thing to other users).

Minor modes can also be used on the "mode:" thingy, tho I don't think
that should make a big difference.

> I did consider turning `so-long' into a
> `so-long-minor-mode' at one point, but I think it either seemed like
> a fairly needless change, or possibly the idea of implementing a
> feature which was designed to mess with minor mode states *as* a minor
> mode seemed wrong to me.  'M-x so-long' is also shorter :)

The point of a minor mode is to provide a standard UI to enable *and*
disable something.

>> Rather than defadvice, please use advice-add.
> I know defadvice and I don't know advice-add, so perhaps someone else
> can write those changes.

E.g.

    (defadvice hack-local-variables (after so-long--file-local-mode disable)
      "..."
      ;; The first arg to `hack-local-variables' is HANDLE-MODE since Emacs 26.1,
      ;; and MODE-ONLY in earlier versions.  In either case we are interested in
      ;; whether it has the value `t'.
      (and (eq (ad-get-arg 0) t)
           ad-return-value ; A file-local mode was set.
           (so-long-handle-file-local-mode ad-return-value)))

can be turned into

    (advice-add 'hack-local-variables :around #'so-long--hlv)
    (defun so-long--hlv (orig-fun &optional handle-mode &rest args)
      ;; The first arg to `hack-local-variables' is HANDLE-MODE since Emacs 26.1,
      ;; and MODE-ONLY in earlier versions.  In either case we are interested in
      ;; whether it has the value `t'.
      (let ((retval (apply orig-fun handle-mode args)))
        (and (eq handle-mode t)
             retval ; A file-local mode was set.
             (so-long-handle-file-local-mode retval))))

BTW, while reading that code, I couldn't quite understand what this
advice is about.  I think a comment describing a use-case would be valuable.

> Such changes will need to work in Emacs 25 at least, and ideally back
> to 24.3

advice-add was added to Emacs-24.4 and it is available in GNU ELPA
for earlier Emacsen, so it's OK.


        Stefan