emacs-orgmode
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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


From: Eli Zaretskii
Subject: bug#12972: [PATCH] Avoid regression in mailcap-view-file similar to Bug#44824
Date: Thu, 01 Jul 2021 21:38:15 +0300

> 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.





reply via email to

[Prev in Thread] Current Thread [Next in Thread]