[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
bug#44824: [PATCH] org.el: Avoid xdg-open silent failure
From: |
Maxim Nikulin |
Subject: |
bug#44824: [PATCH] org.el: Avoid xdg-open silent failure |
Date: |
Sat, 20 Mar 2021 22:45:16 +0700 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.7.1 |
On 19/03/2021 10:50, Kyle Meyer wrote:
Maxim Nikulin writes:
A few comments in addition to Eli's advice to drop the
(eq system-type 'gnu/linux) condition...
Feel free to commit the change suggested in
https://debbugs.gnu.org/cgi/bugreport.cgi?bug=44824#82
instead of this patch.
+(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.
My impression is that org-open-file function is already too long and
complex. Another reason to use standalone function is that I am unsure
if elisp compiler and interpreter are smart enough to reuse single
instance of lambda. I was afraid that every opened file caused creation
of new sentinel possibly with a closure containing chain of stack
frames. On the other hand even in worst case memory footprint is
negligible in comparison to any GUI viewer.
+ (unless (string-match "finished" event)
There's no need for substring matching, right? So it could be
(equal event "finished\n")
I was surprised by final "\n" that is not always suitable and I was in
doubts concerning its stability. I would prefer something like
(starts-with event "finished")
Certainly match-data is not necessary, so even match-string-p is better.
(when (and (memq (process-status proc) '(exit signal))
(/= (process-exit-status proc) 0))
Thank you, I was too lazy to implement such kind of check myself.
Certainly this variant is better.
I hope, I have addressed other your comments in the updated patch.
org-open-file-make-process-v2.patch
Description: Text Data