GNU bug report logs - #44824
27.1; Org export as pdf and open file does not open it

Previous Next

Full log

Message #121 received at 44824 <at> debbugs.gnu.org (full text, mbox):

From: Maxim Nikulin <m.a.nikulin <at> gmail.com>
Cc: 44824 <at> debbugs.gnu.org
Subject: Re: bug#44824: [PATCH] org.el: Avoid xdg-open silent failure
Date: Sat, 20 Mar 2021 22:45:16 +0700

[Message part 1 (text/plain, inline)]

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 (text/x-patch, attachment)]

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.