Re: [Emacs-diffs] scratch/so-long d216f81: fixup! Add so-long library

classic Classic list List threaded Threaded
3 messages Options
Reply | Threaded
Open this post in threaded view
|

Re: [Emacs-diffs] scratch/so-long d216f81: fixup! Add so-long library

Stefan Monnier
> +(defun turn-on-so-long-minor-mode ()
> +  "Enable minor mode `so-long-minor-mode'."
> +  (so-long-minor-mode 1))

You don't need to pass `1`, so you could also write:

    (defalias 'turn-on-so-long-minor-mode #'so-long-minor-mode)
   
or just don't define turn-on-so-long-minor-mode at all, which is what
I recommend.

> +(defun turn-off-so-long-minor-mode ()
> +  "Disable minor mode `so-long-minor-mode'."
> +  (so-long-minor-mode -1))

I also recommend not to bother defining the turn-off function and let
users call (so-long-minor-mode -1) for the few cases where they might
need it.


        Stefan

Reply | Threaded
Open this post in threaded view
|

Re: [Emacs-diffs] scratch/so-long d216f81: fixup! Add so-long library

Phil Sainty
On 2019-04-15 03:03, Stefan Monnier wrote:
> or just don't define turn-on-so-long-minor-mode at all, which is what
> I recommend.

> I also recommend not to bother defining the turn-off function and let
> users call (so-long-minor-mode -1) for the few cases where they might
> need it.

The actual reason for defining these functions is for the associated
so-long-action-alist entry:

     (so-long-minor-mode
      "Enable so-long-minor-mode"
      turn-on-so-long-minor-mode
      turn-off-so-long-minor-mode)

This kept things nice and simple.

We could get away without the turn-on function, but I felt the resulting
lack of symmetry would be strange, so opted to have them both.


-Phil


Reply | Threaded
Open this post in threaded view
|

Re: [Emacs-diffs] scratch/so-long d216f81: fixup! Add so-long library

Stefan Monnier
>> or just don't define turn-on-so-long-minor-mode at all, which is what
>> I recommend.
>> I also recommend not to bother defining the turn-off function and let
>> users call (so-long-minor-mode -1) for the few cases where they might
>> need it.
> The actual reason for defining these functions is for the associated
> so-long-action-alist entry:
>
>     (so-long-minor-mode
>      "Enable so-long-minor-mode"
>      turn-on-so-long-minor-mode
>      turn-off-so-long-minor-mode)

BTW: How 'bout making this alist accept elements of the form
(<minor-mode> "description")?

Being a minor mode means you know how to gets its current status, how to
turn it on and how to turn it off, so you can standardize the treatment
you did for longlines-mode.


        Stefan