bug#65509: [PATCH] Use correct env var when opening frames with freedesktop menu entries

bug#65509: [PATCH] Use correct env var when opening frames with freedesktop menu entries

[Davide Masserut via Bug reports for GNU Emacs, the Swiss army knife of text editors]

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

Tags: patch

When Emacs is built with PGTK, frames opened with the 
emacsclient.desktop entry are opened as X11 windows instead of 
Wayland.

This patch adds a sed command to the Makefile that replaces the 
environment variable "$DISPLAY" with "$WAYLAND_DISPLAY" when built 
with PGTK.



In GNU Emacs 30.0.50 (build 32, x86_64-pc-linux-gnu, GTK+ Version
 3.24.38, cairo version 1.17.8) of 2023-08-24 built on T480s
Repository revision: 3907c884f03cf5f2a09696bda015b1060c7111ba
Repository branch: master
System Description: Arch Linux

Configured using:
 'configure -C --prefix /home/davide/.local --with-pgtk
 --with-native-compilation --enable-link-time-optimization
 --enable-locallisppath=/usr/share/emacs/site-lisp/
 'CFLAGS=-march=native -O2''

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Use-correct-env-var-when-opening-frames-with-freedes.patch --]
[-- Type: text/patch, Size: 1740 bytes --]

From c4f516e30c72fbc88838f2ad52730d5e26f8f358 Mon Sep 17 00:00:00 2001
From: Davide Masserut <dm@mssdvd.com>
Date: Thu, 24 Aug 2023 18:33:39 +0200
Subject: [PATCH] Use correct env var when opening frames with freedesktop menu
 entries

Avoid opening X11 frames with emacsclient.desktop or
emacsclient-mail.desktop when built with PGTK.

* Makefile.in (install-etc): Replace DISPLAY with WAYLAND_DISPLAY when
built with PGTK.
---
 Makefile.in | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/Makefile.in b/Makefile.in
index fdd9353e254..6a08e1a7b18 100644
--- a/Makefile.in
+++ b/Makefile.in
@@ -843,6 +843,9 @@ EMACS_ICON=
 ifeq (${USE_STARTUP_NOTIFICATION},no)
 USE_STARTUP_NOTIFICATION_SED_CMD=-e "/^StartupNotify=true$$/d"
 endif
+ifeq ($(HAVE_PGTK),yes)
+USE_WAYLAND_DISPLAY_SED_CMD=-e "/--display=/ s/DISPLAY/WAYLAND_DISPLAY/"
+endif
 install-etc:
 	umask 022; ${MKDIR_P} "$(DESTDIR)${desktopdir}"
 	tmp=etc/emacs.tmpdesktop; rm -f $${tmp}; \
@@ -857,6 +860,7 @@ install-etc:
 	sed -e "/^Exec=/ s|emacsclient|${bindir}/$${client_name}|" \
 	  -e "/^Icon=emacs/ s/emacs/${EMACS_NAME}/" \
 	  $(USE_STARTUP_NOTIFICATION_SED_CMD) \
+	  $(USE_WAYLAND_DISPLAY_SED_CMD) \
 	  ${srcdir}/etc/emacsclient.desktop > $${tmp}; \
 	${INSTALL_DATA} $${tmp} "$(DESTDIR)${desktopdir}/$${client_name}.desktop"; \
 	rm -f $${tmp}
@@ -870,6 +874,7 @@ install-etc:
 	client_name=`echo emacsclient | sed '$(TRANSFORM)'`${EXEEXT}; \
 	sed -e "/^Exec=/ s|emacsclient|${bindir}/$${client_name}|" \
 	  -e "/^Icon=emacs/ s/emacs/${EMACS_NAME}/" \
+	  $(USE_WAYLAND_DISPLAY_SED_CMD) \
 	  ${srcdir}/etc/emacsclient-mail.desktop > $${tmp}; \
 	${INSTALL_DATA} $${tmp} "$(DESTDIR)${desktopdir}/$${client_name}-mail.desktop"; \
 	rm -f $${tmp}
-- 
2.42.0

bug#65509: [PATCH] Use correct env var when opening frames with freedesktop menu entries

[Eli Zaretskii]

> Date: Thu, 24 Aug 2023 20:32:47 +0200
> From:  Davide Masserut via "Bug reports for GNU Emacs,
>  the Swiss army knife of text editors" <bug-gnu-emacs@gnu.org>
> 
> When Emacs is built with PGTK, frames opened with the 
> emacsclient.desktop entry are opened as X11 windows instead of 
> Wayland.
> 
> This patch adds a sed command to the Makefile that replaces the 
> environment variable "$DISPLAY" with "$WAYLAND_DISPLAY" when built 
> with PGTK.

Po Lu, any comments on this?

bug#65509: [PATCH] Use correct env var when opening frames with freedesktop menu entries

