bug#12972: 24.3.50; Move `org-open-file' and associated code out of Org mode

bug#12972: 24.3.50; Move `org-open-file' and associated code out of Org mode

[Drew Adams]

IIUC, `org-open-file' and its associated code, such as `org-file-apps',
`org-default-apps', and `org-apps-regexp-alist', have nothing
particularly to do with Org mode.  They constitute general-purpose code
for opening files using associated programs.  Code that uses them
should not need to load the Org code, and this code should be
maintained separately for general use.
 
Please move all such code out of the Org files and into a more general
file, such as files.el.  Create a new general file for such things, if
that is more appropriate.
 
Anything in the code that is truly specific to Org mode, such as
Org-state mgt and link recognition/following (e.g.,
`org-mark-ring-push', `org-link-frame-setup',
`org-file-apps-entry-match-against-dlink-p', `org-link-search'), should
be kept (only) for Org mode.  What is important is to factor out the
generic code that opens a file using an associated app.
 
Seems like this should have been done when the Org code was added to
Emacs.  There might be additional opportunities for factoring out some
useful, general-purpose code from Org mode - dunno.
 
In GNU Emacs 24.3.50.1 (i386-mingw-nt5.1.2600)
 of 2012-11-19 on MS-W7-DANI
Bzr revision: 110950 monnier@iro.umontreal.ca-20121119182725-5p6w4wjimm7epggr
Windowing system distributor `Microsoft Corp.', version 5.1.2600
Configured using:
 `configure --with-gcc (4.7) --no-opt --enable-checking --cflags
 -Ic:/emacs/libs/libXpm-3.5.10/include -Ic:/emacs/libs/libXpm-3.5.10/src
 -Ic:/emacs/libs/libpng-1.2.37-lib/include -Ic:/emacs/libs/zlib-1.2.5
 -Ic:/emacs/libs/giflib-4.1.4-1-lib/include
 -Ic:/emacs/libs/jpeg-6b-4-lib/include
 -Ic:/emacs/libs/tiff-3.8.2-1-lib/include
 -Ic:/emacs/libs/libxml2-2.7.8-w32-bin/include/libxml2
 -Ic:/emacs/libs/gnutls-3.0.9-w32-bin/include
 -Ic:/emacs/libs/libiconv-1.9.2-1-lib/include'
 

bug#12972: 24.3.50; Move `org-open-file' and associated code out of Org mode

[Jambunathan K]

"Drew Adams" <drew.adams@oracle.com> writes:

> IIUC, `org-open-file' and its associated code, such as `org-file-apps',
> `org-default-apps', and `org-apps-regexp-alist', have nothing
> particularly to do with Org mode.  They constitute general-purpose code
> for opening files using associated programs.  Code that uses them
> should not need to load the Org code, and this code should be
> maintained separately for general use.

+10.

There should also be key binding within dired for opening files
"blindly". For a long time, within dired, I had `C-u RET' open file in a
system-registered app.

    (defun my-dired-find-file (&optional prefix)
      (interactive "P")
      (if prefix
          (org-open-file (dired-get-file-for-visit) 'system)
        (dired-find-file)))

    (define-key dired-mode-map "\r" 'my-dired-find-file)

When I don't have above snippet installed, I do the following.

On Windows, 

    & open

or 

    ! open

On Linux, I can do,

    C-c C-m C-l

It would be wonderful if I can just rely on Emacs to abstract out the OS
I am working on.
-- 

bug#12972: 24.3.50; Move `org-open-file' and associated code out of Org mode

[Lars Ingebrigtsen]

"Drew Adams" <drew.adams@oracle.com> writes:

> IIUC, `org-open-file' and its associated code, such as `org-file-apps',
> `org-default-apps', and `org-apps-regexp-alist', have nothing
> particularly to do with Org mode.  They constitute general-purpose code
> for opening files using associated programs.  Code that uses them
> should not need to load the Org code, and this code should be
> maintained separately for general use.

`org-open-file' is too Org-specific to move out of Org.  But there
should be a command to do what it essentially does outside of Org --
that is, use ~/.mailcap to determine what viewer to use.  So I've now
added this to Emacs 28 under the name `mailcap-view-file'.

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

bug#12972: 24.3.50; Move `org-open-file' and associated code out of Org mode

[Maxim Nikulin]

On 01/06/2021 13:56, Lars Ingebrigtsen wrote:
> So I've now added this to Emacs 28 under the name
> `mailcap-view-file'.

I am sorry if it is a false alarm. Feel free to close the bug again if 
something changed recently in `start-process-shell-command' or if you 
prefer to discuss the issue as another bug.

It seems that implementation of `mailcap-view-file' is unreliable due to 
creation of unnecessary terminal session and it can cause obscure and
difficult to reproduce failures similar to
https://lists.gnu.org/archive/html/emacs-orgmode/2020-09/msg00195.html
https://lists.gnu.org/archive/html/emacs-orgmode/2020-06/msg00332.html
The thread is actually longer than it is shown in the archive interface.
Another lengthy discussion:
https://debbugs.gnu.org/cgi/bugreport.cgi?bug=44824

In Org latest change was required for compatibility reason:
https://code.orgmode.org/bzg/org-mode/commit/869b7a21b94ed112f6640c8f2711c2a68b661dea

Let-bind (process-connection-type nil) is a minimal required change to 
avoid unnecessary terminal session. However it is not friendly to users 
in the case when troubleshooting is required. `make-process' with 
sentinel is hopefully better.

The following could be ignored since it likely requires significant 
amount of work with unclear benefits.

1. `org-open-file' besides Org-specific stuff allows to specify precise 
target inside the file. It can be quite useful, e.g.

     okular --page 11 --find "some pattern" file.pdf

PDF files have internal anchors as well. I have no consistent vision how 
to express additional "locators" in general API.

2. There are at least two sources of truth for MIME-handlers on linux 
desktop that are not necessary synchronized. Info from extracted from 
.desktop files may be configurable from desktop UI unlike mailcap. 
Distros may have some instruments to mitigate discrepancies. Debian adds 
entries from .desktop handlers to system-wide mailcap DB. Another 
approach is to add to maicap greedy xdg-open handler that tries to guess 
currently running desktop and pass arguments to appropriate command.

Maybe mailcap should be secondary MIME database in Emacs, not the 
primary one.

3. Currently only file suffix is inspected to determine MIME type of a 
file. libmagic (or file command) usually provides more precise info, so 
it is possible to open an incorrectly named file.

4. Mailcap has more features that are not addressed in Emacs. They may 
be handy if Emacs is launched in terminal on remote server. It might 
allow e.g. to open PDF file using pdftotext handler.
- A buffer for command output should be created for "copiousoutput"  option.
- A buffer should be created and terminal session should be enabled if 
an entry "needsterminal".
- There are more substitutions than "%s". However I am unsure if it is 
possible to provide more info than application can obtain from the file.
I think, it is intended for mail multipart messages and additional headers.

On the other hand mailcap handlers might expect safe file names (minimal 
ASCII subset), users may have files with arbitrary names (national 
charset or some special characters). I hope, almost all handlers do not 
have such problem.

In summary, during launch of external command terminal session must be 
suppressed. There is enough room for MIME-related improvements in Emacs 
in general and in Org mode in particular.

Re: bug#12972: 24.3.50; Move `org-open-file' and associated code out of Org mode

[Maxim Nikulin]

On 01/06/2021 13:56, Lars Ingebrigtsen wrote:
> So I've now added this to Emacs 28 under the name
> `mailcap-view-file'.

I am sorry if it is a false alarm. Feel free to close the bug again if 
something changed recently in `start-process-shell-command' or if you 
prefer to discuss the issue as another bug.

It seems that implementation of `mailcap-view-file' is unreliable due to 
creation of unnecessary terminal session and it can cause obscure and
difficult to reproduce failures similar to
https://lists.gnu.org/archive/html/emacs-orgmode/2020-09/msg00195.html
https://lists.gnu.org/archive/html/emacs-orgmode/2020-06/msg00332.html
The thread is actually longer than it is shown in the archive interface.
Another lengthy discussion:
https://debbugs.gnu.org/cgi/bugreport.cgi?bug=44824

In Org latest change was required for compatibility reason:
https://code.orgmode.org/bzg/org-mode/commit/869b7a21b94ed112f6640c8f2711c2a68b661dea

Let-bind (process-connection-type nil) is a minimal required change to 
avoid unnecessary terminal session. However it is not friendly to users 
in the case when troubleshooting is required. `make-process' with 
sentinel is hopefully better.

The following could be ignored since it likely requires significant 
amount of work with unclear benefits.

1. `org-open-file' besides Org-specific stuff allows to specify precise 
target inside the file. It can be quite useful, e.g.

     okular --page 11 --find "some pattern" file.pdf

PDF files have internal anchors as well. I have no consistent vision how 
to express additional "locators" in general API.

2. There are at least two sources of truth for MIME-handlers on linux 
desktop that are not necessary synchronized. Info from extracted from 
.desktop files may be configurable from desktop UI unlike mailcap. 
Distros may have some instruments to mitigate discrepancies. Debian adds 
entries from .desktop handlers to system-wide mailcap DB. Another 
approach is to add to maicap greedy xdg-open handler that tries to guess 
currently running desktop and pass arguments to appropriate command.

Maybe mailcap should be secondary MIME database in Emacs, not the 
primary one.

3. Currently only file suffix is inspected to determine MIME type of a 
file. libmagic (or file command) usually provides more precise info, so 
it is possible to open an incorrectly named file.

4. Mailcap has more features that are not addressed in Emacs. They may 
be handy if Emacs is launched in terminal on remote server. It might 
allow e.g. to open PDF file using pdftotext handler.
- A buffer for command output should be created for "copiousoutput"  option.
- A buffer should be created and terminal session should be enabled if 
an entry "needsterminal".
- There are more substitutions than "%s". However I am unsure if it is 
possible to provide more info than application can obtain from the file.
I think, it is intended for mail multipart messages and additional headers.

On the other hand mailcap handlers might expect safe file names (minimal 
ASCII subset), users may have files with arbitrary names (national 
charset or some special characters). I hope, almost all handlers do not 
have such problem.

In summary, during launch of external command terminal session must be 
suppressed. There is enough room for MIME-related improvements in Emacs 
in general and in Org mode in particular.

bug#12972: [PATCH] Avoid regression in mailcap-view-file similar to Bug#44824

[Maxim Nikulin]

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

On 01/06/2021 13:56, Lars Ingebrigtsen wrote:
> So I've now added this to Emacs 28 under the name `mailcap-view-file'.

I am attaching a patch similar to proposed to Org mode that should help 
to avoid obscure failures of viewers due to unnecessary terminal sessions.

[-- Attachment #2: 0001-mailcap.el-Avoid-xdg-open-silent-failure.patch --]
[-- Type: text/x-patch, Size: 2822 bytes --]

From de55b623810736df04641a4d8f6027ccb04caa7f Mon Sep 17 00:00:00 2001
From: Max Nikulin <manikulin@gmail.com>
Date: Thu, 1 Jul 2021 23:41:16 +0700
Subject: [PATCH] mailcap.el: Avoid xdg-open silent failure

* lisp/net/mailcap.el (mailcap-view-file): Use 'pipe :connection-type
instead of 'pty to prevent killing of background process on handler
exit.  Avoid regression similar to Bug#44824.

Problem happens only in some desktop environments where mailcap handler
launches actual viewer (as defined in .desktop files and obtained from
mimeapps.list) in background.  E.g. xdg-open invokes "gio open" or
kde-open5 for Gnome or KDE accordingly and these handlers launch e.g.
eog or okular in background.  As soon as main process exits, temporary
terminal session created by `start-process-shell-command' is terminated.
As a result background processes receive SIGHUP.

Previously command were executed with no buffer as well, so the change
does not affect "needsterminal" and "copiousoutput" mailcap features,
they are not supported as earlier.

If main process of the handler fails then show a message with exit
reason.  Output (including error messages) is ignored as before.
Gtk applications tend to report significant amount of failed asserts
hardly informative for majority of users.
---
 lisp/net/mailcap.el | 18 +++++++++++++++++-
 1 file changed, 17 insertions(+), 1 deletion(-)

diff --git a/lisp/net/mailcap.el b/lisp/net/mailcap.el
index 54f7f416ab..a53e385662 100644
--- a/lisp/net/mailcap.el
+++ b/lisp/net/mailcap.el
@@ -1177,7 +1177,23 @@ See \"~/.mailcap\", `mailcap-mime-data' and related files and variables."
 		   (shell-quote-argument (convert-standard-filename file))
 		   command
 		   nil t))
