Ah my apologies. I forgot I had to use `file-name-directory' for creating the path to the temporary directory (too long ago since I did this). Here is the corrected version. --- a/lisp/org.el +++ b/lisp/org.el @@ -19340,7 +19340,7 @@ boundaries." (not (cdr (org-element-contents parent))))) (org-string-match-p file-extension-re (org-element-property :path link))) - (let ((file (expand-file-name (org-element-property :path link)))) + (let ((file (substitute-in-file-name (expand-file-name (org-element-property :path link))))) (when (file-exists-p file) (let ((width ;; Apply `org-image-actual-width' specifications. @@ -19378,10 +19378,25 @@ boundaries." 'org-image-overlay))) (if (and (car-safe old) refresh) (image-refresh (overlay-get (cdr old) 'display)) - (let ((image (create-image file - (and width 'imagemagick) - nil - :width width))) + (let ((image + (create-image (if (org-file-remote-p file) + (let* ((tramp-tmpdir (concat + (if (featurep 'xemacs) + (temp-directory) + temporary-file-directory) + "/tramp" + (file-name-directory (expand-file-name file)))) + (newname (concat + tramp-tmpdir + (file-name-nondirectory (expand-file-name file))))) + (make-directory tramp-tmpdir t) + (if (file-newer-than-file-p file newname) + (copy-file file newname t t)) + newname) + file) + (and width 'imagemagick) + nil + :width width))) (when image (let* ((link ;; If inline image is the description On Tue, Nov 25, 2014 at 10:15 AM, Kit-Yan Choi wrote: > Thank you for your comments! They are very helpful. > > > 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. > > I personally needed this functionality. I have tried to reduce the amount > of time spent on fetching the images by checking whether the images have > been fetched before and whether the remote files are newer. Yes these > communications take time as it should be expected if one opens an org file > remotely (therefore connection should have been made) or when one specifies > a remote image as path and wants to display it inline. > Perhaps I could add an option flag or ask a question before fetching for > user to decide whether to display remote images or not? In case the > behaviour of displaying remote images inline is not desired? One scenario > I can think of is that `org-startup-with-inline-images' is set to true and > the file is sometimes visited remotely. > Any opinion or comment on this, please? > > >> - (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? > > Because the file path for a remote file, as far as I have tested, have > redundant slashes "/" at the beginning of the path which makes > org-file-remote-p to return nil for a remote path. > `substitute-in-file-name' corrects such path. `substitute-in-file-name' > is also used in `org-open-file'. So I followed suit. > > > Are you sure the return value (a boolean, i.e., not necessarily > > a string) should belong to the `concat'? > > Good point. I changed the code (see below, and attached). > > > (create-image (if (org-file-remote-p file) ...) > > (and width 'imagemagick) > > nil > > :width width) > > I agree. Thanks. I made the code cleaner now (see below, and attached). > > > @@ -19340,7 +19340,7 @@ boundaries." > (not (cdr (org-element-contents parent))))) > (org-string-match-p file-extension-re > (org-element-property :path link))) > - (let ((file (expand-file-name (org-element-property :path link)))) > + (let ((file (substitute-in-file-name (expand-file-name > (org-element-property :path link))))) > (when (file-exists-p file) > (let ((width > ;; Apply `org-image-actual-width' specifications. > @@ -19378,10 +19378,24 @@ boundaries." > 'org-image-overlay))) > (if (and (car-safe old) refresh) > (image-refresh (overlay-get (cdr old) 'display)) > - (let ((image (create-image file > - (and width 'imagemagick) > - nil > - :width width))) > + (let ((image > + (create-image (if (org-file-remote-p file) > + (let* ((tramp-tmpdir (concat > + (if (featurep 'xemacs) > + (temp-directory) > + temporary-file-directory) > + "/tramp")) > + (newname (concat > + tramp-tmpdir > + (expand-file-name file)))) > + (make-directory tramp-tmpdir t) > + (if (file-newer-than-file-p file newname) > + (copy-file file newname t t)) > + newname) > + file) > + (and width 'imagemagick) > + nil > + :width width))) > (when image > (let* ((link > ;; If inline image is the description > > > > On Tue, Nov 25, 2014 at 3:32 AM, Nicolas Goaziou > wrote: > >> Hello, >> >> Kit-Yan Choi 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 >> > >