[Po Lu via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Eli Zaretskii <eliz@gnu.org> writes:

>> Date: Thu, 24 Aug 2023 20:32:47 +0200
>> From:  Davide Masserut via "Bug reports for GNU Emacs,
>>  the Swiss army knife of text editors" <bug-gnu-emacs@gnu.org>
>> 
>> When Emacs is built with PGTK, frames opened with the 
>> emacsclient.desktop entry are opened as X11 windows instead of 
>> Wayland.
>> 
>> This patch adds a sed command to the Makefile that replaces the 
>> environment variable "$DISPLAY" with "$WAYLAND_DISPLAY" when built 
>> with PGTK.
>
> Po Lu, any comments on this?

I think this is the fix for the bug I couldn't reproduce earlier,
bug#64310.  The patch seems appropriate, thanks.

bug#65509: [PATCH] Use correct env var when opening frames with freedesktop menu entries

[Stefan Kangas]

forcemerge 64310 65509
thanks

Po Lu writes:

> I think this is the fix for the bug I couldn't reproduce earlier,
> bug#64310.  The patch seems appropriate, thanks.

From the previous discussion, it seems like we wanted to get this
fixed on emacs-29?

Eli, WDYT?

bug#65509: [PATCH] Use correct env var when opening frames with freedesktop menu entries

[Eli Zaretskii]

> From: Stefan Kangas <stefankangas@gmail.com>
> Date: Fri, 1 Sep 2023 20:43:03 +0200
> Cc: Eli Zaretskii <eliz@gnu.org>, Davide Masserut <dm@mssdvd.com>, 65509@debbugs.gnu.org
> 
> forcemerge 64310 65509
> thanks
> 
> Po Lu writes:
> 
> > I think this is the fix for the bug I couldn't reproduce earlier,
> > bug#64310.  The patch seems appropriate, thanks.
> 
> >From the previous discussion, it seems like we wanted to get this
> fixed on emacs-29?
> 
> Eli, WDYT?

I don't know enough about these Freedesktop shortcuts to have an
opinion.  If this is installed only for the PGTK build, and you think
it solves the problem, feel free to install on emacs-29.

bug#65509: [PATCH] Use correct env var when opening frames with freedesktop menu entries

[Davide Masserut via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Stefan Kangas <stefankangas@gmail.com>
>> Date: Fri, 1 Sep 2023 20:43:03 +0200
>> Cc: Eli Zaretskii <eliz@gnu.org>, Davide Masserut 
>> <dm@mssdvd.com>, 65509@debbugs.gnu.org
>>
>> forcemerge 64310 65509
>> thanks
>>
>> Po Lu writes:
>>
>> > I think this is the fix for the bug I couldn't reproduce 
>> > earlier,
>> > bug#64310.  The patch seems appropriate, thanks.
>>
>> >From the previous discussion, it seems like we wanted to get 
>> >this
>> fixed on emacs-29?
>>
>> Eli, WDYT?
>
> I don't know enough about these Freedesktop shortcuts to have an
> opinion.  If this is installed only for the PGTK build, and you 
> think
> it solves the problem, feel free to install on emacs-29.

For the same reason that the current emacsclient.desktop doesn't 
work with Wayland, installing the patch would break PGTK builds 
under Xorg because they would look for an environment variable 
that is not set by Xorg DEs.  Sorry, I've just noticed it.

We could choose to always open a new frame instead of reusing 
exiting ones. How does Emacs behave on other operating systems?

bug#65509: [PATCH] Use correct env var when opening frames with freedesktop menu entries

[Eli Zaretskii]

> From: Davide Masserut <dm@mssdvd.com>
> Cc: Stefan Kangas <stefankangas@gmail.com>, luangruo@yahoo.com,
>  65509@debbugs.gnu.org
> Date: Sat, 02 Sep 2023 18:24:43 +0200
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> >> Po Lu writes:
> >>
> >> > I think this is the fix for the bug I couldn't reproduce
> >> > earlier, bug#64310.  The patch seems appropriate, thanks.
> >>
> >> >From the previous discussion, it seems like we wanted to get
> >> >this fixed on emacs-29?
> >>
> >> Eli, WDYT?
> >
> > I don't know enough about these Freedesktop shortcuts to have an
> > opinion.  If this is installed only for the PGTK build, and you 
> > think
> > it solves the problem, feel free to install on emacs-29.
> 
> For the same reason that the current emacsclient.desktop doesn't 
> work with Wayland, installing the patch would break PGTK builds 
> under Xorg because they would look for an environment variable 
> that is not set by Xorg DEs.  Sorry, I've just noticed it.

Can you describe the issue in more detail?  In particular, would it
work to try several variables one after another?

bug#65509: [PATCH] Use correct env var when opening frames with freedesktop menu entries

[Po Lu via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Eli Zaretskii <eliz@gnu.org> writes:

>> For the same reason that the current emacsclient.desktop doesn't 
>> work with Wayland, installing the patch would break PGTK builds 
>> under Xorg because they would look for an environment variable 
>> that is not set by Xorg DEs.  Sorry, I've just noticed it.
>
> Can you describe the issue in more detail?  In particular, would it
> work to try several variables one after another?

PGTK builds don't support X anyway, so this is a nonproblem.

bug#65509: [PATCH] Use correct env var when opening frames with freedesktop menu entries

[Eli Zaretskii]

> From: Po Lu <luangruo@yahoo.com>
> Cc: Davide Masserut <dm@mssdvd.com>,  stefankangas@gmail.com,
>   65509@debbugs.gnu.org
> Date: Sun, 03 Sep 2023 08:59:04 +0800
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> >> For the same reason that the current emacsclient.desktop doesn't 
> >> work with Wayland, installing the patch would break PGTK builds 
> >> under Xorg because they would look for an environment variable 
> >> that is not set by Xorg DEs.  Sorry, I've just noticed it.
> >
> > Can you describe the issue in more detail?  In particular, would it
> > work to try several variables one after another?
> 
> PGTK builds don't support X anyway, so this is a nonproblem.

Not sure I understand: you are saying we should install the patch and
stop worrying about breaking some other configuration?

bug#65509: [PATCH] Use correct env var when opening frames with freedesktop menu entries

[Po Lu via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Eli Zaretskii <eliz@gnu.org> writes:

> Not sure I understand: you are saying we should install the patch and
> stop worrying about breaking some other configuration?

Yes, thanks.

bug#65509: [PATCH] Use correct env var when opening frames with freedesktop menu entries

[Davide Masserut via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Po Lu <luangruo@yahoo.com> writes:

> Eli Zaretskii <eliz@gnu.org> writes:
>
>> Not sure I understand: you are saying we should install the 
>> patch and
>> stop worrying about breaking some other configuration?
>
> Yes, thanks.

I think we should print to stderr the warning that appears when a 
PGTK frame is opened in X.

bug#65509: [PATCH] Use correct env var when opening frames with freedesktop menu entries

[Po Lu via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Davide Masserut <dm@mssdvd.com> writes:

> I think we should print to stderr the warning that appears when a PGTK
> frame is opened in X.

Why?  It's not as intrusive as displaying the warning in a window, and
we want it to be as effective as possible at deterring people from that
configuration.

bug#65509: [PATCH] Use correct env var when opening frames with freedesktop menu entries

[Davide Masserut via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Po Lu <luangruo@yahoo.com> writes:

> Davide Masserut <dm@mssdvd.com> writes:
>
>> I think we should print to stderr the warning that appears when 
>> a PGTK
>> frame is opened in X.
>
> Why?  It's not as intrusive as displaying the warning in a 
> window, and
> we want it to be as effective as possible at deterring people 
> from that
> configuration.

Sorry, I meant we should *also* print the warning to stderr.

bug#65509: [PATCH] Use correct env var when opening frames with freedesktop menu entries

[Stefan Kangas]

Po Lu <luangruo@yahoo.com> writes:

> Davide Masserut <dm@mssdvd.com> writes:
>
>> I think we should print to stderr the warning that appears when a PGTK
>> frame is opened in X.
>
> Why?  It's not as intrusive as displaying the warning in a window, and
> we want it to be as effective as possible at deterring people from that
> configuration.

Not sure it's worth doing, but one option is to do both.  Users that
miss one would see the other.

BTW, if it's unsupported, should we even show a PGTK frame on X?

bug#65509: [PATCH] Use correct env var when opening frames with freedesktop menu entries

[Po Lu via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Stefan Kangas <stefankangas@gmail.com> writes:

> Not sure it's worth doing, but one option is to do both.  Users that
> miss one would see the other.

I won't object to such a change, though I cannot claim to understand the
rationale behind it.  (Who would see the message printed to stderr, but
not the dialog displayed, during connection setup?)

> BTW, if it's unsupported, should we even show a PGTK frame on X?

It's still useful for testing purposes.

bug#65509: [PATCH] Use correct env var when opening frames with freedesktop menu entries

[Stefan Kangas]

Po Lu <luangruo@yahoo.com> writes:

>> BTW, if it's unsupported, should we even show a PGTK frame on X?
>
> It's still useful for testing purposes.

Aha, right.  I believe you already said so in the past.

bug#65509: [PATCH] Use correct env var when opening frames with freedesktop menu entries

[Davide Masserut via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Po Lu <luangruo@yahoo.com> writes:

> Stefan Kangas <stefankangas@gmail.com> writes:
>
>> Not sure it's worth doing, but one option is to do both.  Users 
>> that
>> miss one would see the other.
>
> I won't object to such a change, though I cannot claim to 
> understand the
> rationale behind it.  (Who would see the message printed to 
> stderr, but
> not the dialog displayed, during connection setup?)

X DEs don't set the $WAYLAND_DISPLAY environment variable, so 
querying for it returns an empty string.

If I run this in X:

emacsclient --alternate-editor= --display=$WAYLAND_DISPLAY 
.emacs.el

the following occurs.

When the --display argument is an empty string, Emacs first tries 
to reuse an exiting frame and, if doesn't find one, it falls back 
to open it in the same terminal.  The problem arises when an 
instance is launched through the DE facilities: Emacs can't use a 
terminal as a fallback and therefore it opens in the background 
without showing the frame.

If we decide that handling such case is not worth the trouble, an 
error in system log should warn the user to use the X backend.

bug#65509: [PATCH] Use correct env var when opening frames with freedesktop menu entries

[Po Lu via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Davide Masserut <dm@mssdvd.com> writes:

> X DEs don't set the $WAYLAND_DISPLAY environment variable, so querying
> for it returns an empty string.
>
> If I run this in X:
>
> emacsclient --alternate-editor= --display=$WAYLAND_DISPLAY .emacs.el
>
> the following occurs.
>
> When the --display argument is an empty string, Emacs first tries to
> reuse an exiting frame and, if doesn't find one, it falls back to open
> it in the same terminal.  The problem arises when an instance is
> launched through the DE facilities: Emacs can't use a terminal as a
> fallback and therefore it opens in the background without showing the
> frame.
>
> If we decide that handling such case is not worth the trouble, an
> error in system log should warn the user to use the X backend.

I don't quite follow: if neither of the display environment variables
are present, then Emacs deduces that no display server is running,
whereupon it resorts to displaying a TTY frame.  We do support TTY
frames under PGTK, so such a warning would be purposeless.

bug#65509: [PATCH] Use correct env var when opening frames with freedesktop menu entries

[Davide Masserut via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Po Lu <luangruo@yahoo.com> writes:

> Davide Masserut <dm@mssdvd.com> writes:
>
>> X DEs don't set the $WAYLAND_DISPLAY environment variable, so
>> querying for it returns an empty string.
>>
>> If I run this in X:
>>
>> emacsclient --alternate-editor= --display=$WAYLAND_DISPLAY 
>> .emacs.el
>>
>> the following occurs.
>>
>> When the --display argument is an empty string, Emacs first tries 
>> to
>> reuse an exiting frame and, if doesn't find one, it falls back to
>> open it in the same terminal.  The problem arises when an instance 
>> is
>> launched through the DE facilities: Emacs can't use a terminal as a
>> fallback and therefore it opens in the background without showing 
>> the
>> frame.
>>
>> If we decide that handling such case is not worth the trouble, an
>> error in system log should warn the user to use the X backend.
>
> I don't quite follow: if neither of the display environment 
> variables
> are present, then Emacs deduces that no display server is running,
> whereupon it resorts to displaying a TTY frame.

Correct.  However, that TTY is occupied by the X window manager, which 
hides the Emacs interface.

When the argument of --display doesn't exits or is empty, should we 
check if $DISPLAY is present in the environment and use its value?

When PGTK is enabled, we could check $WAYLAND_DISPLAY and then 
$DISPLAY before resorting to displaying the frame in a terminal or in 
a TTY.

I tested this with Gnome and i3, both started from a TTY using startx. 
I'm not sure how this behaves when using a display manager.

bug#65509: [PATCH] Use correct env var when opening frames with freedesktop menu entries

[Po Lu via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Davide Masserut <dm@mssdvd.com> writes:

> Correct.  However, that TTY is occupied by the X window manager, which
> hides the Emacs interface.
>
> When the argument of --display doesn't exits or is empty, should we
> check if $DISPLAY is present in the environment and use its value?
>
> When PGTK is enabled, we could check $WAYLAND_DISPLAY and then
> $DISPLAY before resorting to displaying the frame in a terminal or in
> a TTY.
>
> I tested this with Gnome and i3, both started from a TTY using
> startx. I'm not sure how this behaves when using a display manager.

Display selection is GTK's territory, and we basically give it carte
blanche to judge which displays are suitable, since we can't account for
every present and future GDK backend.

Only subsequent to the establishment of the display connection are we
capable of taking display-specific action, such as displaying a warning
message.

bug#65509: [PATCH] Use correct env var when opening frames with freedesktop menu entries

[Davide Masserut via Bug reports for GNU Emacs, the Swiss army knife of text editors]

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

Po Lu <luangruo@yahoo.com> writes:

> Davide Masserut <dm@mssdvd.com> writes:
>
>> Correct.  However, that TTY is occupied by the X window manager,
>> which hides the Emacs interface.
>>
>> When the argument of --display doesn't exits or is empty, should we
>> check if $DISPLAY is present in the environment and use its value?
>>
>> When PGTK is enabled, we could check $WAYLAND_DISPLAY and then
>> $DISPLAY before resorting to displaying the frame in a terminal or in
>> a TTY.
>>
>> I tested this with Gnome and i3, both started from a TTY using
>> startx. I'm not sure how this behaves when using a display manager.
>
> Display selection is GTK's territory, and we basically give it carte
> blanche to judge which displays are suitable, since we can't account
> for every present and future GDK backend.
>
> Only subsequent to the establishment of the display connection are we
> capable of taking display-specific action, such as displaying a
> warning message.

It turns out that Emacs 29 has new CLI option that solves exactly these problems.

The attached patch replaces the usage of --display=$DISPLAY with --reuse-frame.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Avoid-using-display-to-reuse-frames.patch --]
[-- Type: text/x-patch, Size: 2569 bytes --]

From bb7ba852c9add6ed5b3c1edd783aaf07eca08a46 Mon Sep 17 00:00:00 2001
From: Davide Masserut <dm@mssdvd.com>
Date: Wed, 6 Sep 2023 15:35:47 +0200
Subject: [PATCH] Avoid using --display to reuse frames

Using hard-coded display values can cause graphical frames to open
using the wrong backend or not open at all.

* etc/emacsclient-mail.desktop, etc/emacsclient.desktop: Use
--reuse-frame instead of --display.
---
 etc/emacsclient-mail.desktop | 4 ++--
 etc/emacsclient.desktop      | 2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/etc/emacsclient-mail.desktop b/etc/emacsclient-mail.desktop
index 0a2420ddead..819e3855567 100644
--- a/etc/emacsclient-mail.desktop
+++ b/etc/emacsclient-mail.desktop
@@ -2,9 +2,9 @@
 Categories=Network;Email;
 Comment=GNU Emacs is an extensible, customizable text editor - and more
 # We want to pass the following commands to the shell wrapper:
-# u=$(echo "$1" | sed 's/[\"]/\\&/g'); exec emacsclient --alternate-editor= --display="$DISPLAY" --eval "(message-mailto \"$u\")"
+# u=$(echo "$1" | sed 's/[\"]/\\&/g'); exec emacsclient --alternate-editor= --reuse-frame --eval "(message-mailto \"$u\")"
 # Special chars '"', '$', and '\' must be escaped as '\\"', '\\$', and '\\\\'.
-Exec=sh -c "u=\\$(echo \\"\\$1\\" | sed 's/[\\\\\\"]/\\\\\\\\&/g'); exec emacsclient --alternate-editor= --display=\\"\\$DISPLAY\\" --eval \\"(message-mailto \\\\\\"\\$u\\\\\\")\\"" sh %u
+Exec=sh -c "u=\\$(echo \\"\\$1\\" | sed 's/[\\\\\\"]/\\\\\\\\&/g'); exec emacsclient --alternate-editor= --reuse-frame --eval \\"(message-mailto \\\\\\"\\$u\\\\\\")\\"" sh %u
 Icon=emacs
 Name=Emacs (Mail, Client)
 MimeType=x-scheme-handler/mailto;
diff --git a/etc/emacsclient.desktop b/etc/emacsclient.desktop
index 4395d3b02bc..cfbd4d97979 100644
--- a/etc/emacsclient.desktop
+++ b/etc/emacsclient.desktop
@@ -3,7 +3,7 @@ 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++;x-scheme-handler/org-protocol;
-Exec=sh -c "if [ -n \\"\\$*\\" ]; then exec emacsclient --alternate-editor= --display=\\"\\$DISPLAY\\" \\"\\$@\\"; else exec emacsclient --alternate-editor= --create-frame; fi" sh %F
+Exec=sh -c "if [ -n \\"\\$*\\" ]; then exec emacsclient --alternate-editor= --reuse-frame \\"\\$@\\"; else exec emacsclient --alternate-editor= --create-frame; fi" sh %F
 Icon=emacs
 Type=Application
 Terminal=false
-- 
2.42.0

bug#65509: [PATCH] Use correct env var when opening frames with freedesktop menu entries

[Eli Zaretskii]

> From: Davide Masserut <dm@mssdvd.com>
> Cc: Stefan Kangas <stefankangas@gmail.com>, Eli Zaretskii <eliz@gnu.org>,
>  65509@debbugs.gnu.org
> Date: Wed, 06 Sep 2023 16:01:58 +0200
> 
> It turns out that Emacs 29 has new CLI option that solves exactly these problems.
> 
> The attached patch replaces the usage of --display=$DISPLAY with --reuse-frame.

Po Lu, any objections to installing this on emacs-29?

bug#65509: [PATCH] Use correct env var when opening frames with freedesktop menu entries

[Po Lu via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Davide Masserut <dm@mssdvd.com>
>> Cc: Stefan Kangas <stefankangas@gmail.com>, Eli Zaretskii <eliz@gnu.org>,
>>  65509@debbugs.gnu.org
>> Date: Wed, 06 Sep 2023 16:01:58 +0200
>> 
>> It turns out that Emacs 29 has new CLI option that solves exactly these problems.
>> 
>> The attached patch replaces the usage of --display=$DISPLAY with --reuse-frame.
>
> Po Lu, any objections to installing this on emacs-29?

Yes.  Davide, would you please rewrite the patch so that your change is
only applied to the PGTK port?

bug#65509: [PATCH] Use correct env var when opening frames with freedesktop menu entries

[Davide Masserut via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Po Lu <luangruo@yahoo.com> writes:

> Eli Zaretskii <eliz@gnu.org> writes:
>
>>> From: Davide Masserut <dm@mssdvd.com> Cc: Stefan Kangas
>>> <stefankangas@gmail.com>, Eli Zaretskii <eliz@gnu.org>,
>>>  65509@debbugs.gnu.org Date: Wed, 06 Sep 2023 16:01:58 +0200
>>>
>>> It turns out that Emacs 29 has new CLI option that solves exactly
>>> these problems.
>>>
>>> The attached patch replaces the usage of --display=$DISPLAY with
>>> --reuse-frame.
>>
>> Po Lu, any objections to installing this on emacs-29?
>
> Yes.  Davide, would you please rewrite the patch so that your change
> is only applied to the PGTK port?

Sure.  Is there any risk regarding multi-user setups or something like that?

bug#65509: [PATCH] Use correct env var when opening frames with freedesktop menu entries

[Davide Masserut via Bug reports for GNU Emacs, the Swiss army knife of text editors]

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

Po Lu <luangruo@yahoo.com> writes:

> Eli Zaretskii <eliz@gnu.org> writes:
>
>>> From: Davide Masserut <dm@mssdvd.com>
>>> Cc: Stefan Kangas <stefankangas@gmail.com>, Eli Zaretskii <eliz@gnu.org>,
>>>  65509@debbugs.gnu.org
>>> Date: Wed, 06 Sep 2023 16:01:58 +0200
>>> 
>>> It turns out that Emacs 29 has new CLI option that solves exactly these problems.
>>> 
>>> The attached patch replaces the usage of --display=$DISPLAY with --reuse-frame.
>>
>> Po Lu, any objections to installing this on emacs-29?
>
> Yes.  Davide, would you please rewrite the patch so that your change is
> only applied to the PGTK port?

Done.  Po, why can't we use --reuse-frame in X builds?


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Avoid-using-display-to-reuse-frames-in-PGTK.patch --]
[-- Type: text/x-patch, Size: 1710 bytes --]

From 43085522a08b94ef594bd270d61a38d7bb26e036 Mon Sep 17 00:00:00 2001
From: Davide Masserut <dm@mssdvd.com>
Date: Thu, 24 Aug 2023 18:33:39 +0200
Subject: [PATCH] Avoid using --display to reuse frames in PGTK

Using hard-coded display values can cause PGTK graphical frames to
open using the wrong backend or not open at all.

* Makefile.in (install-etc): Use --reuse-frame instead of
--display=$DISPLAY.
---
 Makefile.in | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/Makefile.in b/Makefile.in
index fdd9353e254..c82aad1af60 100644
--- a/Makefile.in
+++ b/Makefile.in
@@ -843,6 +843,9 @@ EMACS_ICON=
 ifeq (${USE_STARTUP_NOTIFICATION},no)
 USE_STARTUP_NOTIFICATION_SED_CMD=-e "/^StartupNotify=true$$/d"
 endif
+ifeq ($(HAVE_PGTK),yes)
+USE_WAYLAND_DISPLAY_SED_CMD=-e "s/display=[^ ]*/reuse-frame/"
+endif
 install-etc:
 	umask 022; ${MKDIR_P} "$(DESTDIR)${desktopdir}"
 	tmp=etc/emacs.tmpdesktop; rm -f $${tmp}; \
@@ -857,6 +860,7 @@ install-etc:
 	sed -e "/^Exec=/ s|emacsclient|${bindir}/$${client_name}|" \
 	  -e "/^Icon=emacs/ s/emacs/${EMACS_NAME}/" \
 	  $(USE_STARTUP_NOTIFICATION_SED_CMD) \
+	  $(USE_WAYLAND_DISPLAY_SED_CMD) \
 	  ${srcdir}/etc/emacsclient.desktop > $${tmp}; \
 	${INSTALL_DATA} $${tmp} "$(DESTDIR)${desktopdir}/$${client_name}.desktop"; \
 	rm -f $${tmp}
@@ -870,6 +874,7 @@ install-etc:
 	client_name=`echo emacsclient | sed '$(TRANSFORM)'`${EXEEXT}; \
 	sed -e "/^Exec=/ s|emacsclient|${bindir}/$${client_name}|" \
 	  -e "/^Icon=emacs/ s/emacs/${EMACS_NAME}/" \
+	  $(USE_WAYLAND_DISPLAY_SED_CMD) \
 	  ${srcdir}/etc/emacsclient-mail.desktop > $${tmp}; \
 	${INSTALL_DATA} $${tmp} "$(DESTDIR)${desktopdir}/$${client_name}-mail.desktop"; \
 	rm -f $${tmp}
-- 
2.42.0

bug#65509: [PATCH] Use correct env var when opening frames with freedesktop menu entries

[Eli Zaretskii]

Ping!  Po Lu, could you please respond?

> From: Davide Masserut <dm@mssdvd.com>
> Cc: stefankangas@gmail.com, 65509@debbugs.gnu.org
> Date: Fri, 08 Sep 2023 17:49:43 +0200
> 
> 
> Po Lu <luangruo@yahoo.com> writes:
> 
> > Eli Zaretskii <eliz@gnu.org> writes:
> >
> >>> From: Davide Masserut <dm@mssdvd.com>
> >>> Cc: Stefan Kangas <stefankangas@gmail.com>, Eli Zaretskii <eliz@gnu.org>,
> >>>  65509@debbugs.gnu.org
> >>> Date: Wed, 06 Sep 2023 16:01:58 +0200
> >>> 
> >>> It turns out that Emacs 29 has new CLI option that solves exactly these problems.
> >>> 
> >>> The attached patch replaces the usage of --display=$DISPLAY with --reuse-frame.
> >>
> >> Po Lu, any objections to installing this on emacs-29?
> >
> > Yes.  Davide, would you please rewrite the patch so that your change is
> > only applied to the PGTK port?
> 
> Done.  Po, why can't we use --reuse-frame in X builds?
> 
> 
> >From 43085522a08b94ef594bd270d61a38d7bb26e036 Mon Sep 17 00:00:00 2001
> From: Davide Masserut <dm@mssdvd.com>
> Date: Thu, 24 Aug 2023 18:33:39 +0200
> Subject: [PATCH] Avoid using --display to reuse frames in PGTK
> 
> Using hard-coded display values can cause PGTK graphical frames to
> open using the wrong backend or not open at all.
> 
> * Makefile.in (install-etc): Use --reuse-frame instead of
> --display=$DISPLAY.
> ---
>  Makefile.in | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/Makefile.in b/Makefile.in
> index fdd9353e254..c82aad1af60 100644
> --- a/Makefile.in
> +++ b/Makefile.in
> @@ -843,6 +843,9 @@ EMACS_ICON=
>  ifeq (${USE_STARTUP_NOTIFICATION},no)
>  USE_STARTUP_NOTIFICATION_SED_CMD=-e "/^StartupNotify=true$$/d"
>  endif
> +ifeq ($(HAVE_PGTK),yes)
> +USE_WAYLAND_DISPLAY_SED_CMD=-e "s/display=[^ ]*/reuse-frame/"
> +endif
>  install-etc:
>  	umask 022; ${MKDIR_P} "$(DESTDIR)${desktopdir}"
>  	tmp=etc/emacs.tmpdesktop; rm -f $${tmp}; \
> @@ -857,6 +860,7 @@ install-etc:
>  	sed -e "/^Exec=/ s|emacsclient|${bindir}/$${client_name}|" \
>  	  -e "/^Icon=emacs/ s/emacs/${EMACS_NAME}/" \
>  	  $(USE_STARTUP_NOTIFICATION_SED_CMD) \
> +	  $(USE_WAYLAND_DISPLAY_SED_CMD) \
>  	  ${srcdir}/etc/emacsclient.desktop > $${tmp}; \
>  	${INSTALL_DATA} $${tmp} "$(DESTDIR)${desktopdir}/$${client_name}.desktop"; \
>  	rm -f $${tmp}
> @@ -870,6 +874,7 @@ install-etc:
>  	client_name=`echo emacsclient | sed '$(TRANSFORM)'`${EXEEXT}; \
>  	sed -e "/^Exec=/ s|emacsclient|${bindir}/$${client_name}|" \
>  	  -e "/^Icon=emacs/ s/emacs/${EMACS_NAME}/" \
> +	  $(USE_WAYLAND_DISPLAY_SED_CMD) \
>  	  ${srcdir}/etc/emacsclient-mail.desktop > $${tmp}; \
>  	${INSTALL_DATA} $${tmp} "$(DESTDIR)${desktopdir}/$${client_name}-mail.desktop"; \
>  	rm -f $${tmp}
> -- 
> 2.42.0
> 

bug#65509: [PATCH] Use correct env var when opening frames with freedesktop menu entries

[Po Lu via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Eli Zaretskii <eliz@gnu.org> writes:

> Ping!  Po Lu, could you please respond?
>
>> From: Davide Masserut <dm@mssdvd.com>
>> Cc: stefankangas@gmail.com, 65509@debbugs.gnu.org
>> Date: Fri, 08 Sep 2023 17:49:43 +0200
>> 
>> 
>> Po Lu <luangruo@yahoo.com> writes:
>> 
>> > Eli Zaretskii <eliz@gnu.org> writes:
>> >
>> >>> From: Davide Masserut <dm@mssdvd.com>
>> >>> Cc: Stefan Kangas <stefankangas@gmail.com>, Eli Zaretskii <eliz@gnu.org>,
>> >>>  65509@debbugs.gnu.org
>> >>> Date: Wed, 06 Sep 2023 16:01:58 +0200
>> >>> 
>> >>> It turns out that Emacs 29 has new CLI option that solves exactly these problems.
>> >>> 
>> >>> The attached patch replaces the usage of --display=$DISPLAY with --reuse-frame.
>> >>
>> >> Po Lu, any objections to installing this on emacs-29?
>> >
>> > Yes.  Davide, would you please rewrite the patch so that your change is
>> > only applied to the PGTK port?
>> 
>> Done.  Po, why can't we use --reuse-frame in X builds?
>> 
>> 
>> >From 43085522a08b94ef594bd270d61a38d7bb26e036 Mon Sep 17 00:00:00 2001
>> From: Davide Masserut <dm@mssdvd.com>
>> Date: Thu, 24 Aug 2023 18:33:39 +0200
>> Subject: [PATCH] Avoid using --display to reuse frames in PGTK
>> 
>> Using hard-coded display values can cause PGTK graphical frames to
>> open using the wrong backend or not open at all.
>> 
>> * Makefile.in (install-etc): Use --reuse-frame instead of
>> --display=$DISPLAY.
>> ---
>>  Makefile.in | 5 +++++
>>  1 file changed, 5 insertions(+)
>> 
>> diff --git a/Makefile.in b/Makefile.in
>> index fdd9353e254..c82aad1af60 100644
>> --- a/Makefile.in
>> +++ b/Makefile.in
>> @@ -843,6 +843,9 @@ EMACS_ICON=
>>  ifeq (${USE_STARTUP_NOTIFICATION},no)
>>  USE_STARTUP_NOTIFICATION_SED_CMD=-e "/^StartupNotify=true$$/d"
>>  endif
>> +ifeq ($(HAVE_PGTK),yes)
>> +USE_WAYLAND_DISPLAY_SED_CMD=-e "s/display=[^ ]*/reuse-frame/"
>> +endif
>>  install-etc:
>>  	umask 022; ${MKDIR_P} "$(DESTDIR)${desktopdir}"
>>  	tmp=etc/emacs.tmpdesktop; rm -f $${tmp}; \
>> @@ -857,6 +860,7 @@ install-etc:
>>  	sed -e "/^Exec=/ s|emacsclient|${bindir}/$${client_name}|" \
>>  	  -e "/^Icon=emacs/ s/emacs/${EMACS_NAME}/" \
>>  	  $(USE_STARTUP_NOTIFICATION_SED_CMD) \
>> +	  $(USE_WAYLAND_DISPLAY_SED_CMD) \
>>  	  ${srcdir}/etc/emacsclient.desktop > $${tmp}; \
>>  	${INSTALL_DATA} $${tmp} "$(DESTDIR)${desktopdir}/$${client_name}.desktop"; \
>>  	rm -f $${tmp}
>> @@ -870,6 +874,7 @@ install-etc:
>>  	client_name=`echo emacsclient | sed '$(TRANSFORM)'`${EXEEXT}; \
>>  	sed -e "/^Exec=/ s|emacsclient|${bindir}/$${client_name}|" \
>>  	  -e "/^Icon=emacs/ s/emacs/${EMACS_NAME}/" \
>> +	  $(USE_WAYLAND_DISPLAY_SED_CMD) \
>>  	  ${srcdir}/etc/emacsclient-mail.desktop > $${tmp}; \
>>  	${INSTALL_DATA} $${tmp} "$(DESTDIR)${desktopdir}/$${client_name}-mail.desktop"; \
>>  	rm -f $${tmp}
>> -- 
>> 2.42.0
>> 

That was lost in the spam filter.  I'd rather not use --reuse-frame
under the X build, because that's not safe for Emacs 29.

This LGTM now.

bug#65509: [PATCH] Use correct env var when opening frames with freedesktop menu entries

[Eli Zaretskii]

> From: Po Lu <luangruo@yahoo.com>
> Cc: Davide Masserut <dm@mssdvd.com>,  stefankangas@gmail.com,
>   65509@debbugs.gnu.org
> Date: Fri, 15 Sep 2023 15:12:16 +0800
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> > Ping!  Po Lu, could you please respond?
> >
> >> From: Davide Masserut <dm@mssdvd.com>
> >> Cc: stefankangas@gmail.com, 65509@debbugs.gnu.org
> >> Date: Fri, 08 Sep 2023 17:49:43 +0200
> >> 
> >> 
> >> Po Lu <luangruo@yahoo.com> writes:
> >> 
> >> > Eli Zaretskii <eliz@gnu.org> writes:
> >> >
> >> >>> From: Davide Masserut <dm@mssdvd.com>
> >> >>> Cc: Stefan Kangas <stefankangas@gmail.com>, Eli Zaretskii <eliz@gnu.org>,
> >> >>>  65509@debbugs.gnu.org
> >> >>> Date: Wed, 06 Sep 2023 16:01:58 +0200
> >> >>> 
> >> >>> It turns out that Emacs 29 has new CLI option that solves exactly these problems.
> >> >>> 
> >> >>> The attached patch replaces the usage of --display=$DISPLAY with --reuse-frame.
> >> >>
> >> >> Po Lu, any objections to installing this on emacs-29?
> >> >
> >> > Yes.  Davide, would you please rewrite the patch so that your change is
> >> > only applied to the PGTK port?
> >> 
> >> Done.  Po, why can't we use --reuse-frame in X builds?
> >> 
> >> 
> >> >From 43085522a08b94ef594bd270d61a38d7bb26e036 Mon Sep 17 00:00:00 2001
> >> From: Davide Masserut <dm@mssdvd.com>
> >> Date: Thu, 24 Aug 2023 18:33:39 +0200
> >> Subject: [PATCH] Avoid using --display to reuse frames in PGTK
> >> 
> >> Using hard-coded display values can cause PGTK graphical frames to
> >> open using the wrong backend or not open at all.
> >> 
> >> * Makefile.in (install-etc): Use --reuse-frame instead of
> >> --display=$DISPLAY.
> >> ---
> >>  Makefile.in | 5 +++++
> >>  1 file changed, 5 insertions(+)
> >> 
> >> diff --git a/Makefile.in b/Makefile.in
> >> index fdd9353e254..c82aad1af60 100644
> >> --- a/Makefile.in
> >> +++ b/Makefile.in
> >> @@ -843,6 +843,9 @@ EMACS_ICON=
> >>  ifeq (${USE_STARTUP_NOTIFICATION},no)
> >>  USE_STARTUP_NOTIFICATION_SED_CMD=-e "/^StartupNotify=true$$/d"
> >>  endif
> >> +ifeq ($(HAVE_PGTK),yes)
> >> +USE_WAYLAND_DISPLAY_SED_CMD=-e "s/display=[^ ]*/reuse-frame/"
> >> +endif
> >>  install-etc:
> >>  	umask 022; ${MKDIR_P} "$(DESTDIR)${desktopdir}"
> >>  	tmp=etc/emacs.tmpdesktop; rm -f $${tmp}; \
> >> @@ -857,6 +860,7 @@ install-etc:
> >>  	sed -e "/^Exec=/ s|emacsclient|${bindir}/$${client_name}|" \
> >>  	  -e "/^Icon=emacs/ s/emacs/${EMACS_NAME}/" \
> >>  	  $(USE_STARTUP_NOTIFICATION_SED_CMD) \
> >> +	  $(USE_WAYLAND_DISPLAY_SED_CMD) \
> >>  	  ${srcdir}/etc/emacsclient.desktop > $${tmp}; \
> >>  	${INSTALL_DATA} $${tmp} "$(DESTDIR)${desktopdir}/$${client_name}.desktop"; \
> >>  	rm -f $${tmp}
> >> @@ -870,6 +874,7 @@ install-etc:
> >>  	client_name=`echo emacsclient | sed '$(TRANSFORM)'`${EXEEXT}; \
> >>  	sed -e "/^Exec=/ s|emacsclient|${bindir}/$${client_name}|" \
> >>  	  -e "/^Icon=emacs/ s/emacs/${EMACS_NAME}/" \
> >> +	  $(USE_WAYLAND_DISPLAY_SED_CMD) \
> >>  	  ${srcdir}/etc/emacsclient-mail.desktop > $${tmp}; \
> >>  	${INSTALL_DATA} $${tmp} "$(DESTDIR)${desktopdir}/$${client_name}-mail.desktop"; \
> >>  	rm -f $${tmp}
> >> -- 
> >> 2.42.0
> >> 
> 
> That was lost in the spam filter.  I'd rather not use --reuse-frame
> under the X build, because that's not safe for Emacs 29.

Why is it not safe?

> This LGTM now.

Thanks.

bug#65509: [PATCH] Use correct env var when opening frames with freedesktop menu entries

[Po Lu via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Eli Zaretskii <eliz@gnu.org> writes:

> Why is it not safe?

Because there was previously no problem with the X build under Emacs 29,
and using --reuse-frame there would chance introducing one.

bug#65509: [PATCH] Use correct env var when opening frames with freedesktop menu entries

[Eli Zaretskii]

> From: Po Lu <luangruo@yahoo.com>
> Cc: dm@mssdvd.com,  stefankangas@gmail.com,  65509@debbugs.gnu.org
> Date: Fri, 15 Sep 2023 15:44:02 +0800
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> > Why is it not safe?
> 
> Because there was previously no problem with the X build under Emacs 29,
> and using --reuse-frame there would chance introducing one.

OK, installed on emacs-29 and closing the bug.

Thanks.