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 <[email protected]> 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 <[email protected]>
> wrote:
>
>> Hello,
>>
>> Kit-Yan Choi <[email protected]> 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..44ca7d6 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,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