-    (start-process-shell-command command nil command)))
+    ;; Handlers such as "gio open" and kde-open5 start viewer in background
+    ;; and exit immediately.  Avoid `start-process' since it assumes
+    ;; :connection-type 'pty and kills children processes with SIGHUP
+    ;; when temporary terminal session is finished (Bug#44824).
+    ;; An alternative is `process-connection-type' let-bound to nil for
+    ;; `start-process-shell-command' call (with no chance to report failure).
+    (make-process
+     :name "mailcap-view-file" :connection-type 'pipe :noquery t
+     :buffer nil ; "*Messages*" may be suitable for debugging
+     :sentinel (lambda (proc event)
+                 (when (and (memq (process-status proc) '(exit signal))
+                            (/= (process-exit-status proc) 0))
+                   (message
+                    "Command %s: %s."
+                    (mapconcat #'identity (process-command proc) " ")
+                    (substring event 0 -1))))
+     :command (list shell-file-name shell-command-switch command))))
 
 (provide 'mailcap)
 
-- 
2.25.1

bug#12972: [PATCH] Avoid regression in mailcap-view-file similar to Bug#44824

[Eli Zaretskii]

> From: Maxim Nikulin <manikulin@gmail.com>
> Date: Fri, 2 Jul 2021 00:01:59 +0700
> 
> --- a/lisp/net/mailcap.el
> +++ b/lisp/net/mailcap.el
> @@ -1177,7 +1177,23 @@ See \"~/.mailcap\", `mailcap-mime-data' and related files and variables."
>  		   (shell-quote-argument (convert-standard-filename file))
>  		   command
>  		   nil t))
> -    (start-process-shell-command command nil command)))
> +    ;; Handlers such as "gio open" and kde-open5 start viewer in background
> +    ;; and exit immediately.  Avoid `start-process' since it assumes
> +    ;; :connection-type 'pty and kills children processes with SIGHUP
> +    ;; when temporary terminal session is finished (Bug#44824).
> +    ;; An alternative is `process-connection-type' let-bound to nil for
> +    ;; `start-process-shell-command' call (with no chance to report failure).
> +    (make-process
> +     :name "mailcap-view-file" :connection-type 'pipe :noquery t
> +     :buffer nil ; "*Messages*" may be suitable for debugging
> +     :sentinel (lambda (proc event)
> +                 (when (and (memq (process-status proc) '(exit signal))
> +                            (/= (process-exit-status proc) 0))
> +                   (message
> +                    "Command %s: %s."
> +                    (mapconcat #'identity (process-command proc) " ")
> +                    (substring event 0 -1))))
> +     :command (list shell-file-name shell-command-switch command))))

I have two issues with this change:

First, you replace start-process-shell-command with make-process, and
I'm not sure I understand why.  If all you want is to use pipes, why
not simply bind process-connection-type around the call to
start-process-shell-command?  Does it not work for some reason?

And second, I'm not sure we should make this change unconditionally.
It isn't guaranteed that the handler will be one of those which have
the problem, is it?  And with other handlers, this could be an
incompatible behavior change if the handler behaves differently when
its standard handles are connected to a pipe rather than a terminal
device.  So I'd rather make this a conditional change, ideally only
when one of the affected handlers is used (assuming we can detect that
somehow).

Thanks.

bug#12972: [PATCH] Avoid regression in mailcap-view-file similar to Bug#44824

[Maxim Nikulin]

On 02/07/2021 01:38, Eli Zaretskii wrote:
>> From: Maxim Nikulin Date: Fri, 2 Jul 2021 00:01:59 +0700
>>
>> --- a/lisp/net/mailcap.el
>> +++ b/lisp/net/mailcap.el
>> -    (start-process-shell-command command nil command)))
...
>> +    (make-process
>> +     :name "mailcap-view-file" :connection-type 'pipe :noquery t
>> +     :buffer nil ; "*Messages*" may be suitable for debugging
>> +     :sentinel (lambda (proc event)
>> +                 (when (and (memq (process-status proc) '(exit signal))
>> +                            (/= (process-exit-status proc) 0))
>> +                   (message
>> +                    "Command %s: %s."
>> +                    (mapconcat #'identity (process-command proc) " ")
>> +                    (substring event 0 -1))))
>> +     :command (list shell-file-name shell-command-switch command))))
> 
> First, you replace start-process-shell-command with make-process, and
> I'm not sure I understand why.  If all you want is to use pipes, why
> not simply bind process-connection-type around the call to
> start-process-shell-command?

>> +    ;; An alternative is `process-connection-type' let-bound to nil for
>> +    ;; `start-process-shell-command' call (with no chance to report failure).
-----------------------------------------------^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

>> If main process of the handler fails then show a message with exit
>> reason.
---^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

> And with other handlers, this could be an
> incompatible behavior change if the handler behaves differently when
> its standard handles are connected to a pipe rather than a terminal
> device.

Example of such handler, please.

bug#12972: [PATCH] Avoid regression in mailcap-view-file similar to Bug#44824

[Eli Zaretskii]

> From: Maxim Nikulin <manikulin@gmail.com>
> Date: Fri, 2 Jul 2021 19:21:55 +0700
> Cc: Lars Ingebrigtsen <larsi@gnus.org>
> 
> > And with other handlers, this could be an
> > incompatible behavior change if the handler behaves differently when
> > its standard handles are connected to a pipe rather than a terminal
> > device.
> 
> Example of such handler, please.

Why do you need one?

bug#12972: [PATCH] Avoid regression in mailcap-view-file similar to Bug#44824

[Maxim Nikulin]

On 02/07/2021 19:37, Eli Zaretskii wrote:
>> From: Maxim Nikulin
>> Date: Fri, 2 Jul 2021 19:21:55 +0700
>> Cc: Lars Ingebrigtsen
>>
>>> And with other handlers, this could be an
>>> incompatible behavior change if the handler behaves differently when
>>> its standard handles are connected to a pipe rather than a terminal
>>> device.
>>
>> Example of such handler, please.
> 
> Why do you need one?

Because of I can not imagine such case for mailcap handler in Emacs yet 
and, accordingly to you, "this could be an incompatible behavior change".

bug#12972: [PATCH] Avoid regression in mailcap-view-file similar to Bug#44824

[Eli Zaretskii]

> From: Maxim Nikulin <manikulin@gmail.com>
> Date: Fri, 2 Jul 2021 23:24:23 +0700
> 
> On 02/07/2021 19:37, Eli Zaretskii wrote:
> >> From: Maxim Nikulin
> >> Date: Fri, 2 Jul 2021 19:21:55 +0700
> >> Cc: Lars Ingebrigtsen
> >>
> >>> And with other handlers, this could be an
> >>> incompatible behavior change if the handler behaves differently when
> >>> its standard handles are connected to a pipe rather than a terminal
> >>> device.
> >>
> >> Example of such handler, please.
> > 
> > Why do you need one?
> 
> Because of I can not imagine such case for mailcap handler in Emacs yet 
> and, accordingly to you, "this could be an incompatible behavior change".

You don't need to imagine it, you just need to trust me that I know
what I'm talking about: it would be an incompatible change.

Is it possible to detect that the handler is one of those that are
affected by this issue?  If so, let's do that; it cannot be worse than
what you suggested, or worse than the current situation.

bug#12972: [PATCH] Avoid regression in mailcap-view-file similar to Bug#44824

[Maxim Nikulin]

I am giving up with this issue.

My summary. New function `mailcap-view-file' has a problem similar to 
Bug#44824 (kde-open5 and "gio open" called directly or through xdg-open 
are unreliable in Emacs, I consider it as fixed in Org mode) that was 
reported by several users and refused by developers as not reproducible 
at first. I tried my best to draw attention to this problem and then to 
suggest a change that fixes the obscure problem and improves error 
handling. I am unaware of any real negative consequences of my patch. It 
is up to Emacs developers to leave the new bug as is or to fix it in a 
way they like

