alexmurray / emacs-snap

GNU Emacs in a snap
https://snapcraft.io/emacs
74 stars 13 forks source link

emacs 29.2 changed the window manager name and class used for looking up X resources #83

Closed dburge55 closed 8 months ago

dburge55 commented 8 months ago

It looks like the latest snap release (emacs 29.2) has changed the name of the X window manager name and class, which are used to look up X resources. In the past, resources were found with strings like "emacs.foreground" or "Emacs.foreground", as specified in the emacs documentation. Apparently, the new snap changed these to "emacs-gtk.foreground". This can be discovered using "xprop" to inspect an emacs window. Was this change intentional?

alexmurray commented 8 months ago

No this was not an intentional change per-se - it looks like gnome-shell or whatever the DE is sets the WM_CLASS based on the binary which is executing - and in https://github.com/alexmurray/emacs-snap/commit/85491bbb9db2ef631c08a7369f1879531fec27ca a change was introduced to ship both the old gtk based emacs as well as the new pgtk based emacs as separate binaries (named emacs-gtk and emacs-wayland respectively) and to allow the user to choose which one they wish based on setting EMACS_TOOLKIT in their environment.

So one quick fix on the snap side would be to name the gtk based one back to just emacs and keep the pgtk/wayland one as emacs-wayland - especially since there are various issues with the current pgtk build of emacs anyway (https://github.com/alexmurray/emacs-snap/labels/pgtk) - thoughts?

dburge55 commented 8 months ago

Thank you! I am pleasantly amazed by your quick response!

Unfortunately, I can't contribute much to this discussion because my knowledge of X is ancient and I know practically nothing about Wayland.

I don't know if Wayland has an equivalent of .Xresources or "xrdb". If not, your proposal seems reasonable.

Alternatively... I noticed that I can run "emacs -name emacs", and everything works as expected. Is there, possibly, some way to exploit this inside the snap?

Obviously, it would be easy enough to just change my .Xresources file. I was just caught by surprise, and thought this might need to be documented somewhere in case others run across it.

Thank you, again!

alexmurray commented 8 months ago

oh... hmm well I could change the snap so it always invokes the binary as emacs-gtk -name emacs say... (but ideally we would want to support the user overriding this as well) - but then I see that this changes the default title of the frame as well. The other option might be to name both binaries emacs but put them in different locations within the snap itself. I will give that a try first.

alexmurray commented 8 months ago

So I went with a different option which is to just make the snap always assume it is called "emacs" (rather than using the name of the binary) in https://github.com/alexmurray/emacs-snap/commit/c54ae93fc19697b42813affdcbf512e51927268d - this will build automatically in the next few hours and should be available to test from the beta channel soon after that (ie. once the beta channel has a revision higher than the current one 2372 then it should have landed) - can you please try that and let me know how it works for you?

dburge55 commented 8 months ago

I tested revision 2399. At first glance, it worked as expected. But I ran xprop and got this result WM_CLASS(STRING) = "emacs", "Emacs-gtk"

So, it looks like resources using the window's name will be found, but not the class. Specifically, resources named emacs.foreground will succeed, but Emacs.foreground will fail.

alexmurray commented 8 months ago

I can reproduce this as well but it is odd since as far as I can see, emacs should be setting WM_CLASS to the combination of x-resource-name and x-resource-class which are both emacs and Emacs respectively for me with this new change. Will keep digging...

dburge55 commented 8 months ago

Well, I'm a little embarrassed to admit that I didn't actually test any resources using the "Emacs" class name before writing my previous message. It appears that Emacs.foreground will succeed. It looks like xprop is getting its information from somewhere else.

alexmurray commented 8 months ago

Ok, so it looks like it is GTK that sets the WM_CLASS based on the value of argv[0] during gtk_init() so I have added a subsequent fixup for that as well. It appears to work for me and I now see:

WM_CLASS(STRING) = "emacs", "Emacs"

Let me know if this works better for you too. Thanks.

dburge55 commented 8 months ago

Yes, that looks good. The WM_CLASS in "xprop" looks right, and resources specified with "Emacs" work.

alexmurray commented 8 months ago

Awesome.

dburge55 commented 8 months ago

Thank you!

alexmurray commented 8 months ago

Reopening this as it has reappeared after the change in https://github.com/alexmurray/emacs-snap/commit/6c2d6fe0a343220d3711bee7adc2a914cacf6cbb

WM_CLASS(STRING) = "emacs-gtk", "Emacs"