[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
bug#44824: [PATCH] org.el: Avoid xdg-open silent failure
From: |
Kyle Meyer |
Subject: |
bug#44824: [PATCH] org.el: Avoid xdg-open silent failure |
Date: |
Thu, 18 Mar 2021 23:50:06 -0400 |
Maxim Nikulin writes:
> org.el: Avoid xdg-open silent failure
>
> * lisp/org.el (org-open-file): Use 'pipe :connection-type instead of
> 'pty to prevent killing of background process on handler exit.
>
> Problem happens only in some desktop environments where configured
> through `org-file-apps' or mailcap handlers 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 launches 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, so the change
> does not affect "needsterminal" and "copiousoutput" mailcap features,
> they are not supported as earlier.
>
> If handler main process fails then show a message with exit reason.
> Output (including error messages) is ignored as before.
> Gtk application tends to report significant amount of failed asserts
> hardly informative for majority of users.
Thanks for the detailed commit message.
A few comments in addition to Eli's advice to drop the
(eq system-type 'gnu/linux) condition...
> diff --git a/lisp/org.el b/lisp/org.el
> index 7d8733448..a199a65c9 100644
> --- a/lisp/org.el
> +++ b/lisp/org.el
> @@ -8645,6 +8645,15 @@ opened in Emacs."
> (when add-auto-mode
> (mapcar (lambda (x) (cons (car x) 'emacs)) auto-mode-alist))))
>
> +(defun org--error-process-sentinel (proc event)
> + "Show a message if process failed (exited with non-zero code
> +or killed by a signal. Pass the function as :SENTINEL argument
Please rework the first sentence so that it fits on the first line,
though I'd be in favor dropping the function and using a lambda in the
make-process call.
> +of `make-process'."
> + (unless (string-match "finished" event)
There's no need for substring matching, right? So it could be
(equal event "finished\n")
Or perhaps
(when (and (memq (process-status proc) '(exit signal))
(/= (process-exit-status proc) 0))
...)
> + (message "Command %s: %s."
> + (mapconcat 'identity (process-command proc) " ")
s/'identity/#'identity/
> + (substring event 0 -1))))
> +
> ;;;###autoload
> (defun org-open-file (path &optional in-emacs line search)
> "Open the file at PATH.
> @@ -8766,7 +8775,17 @@ If the file does not exist, throw an error."
>
> (save-window-excursion
> (message "Running %s...done" cmd)
> - (start-process-shell-command cmd nil cmd)
> + (if (eq system-type 'gnu/linux)
> + ;; Handlers as "gio open" and kde-open5 start viewer in background
s/as/such as/ ?
> + ;; and exit immediately. Avoid start-process since it assumes
^ missing space
> + ;; :connection-type 'pty and kills children processes with SIGHUP
> + ;; when temporary terminal session is finished.
> + (make-process
> + :name "org-open-file" :connection-type 'pipe :noquery 't
s/'t/t/
> + :buffer nil ; use "*Messages*" for debugging
> + :sentinel 'org--error-process-sentinel
> + :command (list shell-file-name shell-command-switch cmd))
> + (start-process-shell-command cmd nil cmd))
> (and (boundp 'org-wait) (numberp org-wait) (sit-for org-wait))))
> ((or (stringp cmd)
> (eq cmd 'emacs))
Thanks.
- bug#44824: [PATCH] org.el: Avoid xdg-open silent failure,
Kyle Meyer <=