On 03/07/2021 00:28, Eli Zaretskii wrote:
>> From: Maxim Nikulin Date: Fri, 2 Jul 2021 23:24:23 +0700
>>
>> On 02/07/2021 19:37, Eli Zaretskii wrote:
>>>> From: Maxim Nikulin Date: Fri, 2 Jul 2021 19:21:55 +0700
>>>>
>>>>> And with other handlers, this could be an
>>>>> incompatible behavior change if the handler behaves differently when
>>>>> its standard handles are connected to a pipe rather than a terminal
>>>>> device.
>>>>
>>>> Example of such handler, please.
>>>
>>> Why do you need one?
>>
>> Because of I can not imagine such case for mailcap handler in Emacs yet
>> and, accordingly to you, "this could be an incompatible behavior change".
> 
> You don't need to imagine it, you just need to trust me that I know
> what I'm talking about: it would be an incompatible change.

Is it a kind of Church of Emacs that I have to just believe in you? 
Previous time you were trying to convince me that unconditional 'pipe is 
perfectly safe when I was unsure concerning behavior on Windows.

You prefer to keep reasons of your objections unveiled. I see no issue 
with the patch. It can be by a few lines shorter but the price is worse 
user experience. I have no idea how to move further.

Finally, the patch touches month-old unreleased code, so I see no point 
to discuss that it is "incompatible".

