bug#73004: [PATCH] Make `dired-do-open' work on non GNU/Linux systems

bug#73004: [PATCH] Make `dired-do-open' work on non GNU/Linux systems

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

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

Tags: patch

Hi,

Here is a patch to make `dired-do-open' work on some non GNU/Linux
systems.  I have tested it on OpenBSD with "xdg-open" installed.

Thanks,

In GNU Emacs 31.0.50 (build 19, x86_64-unknown-openbsd7.6) of 2024-09-02
 built on computer
Repository revision: 92ea393a16e5c99a8860dab368c6ca3ca6abc3c5
Repository branch: master
Windowing system distributor 'The X.Org Foundation', version 11.0.12101013
System Description: OpenBSD computer 7.6 GENERIC.MP#294 amd64

Configured using:
 'configure CC=egcc CPPFLAGS=-I/usr/local/include
 LDFLAGS=-L/usr/local/lib MAKEINFO=gmakeinfo --prefix=/home/manuel/emacs
 --bindir=/home/manuel/bin --with-x-toolkit=no --without-cairo
 --without-compress-install'


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Make-dired-do-open-work-on-non-GNU-Linux-systems.patch --]
[-- Type: text/patch, Size: 910 bytes --]

From ab26a89395b5745c8e3d87a8907344ba774a5ca1 Mon Sep 17 00:00:00 2001
From: Manuel Giraud <manuel@ledu-giraud.fr>
Date: Tue, 3 Sep 2024 15:13:51 +0200
Subject: [PATCH] Make `dired-do-open' work on non GNU/Linux systems

* lisp/dired-aux.el (dired-do-open): Permit this function to
work on some non GNU/Linux systems.
---
 lisp/dired-aux.el | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lisp/dired-aux.el b/lisp/dired-aux.el
