joostkremers / writeroom-mode

Writeroom-mode: distraction-free writing for Emacs.
BSD 3-Clause "New" or "Revised" License
610 stars 28 forks source link

writeroom-global-effects could support standard minor mode argument interpretation #21

Closed andersjohansson closed 9 years ago

andersjohansson commented 9 years ago

In my writeroom I use the standard modeline but hide my panel (fullboth) and thus want to show time and battery status in the modeline. This is easily done with display-time-mode and display-battery-mode. I thought I would be able to just add these to writeroom-global-effects but I had to write a short wrapper function and add that to writeroom-global-effects for it to work:

(defun aj/wr-time-and-battery (arg)
  (if arg
      (progn (display-time-mode 1) (display-battery-mode 1))
    (display-time-mode -1) (display-battery-mode -1)))

This was no problem of course, but what I propose is that the interpretation of the argument in writeroom--activate-global-effects should be the same as the interpretation of arguments for minor modes, i.e. "If called from Lisp, enable the mode if ARG is omitted or nil" (or non-negative really) and disable for negative arguments. This would enable anyone to just throw in any (global) minor mode into writeroom-global-effects, but changing it would of course possibly break current configurations.

Thanks for a great package!

joostkremers commented 9 years ago

Yes, I've had the same thought myself and regret not doing it this way from the start. But my main reason for regretting it was the lack of consistency, which I thought wasn't a good enough reason to risk breaking people's config.

The situation you're describing is a practical matter, however, which may make it worth reconsidering. I'll think about it for a bit and let you know what I decide.

joostkremers commented 9 years ago

Well, I implemented the change, bumped up the major version number and added a note to the README. Hope it won't trip up too many people...

Note that writeroom-mode calls the global effect functions with 1 or -1, so they don't need to accept any other arguments. But that's compatible with minor mode functions, so they should work too. Let me know when you run into trouble, though.

andersjohansson commented 9 years ago

Great! It works well for me with latest from melpa.