P.S. It was my fault to use `make-process' in Org since the function is 
not available in Emacs-24. I'm sorry for that incompatibility.

bug#12972: [PATCH] Avoid regression in mailcap-view-file similar to Bug#44824

[Eli Zaretskii]

> From: Maxim Nikulin <manikulin@gmail.com>
> Date: Sat, 3 Jul 2021 18:29:30 +0700
> 
> I am giving up with this issue.

That's too bad.  I see no reason to give up, and I urge you to
reconsider, please.

> >> Because of I can not imagine such case for mailcap handler in Emacs yet
> >> and, accordingly to you, "this could be an incompatible behavior change".
> > 
> > You don't need to imagine it, you just need to trust me that I know
> > what I'm talking about: it would be an incompatible change.
> 
> Is it a kind of Church of Emacs that I have to just believe in you? 

It isn't a church, but some kind of trust cannot harm.

> Previous time you were trying to convince me that unconditional 'pipe is 
> perfectly safe when I was unsure concerning behavior on Windows.

It is indeed safe for Windows, because Emacs on Windows always uses
pipes (as PTYs are not available there).

My concern here is for systems other than Windows and other than those
where you saw the issue.

Your patch unconditionally assumes that every handler will immediately
exit, and that it doesn't care about the connection type with the
parent Emacs process, but that is not guaranteed to be true.  What I'm
asking is to let some kind of "fire escape" for users who could be
adversely affected by this assumption.  Ideally, some automatic
detection of the handlers that need pipes would be the best.  If that
is not feasible, at least an option to control process-connection-type
would be enough.

> You prefer to keep reasons of your objections unveiled. I see no issue 
> with the patch. It can be by a few lines shorter but the price is worse 
> user experience. I have no idea how to move further.

I explained the issue I have with unconditionally changing the
interface.  I have explained it above again.  I hope it is clear
enough.

> Finally, the patch touches month-old unreleased code, so I see no point 
> to discuss that it is "incompatible".

Hmm... that's true.  So I guess an option to use PTYs should be good
enough here.

> P.S. It was my fault to use `make-process' in Org since the function is 
> not available in Emacs-24. I'm sorry for that incompatibility.