index cd948bd7dd9..97b1e28a4ff 100644
--- a/lisp/dired-aux.el
+++ b/lisp/dired-aux.el
@@ -1472,7 +1472,7 @@ dired-do-open
     (when command
       (dolist (file files)
         (cond
-         ((memq system-type '(gnu/linux))
+         ((memq system-type '(gnu/linux berkeley-unix))
           (call-process command nil 0 nil file))
          ((memq system-type '(ms-dos))
           (shell-command (concat command " " (shell-quote-argument file))))
-- 
2.46.0


[-- Attachment #3: Type: text/plain, Size: 18 bytes --]

-- 
Manuel Giraud

bug#73004: [PATCH] Make `dired-do-open' work on non GNU/Linux systems

[Philip Kaludercic]

Manuel Giraud <manuel@ledu-giraud.fr> writes:

> Tags: patch
>
> Hi,
>
> Here is a patch to make `dired-do-open' work on some non GNU/Linux
> systems.  I have tested it on OpenBSD with "xdg-open" installed.
>
> Thanks,
>
> In GNU Emacs 31.0.50 (build 19, x86_64-unknown-openbsd7.6) of 2024-09-02
>  built on computer
> Repository revision: 92ea393a16e5c99a8860dab368c6ca3ca6abc3c5
> Repository branch: master
> Windowing system distributor 'The X.Org Foundation', version 11.0.12101013
> System Description: OpenBSD computer 7.6 GENERIC.MP#294 amd64
>
> Configured using:
>  'configure CC=egcc CPPFLAGS=-I/usr/local/include
>  LDFLAGS=-L/usr/local/lib MAKEINFO=gmakeinfo --prefix=/home/manuel/emacs
>  --bindir=/home/manuel/bin --with-x-toolkit=no --without-cairo
>  --without-compress-install'
>
>>From ab26a89395b5745c8e3d87a8907344ba774a5ca1 Mon Sep 17 00:00:00 2001
> From: Manuel Giraud <manuel@ledu-giraud.fr>
> Date: Tue, 3 Sep 2024 15:13:51 +0200
> Subject: [PATCH] Make `dired-do-open' work on non GNU/Linux systems
>
> * lisp/dired-aux.el (dired-do-open): Permit this function to
> work on some non GNU/Linux systems.
> ---
>  lisp/dired-aux.el | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/lisp/dired-aux.el b/lisp/dired-aux.el
> index cd948bd7dd9..97b1e28a4ff 100644
> --- a/lisp/dired-aux.el
> +++ b/lisp/dired-aux.el
> @@ -1472,7 +1472,7 @@ dired-do-open
>      (when command
>        (dolist (file files)
>          (cond
> -         ((memq system-type '(gnu/linux))
> +         ((memq system-type '(gnu/linux berkeley-unix))

Are these the only systems with xdg-open?  And is xdg-open reliably
available on BSD systems, so that we don't generate an opaque error
message if that is not the case?

>            (call-process command nil 0 nil file))
>           ((memq system-type '(ms-dos))
>            (shell-command (concat command " " (shell-quote-argument file))))
> -- 
> 2.46.0

-- 
	Philip Kaludercic on siskin

bug#73004: [PATCH] Make `dired-do-open' work on non GNU/Linux systems

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

Philip Kaludercic <philipk@posteo.net> writes:

[...]

>> diff --git a/lisp/dired-aux.el b/lisp/dired-aux.el
>> index cd948bd7dd9..97b1e28a4ff 100644
>> --- a/lisp/dired-aux.el
>> +++ b/lisp/dired-aux.el
>> @@ -1472,7 +1472,7 @@ dired-do-open
>>      (when command
>>        (dolist (file files)
>>          (cond
>> -         ((memq system-type '(gnu/linux))
>> +         ((memq system-type '(gnu/linux berkeley-unix))
>
> Are these the only systems with xdg-open?

I don't really know.  I guess that FreeBSD and NetBSD also have ports of
xdg-open.  But I don't know for others "berkeley-unix".  Anyway, this is
less restrictive than just "gnu/linux".

> And is xdg-open reliably available on BSD systems, so that we don't
> generate an opaque error message if that is not the case?

I think it would work whether xdg-open is install or not because
`shell-command-guess-open' is defined by testing the presence of
xdg-open.  So if xdg-open is not installed, `command' in the code above
will be nil and nothing will happen.
-- 
Manuel Giraud

bug#73004: [PATCH] Make `dired-do-open' work on non GNU/Linux systems

[Eli Zaretskii]

> Cc: 73004@debbugs.gnu.org
> Date: Thu, 05 Sep 2024 15:48:12 +0200
> From:  Manuel Giraud via "Bug reports for GNU Emacs,
>  the Swiss army knife of text editors" <bug-gnu-emacs@gnu.org>
> 
> > And is xdg-open reliably available on BSD systems, so that we don't
> > generate an opaque error message if that is not the case?
> 
> I think it would work whether xdg-open is install or not because
> `shell-command-guess-open' is defined by testing the presence of
> xdg-open.  So if xdg-open is not installed, `command' in the code above
> will be nil and nothing will happen.

Then why do we need to condition this by system-type at all?

bug#73004: [PATCH] Make `dired-do-open' work on non GNU/Linux systems

[Philip Kaludercic]

Eli Zaretskii <eliz@gnu.org> writes:

>> Cc: 73004@debbugs.gnu.org
>> Date: Thu, 05 Sep 2024 15:48:12 +0200
>> From:  Manuel Giraud via "Bug reports for GNU Emacs,
>>  the Swiss army knife of text editors" <bug-gnu-emacs@gnu.org>
>> 
>> > And is xdg-open reliably available on BSD systems, so that we don't
>> > generate an opaque error message if that is not the case?
>> 
>> I think it would work whether xdg-open is install or not because
>> `shell-command-guess-open' is defined by testing the presence of
>> xdg-open.  So if xdg-open is not installed, `command' in the code above
>> will be nil and nothing will happen.
>
> Then why do we need to condition this by system-type at all?

Juri implemented the command, so perhaps he could explain.  I don't
understand why one doesn't just use `shell-command-guess-open' directly?

-- 
	Philip Kaludercic on siskin

bug#73004: [PATCH] Make `dired-do-open' work on non GNU/Linux systems

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

Eli Zaretskii <eliz@gnu.org> writes:

>> Cc: 73004@debbugs.gnu.org
>> Date: Thu, 05 Sep 2024 15:48:12 +0200
>> From:  Manuel Giraud via "Bug reports for GNU Emacs,
>>  the Swiss army knife of text editors" <bug-gnu-emacs@gnu.org>
>> 
>> > And is xdg-open reliably available on BSD systems, so that we don't
>> > generate an opaque error message if that is not the case?
>> 
>> I think it would work whether xdg-open is install or not because
>> `shell-command-guess-open' is defined by testing the presence of
>> xdg-open.  So if xdg-open is not installed, `command' in the code above
>> will be nil and nothing will happen.
>
> Then why do we need to condition this by system-type at all?

Yes I wondered this too.  The current "otherwise" clause is an error
message but it could be this `call-process' instead.
-- 
Manuel Giraud

bug#73004: [PATCH] Make `dired-do-open' work on non GNU/Linux systems

[Juri Linkov]

>>> > And is xdg-open reliably available on BSD systems, so that we don't
>>> > generate an opaque error message if that is not the case?
>>>
>>> I think it would work whether xdg-open is install or not because
>>> `shell-command-guess-open' is defined by testing the presence of
>>> xdg-open.  So if xdg-open is not installed, `command' in the code above
>>> will be nil and nothing will happen.
>>
>> Then why do we need to condition this by system-type at all?
>
> Juri implemented the command, so perhaps he could explain.  I don't
> understand why one doesn't just use `shell-command-guess-open' directly?

The current implementation is based on long discussions including bug#18132.
So everyone is welcome to improve it, it's not cast in stone.

bug#73004: [PATCH] Make `dired-do-open' work on non GNU/Linux systems

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

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

Juri Linkov <juri@linkov.net> writes:

[...]

> The current implementation is based on long discussions including bug#18132.
> So everyone is welcome to improve it, it's not cast in stone.

Thanks for the pointer.  I'm proposing the following patch then.

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Make-dired-do-open-work-on-more-nix-systems.patch --]
[-- Type: text/x-patch, Size: 2094 bytes --]

From d8e59996f8a8dccd564cb27e5a2a56f83cb2db6f Mon Sep 17 00:00:00 2001
From: Manuel Giraud <manuel@ledu-giraud.fr>
Date: Fri, 6 Sep 2024 09:47:33 +0200
Subject: [PATCH] Make `dired-do-open' work on more *nix systems

* lisp/dired-aux.el (dired-do-open): Make `dired-do-open' work
on more *nix systems.
---
 lisp/dired-aux.el | 29 ++++++++++++++---------------
 1 file changed, 14 insertions(+), 15 deletions(-)

diff --git a/lisp/dired-aux.el b/lisp/dired-aux.el
index cd948bd7dd9..1d0e29b8782 100644
--- a/lisp/dired-aux.el
+++ b/lisp/dired-aux.el
@@ -1469,21 +1469,20 @@ dired-do-open
     (when (and (memq system-type '(windows-nt))
                (equal command "start"))
       (setq command "open"))
-    (when command
-      (dolist (file files)
-        (cond
-         ((memq system-type '(gnu/linux))
-          (call-process command nil 0 nil file))
-         ((memq system-type '(ms-dos))
-          (shell-command (concat command " " (shell-quote-argument file))))
-         ((memq system-type '(windows-nt))
-          (w32-shell-execute command (convert-standard-filename file)))
-         ((memq system-type '(cygwin))
-          (call-process command nil nil nil file))
-         ((memq system-type '(darwin))
-          (start-process (concat command " " file) nil command file))
-         (t
-          (error "Open not supported on this system")))))))
+    (if command
+        (dolist (file files)
+          (cond
+           ((memq system-type '(ms-dos))
+            (shell-command (concat command " " (shell-quote-argument file))))
+           ((memq system-type '(windows-nt))
+            (w32-shell-execute command (convert-standard-filename file)))
+           ((memq system-type '(cygwin))
+            (call-process command nil nil nil file))
+           ((memq system-type '(darwin))
+            (start-process (concat command " " file) nil command file))
+           (t
+            (call-process command nil 0 nil file))))
+      (error "Open not supported on this system"))))
 
 \f
 ;;; Commands that delete or redisplay part of the dired buffer
-- 
2.46.0


[-- Attachment #3: Type: text/plain, Size: 18 bytes --]

-- 
Manuel Giraud

bug#73004: [PATCH] Make `dired-do-open' work on non GNU/Linux systems

[Eli Zaretskii]

> From: Manuel Giraud <manuel@ledu-giraud.fr>
> Cc: Philip Kaludercic <philipk@posteo.net>,  Eli Zaretskii <eliz@gnu.org>,
>   73004@debbugs.gnu.org
> Date: Fri, 06 Sep 2024 09:50:22 +0200
> 
> Juri Linkov <juri@linkov.net> writes:
> 
> [...]
> 
> > The current implementation is based on long discussions including bug#18132.
> > So everyone is welcome to improve it, it's not cast in stone.
> 
> Thanks for the pointer.  I'm proposing the following patch then.

Juri, is this change okay with you?

bug#73004: [PATCH] Make `dired-do-open' work on non GNU/Linux systems

[Juri Linkov]

close 73004 31.0.50
thanks

>> > The current implementation is based on long discussions including bug#18132.
>> > So everyone is welcome to improve it, it's not cast in stone.
>>
>> Thanks for the pointer.  I'm proposing the following patch then.
>
> Juri, is this change okay with you?

Yes, this looks right, so now pushed to master.
Thanks Manuel for the patch.

bug#73004: [PATCH] Make `dired-do-open' work on non GNU/Linux systems

[Howard Melman]

Juri Linkov <juri@linkov.net> writes:

>>> > The current implementation is based on long
>>> > discussions including bug#18132.  So everyone is
>>> > welcome to improve it, it's not cast in stone.
>>>
>>> Thanks for the pointer.  I'm proposing the following patch then.
>>
>> Juri, is this change okay with you?
>
> Yes, this looks right, so now pushed to master.
> Thanks Manuel for the patch.

Forgive the micro-optimization, but do you have to check the
os for each and every file in files?

-- 

Howard

bug#73004: [PATCH] Make `dired-do-open' work on non GNU/Linux systems

[Juri Linkov]

>> Yes, this looks right, so now pushed to master.
>> Thanks Manuel for the patch.
>
> Forgive the micro-optimization, but do you have to check the
> os for each and every file in files?

Thanks for noticing a possibility for optimization.
The first version was just a proof of concept,
and now pushed the optimized version.