Hello, Kit-Yan Choi <k...@kychoi.org> writes:
> I would like to submit a patch to support displaying remote images inline > in Org-mode. Attached is the formatted patch (or github branch here > <https://github.com/kitchoi/org-mode/commit/2e600da455c371754f028ddaaed1ae1724cbe6b6> > .) Thanks for your patch. However, I wonder if we really want this. Remote images could be slow to fetch, and it would make buffer unusable. > Feedbacks are most welcome. Thanks. Some comments follow. > - (let ((file (expand-file-name (org-element-property :path link)))) > + (let ((file (substitute-in-file-name (expand-file-name > (org-element-property :path link))))) Why is it needed? > + (let* ((image > + (let ((newname > + (if (org-file-remote-p file) > + (let* ((tramp-tmpdir (concat > + (if (featurep > 'xemacs) > + > (temp-directory) > + > temporary-file-directory) > + "/tramp" > + > (org-file-remote-p file) ^^^^^^^^^^^^^^^^^^^^^^^^ Are you sure the return value (a boolean, i.e., not necessarily a string) should belong to the `concat'? > + > (file-name-directory > + > (org-babel-local-file-name file)))) > + (newname (concat > + tramp-tmpdir > + > (file-name-nondirectory file)))) > + (make-directory tramp-tmpdir t) > + (if > (tramp-handle-file-newer-than-file-p file newname) > + (tramp-compat-copy-file file > newname t t)) > + newname) > + file))) > + (create-image newname > + (and width 'imagemagick) > + nil > + :width width)))) IMO, it is clearer to use (create-image (if (org-file-remote-p file) ...) (and width 'imagemagick) nil :width width) Regards, -- Nicolas Goaziou