Great, thanks.  So I think it should be easy to adjust your patch to
have a variable that controls process-connection-type, and then it
could be installed.

bug#12972: [PATCH] Avoid regression in mailcap-view-file similar to Bug#44824

[Maxim Nikulin]

On 03/07/2021 18:56, Eli Zaretskii wrote:
>> From: Maxim Nikulin Date: Sat, 3 Jul 2021 18:29:30 +0700
>>
>> I am giving up with this issue.
> 
> That's too bad.  I see no reason to give up, and I urge you to
> reconsider, please.

Sorry, but the space of your assumptions and maybe confusions has too 
high number of dimension to realize what you actually mean and what you 
consider as a problem that should be fixed. So any further steps are 
impossible.

> Your patch unconditionally assumes that every handler will immediately
> exit, and that it doesn't care about the connection type with the
> parent Emacs process, but that is not guaranteed to be true.

There is no intention of such assumption and it *works* even for 
handlers that does not exit immediately.

I admit that I wrongly added ":noquery t", for some reason I believed 
that it allows to choose whether processes are allowed to exist longer 
than emacs or it is preferred to kill them with emacs. Actually 
asynchronous processes are killed always and the option manages the 
query only. This option should be dropped to restore compatibility with 
previous variant.

I have not found a way to detach asynchronous process from emacs. 
Surprisingly it is possible for synchronous processes but it is 
impossible to detect failure (thus to allow a user to figure out what 
has happened)

     (process-file-shell-command command nil 0 nil)

