bug#37847: [PATCH] Improve client/daemon xdg/systemd experience

bug#37847: [PATCH] Improve client/daemon xdg/systemd experience

[Carlos Pita]

[-- Attachment #1: Type: text/plain, Size: 1028 bytes --]

The attached patch adds an emacsclient.desktop launcher with
StartWMClass=Emacsclient and also makes systemd start the daemon with
name emacsclient.

This is intended to fix a number of shortcomings in emacs xdg/systemd
integration:

1. There is no emacsclient desktop launcher.

2. emacsclient run with the same class/name hints than emacs, so:

2.1 It's not possible to add it to dock/dash/panel favorites since
desktops identify the launcher for a window using its hints. So you
always get an emacs launcher instead of the emacsclient one.

2.2 Even if manually added to dock/dash/panel, launched frames will
not group under this icon but under the one where standalone emacs
instances group.

The only drawback I can think of is that X resources not specified by
class (Emacs) but by name (emacs vs. emacsclient) will fail to apply.
But I don't think this is usual practice. Note that even if xprop
reports Emacsclient as the class hint, resource resolution always uses
Emacs, so we're ok with that.

Best regards
--
Carlos

[-- Attachment #2: 0001-Improve-client-daemon-xdg-systemd-experience.patch --]
[-- Type: text/x-patch, Size: 2487 bytes --]

From e3ae07705c9d561828bc05814b8d84181c07d2b2 Mon Sep 17 00:00:00 2001
From: memeplex <carlosjosepita@gmail.com>
Date: Mon, 21 Oct 2019 03:58:55 -0300
Subject: [PATCH] Improve client/daemon xdg/systemd experience

* Makefile.in: Add emacsclient.desktop generation.
* etc/emacsclient.desktop: Add file.
* etc/emacs.service: Run with name emacsclient.
---
 Makefile.in             |  7 +++++++
 etc/emacs.service       |  2 +-
 etc/emacsclient.desktop | 12 ++++++++++++
 3 files changed, 20 insertions(+), 1 deletion(-)
 create mode 100644 etc/emacsclient.desktop

diff --git a/Makefile.in b/Makefile.in
index aa11e6b0b7..0d50c1636a 100644
--- a/Makefile.in
+++ b/Makefile.in
@@ -714,6 +714,13 @@ install-etc:
 	  ${srcdir}/etc/emacs.desktop > $${tmp}; \
 	${INSTALL_DATA} $${tmp} "$(DESTDIR)${desktopdir}/${EMACS_NAME}.desktop"; \
 	rm -f $${tmp}
+	tmp=etc/emacsclient.tmpdesktop; rm -f $${tmp}; \
+	client_name=`echo emacsclient | sed '$(TRANSFORM)'`${EXEEXT}; \
+	sed -e "/^Exec=emacsclient/ s|emacsclient|${bindir}/$${client_name}|" \
+	  -e "/^Icon=emacs/ s/emacs/${EMACS_NAME}/" \
+	  ${srcdir}/etc/emacsclient.desktop > $${tmp}; \
+	${INSTALL_DATA} $${tmp} "$(DESTDIR)${desktopdir}/$${client_name}.desktop"; \
+	rm -f $${tmp}
 	umask 022; ${MKDIR_P} "$(DESTDIR)${appdatadir}"
 	tmp=etc/emacs.tmpappdata; rm -f $${tmp}; \
 	sed -e "s/emacs\.desktop/${EMACS_NAME}.desktop/" \
diff --git a/etc/emacs.service b/etc/emacs.service
index c99c6779f5..8f4a10e076 100644
--- a/etc/emacs.service
+++ b/etc/emacs.service
@@ -8,7 +8,7 @@ Documentation=info:emacs man:emacs(1) https://gnu.org/software/emacs/
 
 [Service]
 Type=notify
-ExecStart=emacs --fg-daemon
+ExecStart=@emacs emacsclient --fg-daemon
 ExecStop=emacsclient --eval "(kill-emacs)"
 # The location of the SSH auth socket varies by distribution, and some
 # set it from PAM, so don't override by default.
diff --git a/etc/emacsclient.desktop b/etc/emacsclient.desktop
new file mode 100644
index 0000000000..ba981cc136
--- /dev/null
+++ b/etc/emacsclient.desktop
@@ -0,0 +1,12 @@
+[Desktop Entry]
+Name=Emacs
+GenericName=Text Editor
+Comment=Edit text
+MimeType=text/english;text/plain;text/x-makefile;text/x-c++hdr;text/x-c++src;text/x-chdr;text/x-csrc;text/x-java;text/x-moc;text/x-pascal;text/x-tcl;text/x-tex;application/x-shellscript;text/x-c;text/x-c++;
+Exec=emacsclient -c %F
+Icon=emacs
+Type=Application
+Terminal=false
+Categories=Development;TextEditor;
+StartupWMClass=Emacsclient
+Keywords=Text;Editor;
-- 
2.20.1

bug#37847: [PATCH] Improve client/daemon xdg/systemd experience

[Carlos Pita]

[-- Attachment #1: Type: text/plain, Size: 383 bytes --]

I've changed the name of the emacsclient launcher from "Emacs" to
"Emacs (Client)".

Also added the bug number to the commit message.

One last remark: the name emacsclient to run `emacs --daemon` is
admittedly confusing. OTOH, it is natural as the name of the frames
later launched by the daemon (in particular, it's natural as
StartWMClass). If you prefer I can change it, though.

[-- Attachment #2: 0001-Improve-client-daemon-xdg-systemd-experience-Bug-378.patch --]
[-- Type: text/x-patch, Size: 2508 bytes --]

From 2d9e1951b0a30bb7053cd371de37ac22690d7c8c Mon Sep 17 00:00:00 2001
From: memeplex <carlosjosepita@gmail.com>
Date: Mon, 21 Oct 2019 03:58:55 -0300
Subject: [PATCH] Improve client/daemon xdg/systemd experience (Bug#37847)

* Makefile.in: Add emacsclient.desktop generation.
* etc/emacsclient.desktop: Add file.
* etc/emacs.service: Run with name emacsclient.
---
 Makefile.in             |  7 +++++++
 etc/emacs.service       |  2 +-
 etc/emacsclient.desktop | 12 ++++++++++++
 3 files changed, 20 insertions(+), 1 deletion(-)
 create mode 100644 etc/emacsclient.desktop

diff --git a/Makefile.in b/Makefile.in
index aa11e6b0b7..0d50c1636a 100644
--- a/Makefile.in
+++ b/Makefile.in
@@ -714,6 +714,13 @@ install-etc:
 	  ${srcdir}/etc/emacs.desktop > $${tmp}; \
 	${INSTALL_DATA} $${tmp} "$(DESTDIR)${desktopdir}/${EMACS_NAME}.desktop"; \
 	rm -f $${tmp}
+	tmp=etc/emacsclient.tmpdesktop; rm -f $${tmp}; \
+	client_name=`echo emacsclient | sed '$(TRANSFORM)'`${EXEEXT}; \
+	sed -e "/^Exec=emacsclient/ s|emacsclient|${bindir}/$${client_name}|" \
+	  -e "/^Icon=emacs/ s/emacs/${EMACS_NAME}/" \
+	  ${srcdir}/etc/emacsclient.desktop > $${tmp}; \
+	${INSTALL_DATA} $${tmp} "$(DESTDIR)${desktopdir}/$${client_name}.desktop"; \
+	rm -f $${tmp}
 	umask 022; ${MKDIR_P} "$(DESTDIR)${appdatadir}"
 	tmp=etc/emacs.tmpappdata; rm -f $${tmp}; \
 	sed -e "s/emacs\.desktop/${EMACS_NAME}.desktop/" \
diff --git a/etc/emacs.service b/etc/emacs.service
index c99c6779f5..8f4a10e076 100644
--- a/etc/emacs.service
+++ b/etc/emacs.service
@@ -8,7 +8,7 @@ Documentation=info:emacs man:emacs(1) https://gnu.org/software/emacs/
 
 [Service]
 Type=notify
-ExecStart=emacs --fg-daemon
+ExecStart=@emacs emacsclient --fg-daemon
 ExecStop=emacsclient --eval "(kill-emacs)"
 # The location of the SSH auth socket varies by distribution, and some
 # set it from PAM, so don't override by default.
diff --git a/etc/emacsclient.desktop b/etc/emacsclient.desktop
new file mode 100644
index 0000000000..4897267c1f
--- /dev/null
+++ b/etc/emacsclient.desktop
@@ -0,0 +1,12 @@
+[Desktop Entry]
+Name=Emacs (Client)
+GenericName=Text Editor
+Comment=Edit text
+MimeType=text/english;text/plain;text/x-makefile;text/x-c++hdr;text/x-c++src;text/x-chdr;text/x-csrc;text/x-java;text/x-moc;text/x-pascal;text/x-tcl;text/x-tex;application/x-shellscript;text/x-c;text/x-c++;
+Exec=emacsclient -c %F
+Icon=emacs
+Type=Application
+Terminal=false
+Categories=Development;TextEditor;
+StartupWMClass=Emacsclient
+Keywords=Text;Editor;
-- 
2.20.1

bug#37847: [PATCH] Improve client/daemon xdg/systemd experience

[Carlos Pita]

[-- Attachment #1: Type: text/plain, Size: 251 bytes --]

> One last remark: the name emacsclient to run `emacs --daemon` is admittedly confusing.

I've renamed it to emacsd since it's launched by systemd. Now it's a
bit confusing that StartupWMClass = Emacsd, but I think it's
preferable to the alternative.

[-- Attachment #2: 0001-Improve-client-daemon-xdg-systemd-experience-Bug-378.patch --]
[-- Type: text/x-patch, Size: 2523 bytes --]

From adee7b90139111ce504a61d710b8de2e4c52846e Mon Sep 17 00:00:00 2001
From: memeplex <carlosjosepita@gmail.com>
Date: Mon, 21 Oct 2019 03:58:55 -0300
Subject: [PATCH] Improve client/daemon xdg/systemd experience (Bug#37847)

* Makefile.in: Add emacsclient.desktop generation.
* etc/emacsclient.desktop: Add file, use emacsd as StartupWMClass.
* etc/emacs.service: Run with name emacsd.
---
 Makefile.in             |  7 +++++++
 etc/emacs.service       |  2 +-
 etc/emacsclient.desktop | 12 ++++++++++++
 3 files changed, 20 insertions(+), 1 deletion(-)
 create mode 100644 etc/emacsclient.desktop

diff --git a/Makefile.in b/Makefile.in
index aa11e6b0b7..0d50c1636a 100644
--- a/Makefile.in
+++ b/Makefile.in
@@ -714,6 +714,13 @@ install-etc:
 	  ${srcdir}/etc/emacs.desktop > $${tmp}; \
 	${INSTALL_DATA} $${tmp} "$(DESTDIR)${desktopdir}/${EMACS_NAME}.desktop"; \
 	rm -f $${tmp}
+	tmp=etc/emacsclient.tmpdesktop; rm -f $${tmp}; \
+	client_name=`echo emacsclient | sed '$(TRANSFORM)'`${EXEEXT}; \
+	sed -e "/^Exec=emacsclient/ s|emacsclient|${bindir}/$${client_name}|" \
+	  -e "/^Icon=emacs/ s/emacs/${EMACS_NAME}/" \
+	  ${srcdir}/etc/emacsclient.desktop > $${tmp}; \
+	${INSTALL_DATA} $${tmp} "$(DESTDIR)${desktopdir}/$${client_name}.desktop"; \
+	rm -f $${tmp}
 	umask 022; ${MKDIR_P} "$(DESTDIR)${appdatadir}"
 	tmp=etc/emacs.tmpappdata; rm -f $${tmp}; \
 	sed -e "s/emacs\.desktop/${EMACS_NAME}.desktop/" \
diff --git a/etc/emacs.service b/etc/emacs.service
index c99c6779f5..0dc2418269 100644
--- a/etc/emacs.service
+++ b/etc/emacs.service
@@ -8,7 +8,7 @@ Documentation=info:emacs man:emacs(1) https://gnu.org/software/emacs/
 
 [Service]
 Type=notify
-ExecStart=emacs --fg-daemon
+ExecStart=@emacs emacsd --fg-daemon
 ExecStop=emacsclient --eval "(kill-emacs)"
 # The location of the SSH auth socket varies by distribution, and some
 # set it from PAM, so don't override by default.
diff --git a/etc/emacsclient.desktop b/etc/emacsclient.desktop
new file mode 100644
index 0000000000..3feb83c729
--- /dev/null
+++ b/etc/emacsclient.desktop
@@ -0,0 +1,12 @@
+[Desktop Entry]
+Name=Emacs (Client)
+GenericName=Text Editor
+Comment=Edit text
+MimeType=text/english;text/plain;text/x-makefile;text/x-c++hdr;text/x-c++src;text/x-chdr;text/x-csrc;text/x-java;text/x-moc;text/x-pascal;text/x-tcl;text/x-tex;application/x-shellscript;text/x-c;text/x-c++;
+Exec=emacsclient -c %F
+Icon=emacs
+Type=Application
+Terminal=false
+Categories=Development;TextEditor;
+StartupWMClass=Emacsd
+Keywords=Text;Editor;
-- 
2.20.1

bug#37847: [PATCH] Improve client/daemon xdg/systemd experience

[Lars Ingebrigtsen]

Carlos Pita <carlosjosepita@gmail.com> writes:

> Subject: [PATCH] Improve client/daemon xdg/systemd experience (Bug#37847)
>
> * Makefile.in: Add emacsclient.desktop generation.
> * etc/emacsclient.desktop: Add file, use emacsd as StartupWMClass.
> * etc/emacs.service: Run with name emacsd.

Thanks.

I'm not very familiar with how systemd works, but from my limited
knowledge of it, I think this makes sense?  So I've applied your patch
to Emacs 28.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#37847: [PATCH] Improve client/daemon xdg/systemd experience

[Vojtech Stepancik]

This appears to break packages that rely on `invocation-name' to 
be
executable.

When started as a systemd service, the `invocation-name' of the 
server
process
is `emacsd', which is not indicative of the way the process was
launched.

For example Flycheck tries to use this variable to compile elisp 
code
with the same
version of Emacs as the one being used. I'm sure there are others.

Would it be possible to not alter the argv of the daemon under
systemd?

Thanks for consideration.

Vojtěch Štěpančík

bug#37847: [PATCH] Improve client/daemon xdg/systemd experience

[Lars Ingebrigtsen]

Vojtech Stepancik <vojtechstepancik@outlook.com> writes:

> When started as a systemd service, the `invocation-name' of the server
> process is `emacsd', which is not indicative of the way the process
> was launched.
>
> For example Flycheck tries to use this variable to compile elisp code
> with the same version of Emacs as the one being used. I'm sure there
> are others.

Would reverting this bit of the patch fix the problem?

 [Service]
 Type=notify
-ExecStart=emacs --fg-daemon
+ExecStart=@emacs emacsd --fg-daemon
 ExecStop=emacsclient --eval "(kill-emacs)"


-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#37847: [PATCH] Improve client/daemon xdg/systemd experience

[Vojtech Stepancik]

Yes, that fixes my issue, however as I'm not quite sure why it was 
introduced in the first place, I don't know how it interacts with 
the rest of the patch.

Lars Ingebrigtsen writes:

>
> Would reverting this bit of the patch fix the problem?
>
>  [Service]
>  Type=notify
> -ExecStart=emacs --fg-daemon
> +ExecStart=@emacs emacsd --fg-daemon
>  ExecStop=emacsclient --eval "(kill-emacs)"

bug#37847: [PATCH] Improve client/daemon xdg/systemd experience

[Lars Ingebrigtsen]

Vojtech Stepancik <vojtechstepancik@outlook.com> writes:

> Yes, that fixes my issue, however as I'm not quite sure why it was
> introduced in the first place, I don't know how it interacts with the
> rest of the patch.
>
> Lars Ingebrigtsen writes:
>
>>
>> Would reverting this bit of the patch fix the problem?
>>
>>  [Service]
>>  Type=notify
>> -ExecStart=emacs --fg-daemon
>> +ExecStart=@emacs emacsd --fg-daemon
>>  ExecStop=emacsclient --eval "(kill-emacs)"

Yeah, me neither.  Perhaps Carlos has some insight here...

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#37847: [PATCH] Improve client/daemon xdg/systemd experience

[Lars Ingebrigtsen]

Lars Ingebrigtsen <larsi@gnus.org> writes:

> Vojtech Stepancik <vojtechstepancik@outlook.com> writes:
>
>> Yes, that fixes my issue, however as I'm not quite sure why it was
>> introduced in the first place, I don't know how it interacts with the
>> rest of the patch.
>>
>> Lars Ingebrigtsen writes:
>>
>>>
>>> Would reverting this bit of the patch fix the problem?
>>>
>>>  [Service]
>>>  Type=notify
>>> -ExecStart=emacs --fg-daemon
>>> +ExecStart=@emacs emacsd --fg-daemon
>>>  ExecStop=emacsclient --eval "(kill-emacs)"
>
> Yeah, me neither.  Perhaps Carlos has some insight here...

This was four weeks ago, so I've now pushed this partial reversion to
the trunk.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#37847: [PATCH] Improve client/daemon xdg/systemd experience

[Peter Oliver]

[-- Attachment #1: Type: text/plain, Size: 976 bytes --]

In the systemd service, I believe that Carlos was setting $0 to emacsd for the following effect:

$ xprop -name emacsd@$HOST | grep WM_CLASS
WM_CLASS(STRING) = "emacsd", "Emacsd"

This is because desktops use the StartupWMClass in the .desktop file to match which windows are associated with which .desktop file, and we want a daemonised Emacs to be matched to emacsclient.desktop rather than emacs.desktop.

Consequently, now that this section of the patch has been reverted, the following line remaining in emacsclient.desktop makes no sense:

StartupWMClass=Emacsd


Perhaps we can find a different way of setting the WM_CLASS X11 property?

Alternatively, if we merge the two .desktop files, this issue goes away.  There is currently some discussion of doing this for other reasons on emacs-devel.


I’ve unarchived bug 37847 in order to add this comment, and hope that that isn’t a breach of etiquette.  I’ll happily raise a new bug if that’s preferred.

-- 
Peter Oliver