joostkremers / writeroom-mode

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

add "fullscreen" for emacs-mac port? #34

Closed glyph closed 5 years ago

glyph commented 8 years ago

https://bitbucket.org/mituharu/emacs-mac is an alternate Cocoa front-end to Emacs. Its window-system is 'mac as opposed to the official 'ns.

Its handling of fullscreen is slightly different than that of the official mac version as well. Whereas normal emacs has 2 behaviors, fullboth and maximized, where fullboth means "native fullscreen" and maximized means "resized as big as the screen", emacs-mac has three behaviors: fullboth, fullscreen, and maximized. fullboth means "old-style fullscreen, just covering the entire normal desktop", and fullscreen means "use native fullscreen support", similar to what fullboth means in the GNU ns frontend.

It would be appreciated if you could add fullscreen to the list of options - or, better yet, detect this, so that my configuration would do fullboth on linux and fullscreen on emacs-mac.

Thank you!

glyph commented 8 years ago

(As I was writing up this report I realized that an easy workaround is just to change to 'lisp' mode in Customize and type 'fullscreen...)

joostkremers commented 8 years ago

emacs-mac has three behaviors: fullboth, fullscreen, and maximized. fullboth means "old-style fullscreen, just covering the entire normal desktop", and fullscreen means "use native fullscreen support", similar to what fullboth means in the GNU ns frontend.

Well, if fullscreen on emacs-mac is what fullboth is on the ns port of Emacs, that sounds like a bug to me. It seems to make more sense to keep the meaning of fullboth and add a new option for the third effect. But perhaps there are good reasons for doing it this way that I don't know about.

Anyway, your problem is easily solved by manually setting the option in your init file:

(setq writeroom-fullscreen-effect 'fullscreen)

The type/value of a customisation option is only tested in the customisation buffer, writeroom-mode itself doesn't care what value you give it. If you want to make sure your config works on Linux and on OS X, then you could use something like the following snippet:

(cond
 ((eq system-type 'gnu/linux)
  (setq writeroom-fullscreen-effect 'fullboth))
 ((eq system-type 'darwin)
  (setq writeroom-fullscreen-effect 'fullscreen)))

As for adding this option to writeroom-mode, I'll have to think about that. The emacs-mac port is not officially supported. It's essentially a fork of GNU/Emacs, so that would an argument against it. OTOH it's well-supported by its maintainer, it's kept in sync with GNU/Emacs and AFAIU has quite a following on OS X, so that would be an argument in favour.

There would have to be a reliable way to distinguish the emacs-mac port from the official version, though. I'm not sure how to do that.

glyph commented 8 years ago

There would have to be a reliable way to distinguish the emacs-mac port from the official version, though. I'm not sure how to do that.

(eq (window-system) 'mac) :)

glyph commented 8 years ago

Well, if fullscreen on emacs-mac is what fullboth is on the ns port of Emacs, that sounds like a bug to me. It seems to make more sense to keep the meaning of fullboth and add a new option for the third effect. But perhaps there are good reasons for doing it this way that I don't know about.

This is arguably true, but I believe the reason that this is the case is that the "mac port" actually predates the ns port, and when it was first written its behavior of fullboth was the only one available. There are still some reasons to prefer that mode, so they added fullscreen later rather than just changing its behavior.

joostkremers commented 8 years ago

(eq (window-system) 'mac) :)

Yeah, that's what I was looking for. Since I don't have a Mac, I couldn't test it. The doc string of window-system says that the NS port returns ns, so this should work reliably.

I did notice that window-system seems to be deprecated and one is advised to use one of the display-*-p predicates instead, but none of them seems to provide the info that window-system does...

This is arguably true, but I believe the reason that this is the case is that the "mac port" actually predates the ns port, and when it was first written its behavior of fullboth was the only one available. There are still some reasons to prefer that mode, so they added fullscreen later rather than just changing its behavior.

So, historical reasons. I suspected that might be the case.

Anyway, I'll think about this a bit and let you know.

spott commented 7 years ago

I'm just going to throw out another vote for adding in "first class" support for fullscreen.

glyph commented 7 years ago

I'm just going to throw out another vote for adding in "first class" support for fullscreen.

I suspect a PR would increase the value of our votes :)

spott commented 7 years ago

@glyph: yea... I'm very very much a beginner at elisp...

I expect something like:

(defcustom writeroom-fullscreen-effect 'fullboth
  "Effect applied when enabling fullscreen.
The value can be `fullboth', in which case fullscreen is
activated, or `maximized', in which case the relevant frame is
maximized but window decorations are still available."
  :group 'writeroom
  :type '(choice (const :tag "Fullscreen" fullboth)
                 (const :tag "Maximized" maximized)
                 (cond ((eq (window-system) 'mac) (const :tag "Mac Native Fullscreen" fullscreen)))
)
)

But I'm not sure if this will just make nil show up on linux and window boxes as an option.

joostkremers commented 7 years ago

But I'm not sure if this will just make nil show up on linux and window boxes as an option.

Worse, it makes the customize option non-functional...

Could you try the following:

(defcustom writeroom-fullscreen-effect 'fullboth
  "Effect applied when enabling fullscreen.
The value can be `fullboth', in which case fullscreen is
activated, or `maximized', in which case the relevant frame is
maximized but window decorations are still available."
  :group 'writeroom
  :type `(choice (const :tag "Fullscreen" fullboth)
                 (const :tag "Maximized" maximized)
                 ,(cond ((eq (window-system) 'mac) (const :tag "Mac Native Fullscreen" fullscreen)))))

This seems to work OK on my Linux box.

spott commented 7 years ago

Sorry for the (very) late reply.

That didn't work. I only saw the first two options in the customize menu, not the conditional third one.

joostkremers commented 7 years ago

How did you test it? defcustom is a bit picky, it won't redefine a symbol unless done in the right way. If writeroom-mode has already been loaded, you need to kill any writeroom-mode customisation buffers, paste the defcustom above in the *scratch* buffer and evaluate it with C-M-x (with point anywhere in the defcustom). Then recreate the customisation buffer for writeroom-mode (or just the option writeroom-fullscreen-effect).

Alternatively, paste the defcustom above in the file writeroom-mode.el, replacing the definition that's there, byte-compile the file and restart Emacs.

spott commented 7 years ago

I pasted the defcustom in the file writeroom-mode.el and replaced the definition that is there. I didn't byte-compile the file, but I did restart emacs.

How do I go about byte-compiling the file? or will just deleting the byte-compiled version work?

joostkremers commented 7 years ago

I pasted the defcustom in the file writeroom-mode.el and replaced the definition that is there. I didn't byte-compile the file, but I did restart emacs.

But Emacs will load the byte-compiled file anyway, even if the non-byte-compiled version is newer. So in that case, the new definition wasn't used.

How do I go about byte-compiling the file? or will just deleting the byte-compiled version work?

Open the file and do M-x emacs-lisp-byte-compile. That will recompile the file in the current buffer.

spott commented 7 years ago

Ok, I recompiled the file, then reloaded.

The drop-down menu doesn't work anymore: widget-apply: Symbol’s function definition is void: nil.

So it looks like that isn't working.

joostkremers commented 7 years ago

Yes, I get that too, now. Not sure why it seemed to be working a month ago...

I need some more time to figure out how to do this. In the mean time, you can use the following defcustom:


(defcustom writeroom-fullscreen-effect 'fullboth
  "Effect applied when enabling fullscreen.
The value can be `fullboth', in which case fullscreen is
activated, or `maximized', in which case the relevant frame is
maximized but window decorations are still available."
  :group 'writeroom
  :type '(choice (const :tag "Fullscreen" fullboth)
                 (const :tag "Maximized" maximized)
                 (const :tag "Mac Native Fullscreen" fullscreen)))

Or simply put:

(setq writeroom-fullscreen-effect 'maximized)

in your init file.

spott commented 7 years ago

Thanks, let me know if I can be of help.

joostkremers commented 7 years ago

Hi, I'm not sure if anyone's still interested in this, but in case someone is: I created a branch mac-fullscreen, in which I implemented a slightly different solution. Instead of adding another option for the Mac port, the option fullboth is changed to fullscreen when (eq (window-system) 'mac).

Since I don't have a Mac available to me, I can't test it, which is why I haven't added it to master yet. I'd appreciate it if someone could try the mac-fullscreen branch and let me know if it works.