So process API in emacs is a kind of a short blanket.

Accidentally I have created an example of program that is incompatible 
with 'pipe asynchronous processes in emacs

#!/bin/sh
exec 1>&-
exec 2>&-
sleep 30

(let ((command "cpu-stress-test")
       (process-connection-type nil))
   (start-process-shell-command command nil command))

And emacs (at least 26.3) consumes 100% CPU for the specified amount of 
time. I do not see any reason to do so since the program does not do 
anything ugly. I have not found a way to explicitly force emacs to close 
pipes. That is why I consider it as an outstanding bug. Emacs must 
properly handle closed pipes.

So `process-file-shell-command' ... 0 is better than current 
`start-process-shell-command' but it does not allow to add error handling.

So besides that I still have no guess what problem you suspect, now I 
know that emacs may become mad in response to purely innocent action of 
a child process.

bug#12972: [PATCH] Avoid regression in mailcap-view-file similar to Bug#44824

[Eli Zaretskii]

> From: Maxim Nikulin <manikulin@gmail.com>
> Date: Sun, 4 Jul 2021 20:37:24 +0700
> 
> I admit that I wrongly added ":noquery t", for some reason I believed 
> that it allows to choose whether processes are allowed to exist longer 
> than emacs or it is preferred to kill them with emacs. Actually 
> asynchronous processes are killed always and the option manages the 
> query only. This option should be dropped to restore compatibility with 
> previous variant.
> 
> I have not found a way to detach asynchronous process from emacs. 
> Surprisingly it is possible for synchronous processes but it is 
> impossible to detect failure (thus to allow a user to figure out what 
> has happened)
> 
>      (process-file-shell-command command nil 0 nil)
> 
> So process API in emacs is a kind of a short blanket.
> 
> Accidentally I have created an example of program that is incompatible 
> with 'pipe asynchronous processes in emacs
> 
> #!/bin/sh
> exec 1>&-
> exec 2>&-
> sleep 30
> 
> (let ((command "cpu-stress-test")
>        (process-connection-type nil))
>    (start-process-shell-command command nil command))
> 
> And emacs (at least 26.3) consumes 100% CPU for the specified amount of 
> time. I do not see any reason to do so since the program does not do 
> anything ugly. I have not found a way to explicitly force emacs to close 
> pipes. That is why I consider it as an outstanding bug. Emacs must 
> properly handle closed pipes.
> 
> So `process-file-shell-command' ... 0 is better than current 
> `start-process-shell-command' but it does not allow to add error handling.
> 
> So besides that I still have no guess what problem you suspect, now I 
> know that emacs may become mad in response to purely innocent action of 
> a child process.

