On 13/05/2022 05:52, Paul Eggert wrote:
On 5/12/22 09:55, Max Nikulin wrote:
+ (unless (file-exists-p file)
+ (error "File to tangle does not exist: %s" file))
+ (when (file-newer-than-file-p file tangled-file)
(org-babel-tangle-file file ...
file-newer-than-file-p succeeds only if FILE exists, so in that case
it'd be a bit more efficient to avoid testing FILE's existence again, e.g.:
(cond
((file-newer-than-file-p file tangled-file)
(org-bable-tangle-file file ...))
((not (file-exists-p file))
(error "File to tangle does not exist: %s" file)))
My opinion is that performance improvement is negligible while negative
impact related to code readability is noticeable. Maybe it is just
because elisp is not my favorite language.
Feel free to commit your variant though, I will not object, but I am not
going to update my patch in this way as well.
P.S. Since I believe it should be fixed in the bugfix branch, I tried to
keep changes as minimal as possible.
I am not sure which kind of code I would prefer to see. I do not like
`cond' with 2 branches. Even nested ifs are a bit better while still to
"heavy"
(if (not (file-newer-than-file-p file tangled-file))
(unless (file-exists-p file)
(error ...))
(org-babel-tangle-file ...)) ; intentionally put below to the
else branch
Maybe I would prefer something like file-target-is-up-to-date-p
predicate that returns nil if target does not exist. For some reason
`org-babel-tangle-file' silently ignores missed file neither signalling
a error nor returning a special value. Delegating error handling to
`org-babel-tangle-file' would allow to get consistent error messages.
In the previous version of the patch (with inline implementation of file
modification time comparison) the suggested optimization was quite
natural, with `file-newer-than-file-p' it is trade off with code
readability.
