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 <m...@nicolasgoaziou.fr>
wrote:
> 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
>
diff --git a/lisp/org.el b/lisp/org.el
index 6ab13f4..96b04b6 100755
--- 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,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