Sorry, I'm not sure I understand what this is all about.  Are you
still talking about the patch you proposed?

bug#12972: [PATCH] Avoid regression in mailcap-view-file similar to Bug#44824

[Maxim Nikulin]

By mistake I sent the message below as private one at first. However it 
actually does not add anything new to my previous comments to the bug.

On 04/07/2021 20:49, Eli Zaretskii wrote:
>> From: Maxim Nikulin Date: Sun, 4 Jul 2021 20:37:24 +0700
> 
> Sorry, I'm not sure I understand what this is all about.  Are you
> still talking about the patch you proposed?

Yes, I am. It is about proper way to a launch viewer in 
`mailcap-view-file'. Original `start-process-shell-command' with 'pty 
connection type prematurely kills children of kde-open5 or gio open. 
With 'pipe connection type it or `make-process' might make emacs CPU 
hungry if a child decides to close stdout and stderr:

>> #!/bin/sh
>> exec 1>&-
>> exec 2>&-
>> sleep 30

and finally `process-file-shell-command' does not allow to report 
failure. Moreover you suspect another secret compatibility problem with 
'pipe.

bug#12972: [PATCH] Avoid regression in mailcap-view-file similar to Bug#44824

[Eli Zaretskii]

> From: Maxim Nikulin <manikulin@gmail.com>
> Date: Mon, 5 Jul 2021 20:12:34 +0700
> 
> It is about proper way to a launch viewer in 
> `mailcap-view-file'. Original `start-process-shell-command' with 'pty 
> connection type prematurely kills children of kde-open5 or gio open. 
> With 'pipe connection type it or `make-process' might make emacs CPU 
> hungry if a child decides to close stdout and stderr:
> 
> >> #!/bin/sh
> >> exec 1>&-
> >> exec 2>&-
> >> sleep 30

Is the above something a file viewer is likely to do?

And if it does, how would you suggest to countermand that?

> and finally `process-file-shell-command' does not allow to report 
> failure.

The original code uses start-process-shell-command, and I don't think
I understand why you wanted to call process-file-shell-command
instead.  Can you explain?

bug#12972: 24.3.50; Move `org-open-file' and associated code out of Org mode

[Lars Ingebrigtsen]

Maxim Nikulin <manikulin@gmail.com> writes:

> I am attaching a patch similar to proposed to Org mode that should
> help to avoid obscure failures of viewers due to unnecessary terminal
> sessions.

Thanks; makes sense to me (and seems to fix these persistent issues with
xgd-open etc), so I've applied it to Emacs 28.

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

bug#12972: 24.3.50; Move `org-open-file' and associated code out of Org mode

[Maxim Nikulin]

On 30/07/2021 19:01, Lars Ingebrigtsen wrote:
> Maxim Nikulin writes:
> 
>> I am attaching a patch similar to proposed to Org mode that should
>> help to avoid obscure failures of viewers due to unnecessary terminal
>> sessions.
> 
> Thanks; makes sense to me (and seems to fix these persistent issues with
> xgd-open etc), so I've applied it to Emacs 28.

Thanks for looking into this issue. Please, consider the following 
additional change:

----- 8< -----
diff --git a/lisp/net/mailcap.el b/lisp/net/mailcap.el
index f64897ac9b..aeeb9bd8d3 100644
--- a/lisp/net/mailcap.el
+++ b/lisp/net/mailcap.el
@@ -1186,7 +1186,6 @@ See \"~/.mailcap\", `mailcap-mime-data' and 
related files and variables."
      (make-process
       :name "mailcap-view-file"
       :connection-type 'pipe
-     :noquery t
       :buffer nil ; "*Messages*" may be suitable for debugging
       :sentinel (lambda (proc event)
                   (when (and (memq (process-status proc) '(exit signal))
---- >8 ----

I did not update the patch since Eli had some objections (unclear to 
me). I wrongly added :noquery expecting to get a process completely 
detached from Emacs, something like result of "synchronous"

     (process-file-shell-command command nil 0 nil)

on linux (on stackoverflow I have seen a note that w32 requires special 
treatment).

Notice that "pipe" asynchronous Emacs processes have a problem with CPU 
consumption if a process does something like

#!/bin/sh
exec 1>&-
exec 2>&-
sleep 30

Though I believe even silently killed on exit children and CPU-hungry 
Emacs in rare cases are better than decade old pseudoterminal+SIGHUP 
problem with xdg-open. I suppose, it is an Emacs bug, note Gnome or KDE 
issue.
https://lists.gnu.org/archive/html/emacs-devel/2009-07/msg00279.html

bug#12972: 24.3.50; Move `org-open-file' and associated code out of Org mode

[Maxim Nikulin]

On 30/07/2021 19:01, Lars Ingebrigtsen wrote:
> Maxim Nikulin writes:
> 
>> I am attaching a patch similar to proposed to Org mode that should
>> help to avoid obscure failures of viewers due to unnecessary terminal
>> sessions.
> 
> Thanks; makes sense to me (and seems to fix these persistent issues with
> xgd-open etc), so I've applied it to Emacs 28.

Thanks for looking into this issue. Please, consider the following 
additional change:

----- 8< -----
diff --git a/lisp/net/mailcap.el b/lisp/net/mailcap.el
index f64897ac9b..aeeb9bd8d3 100644
--- a/lisp/net/mailcap.el
+++ b/lisp/net/mailcap.el
@@ -1186,7 +1186,6 @@ See \"~/.mailcap\", `mailcap-mime-data' and 
related files and variables."
      (make-process
       :name "mailcap-view-file"
       :connection-type 'pipe
-     :noquery t
       :buffer nil ; "*Messages*" may be suitable for debugging
       :sentinel (lambda (proc event)
                   (when (and (memq (process-status proc) '(exit signal))
---- >8 ----

I did not update the patch since Eli had some objections (unclear to 
me). I wrongly added :noquery expecting to get a process completely 
detached from Emacs, something like result of "synchronous"

     (process-file-shell-command command nil 0 nil)

on linux (on stackoverflow I have seen a note that w32 requires special 
treatment).

Notice that "pipe" asynchronous Emacs processes have a problem with CPU 
consumption if a process does something like

#!/bin/sh
exec 1>&-
exec 2>&-
sleep 30

Though I believe even silently killed on exit children and CPU-hungry 
Emacs in rare cases are better than decade old pseudoterminal+SIGHUP 
problem with xdg-open. I suppose, it is an Emacs bug, note Gnome or KDE 
issue.
https://lists.gnu.org/archive/html/emacs-devel/2009-07/msg00279.html

bug#12972: 24.3.50; Move `org-open-file' and associated code out of Org mode

[Lars Ingebrigtsen]

Maxim Nikulin <manikulin@gmail.com> writes:

> Thanks for looking into this issue. Please, consider the following
> additional change:

Thanks; applied to Emacs 28.

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

bug#12972: 24.3.50; Move `org-open-file' and associated code out of Org mode

[Lars Ingebrigtsen]

Maxim Nikulin <manikulin@gmail.com> writes:

> Thanks for looking into this issue. Please, consider the following
> additional change:

Thanks; applied to Emacs 28.

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