From mboxrd@z Thu Jan 1 00:00:00 1970 From: Kit-Yan Choi Subject: [Patch] org-display-inline-images: Add support for remote images Date: Fri, 21 Nov 2014 23:43:17 -0500 Message-ID: Mime-Version: 1.0 Content-Type: multipart/mixed; boundary=001a11c2531838a93805086b3389 Return-path: Received: from eggs.gnu.org ([2001:4830:134:3::10]:43824) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Xs2XX-0001CX-5W for emacs-orgmode@gnu.org; Fri, 21 Nov 2014 23:43:20 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Xs2XW-0006iY-18 for emacs-orgmode@gnu.org; Fri, 21 Nov 2014 23:43:19 -0500 Received: from mail-qc0-x233.google.com ([2607:f8b0:400d:c01::233]:40282) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Xs2XV-0006iU-SF for emacs-orgmode@gnu.org; Fri, 21 Nov 2014 23:43:17 -0500 Received: by mail-qc0-f179.google.com with SMTP id c9so4649962qcz.38 for ; Fri, 21 Nov 2014 20:43:17 -0800 (PST) List-Id: "General discussions about Org-mode." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: emacs-orgmode-bounces+geo-emacs-orgmode=m.gmane.org@gnu.org Sender: emacs-orgmode-bounces+geo-emacs-orgmode=m.gmane.org@gnu.org To: emacs-orgmode@gnu.org --001a11c2531838a93805086b3389 Content-Type: multipart/alternative; boundary=001a11c2531838a93205086b3387 --001a11c2531838a93205086b3387 Content-Type: text/plain; charset=UTF-8 Hi, 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 .) I have tested the code with "make test". FSF document is signed (waiting for gnu to send it back). Feedbacks are most welcome. Thanks. Kit --001a11c2531838a93205086b3387 Content-Type: text/html; charset=UTF-8 Content-Transfer-Encoding: quoted-printable
Hi,

I would like to submit a patch to s= upport displaying remote images inline in Org-mode.=C2=A0 Attached is the f= ormatted patch (or github branch here.)
=
I have tested the code with "make test".=C2=A0 FSF= document is signed (waiting for gnu to send it back).

Feedbacks are most welcome.=C2=A0 Thanks.

Kit

--001a11c2531838a93205086b3387-- --001a11c2531838a93805086b3389 Content-Type: text/plain; charset=US-ASCII; name="0001-org.el.diff" Content-Disposition: attachment; filename="0001-org.el.diff" Content-Transfer-Encoding: base64 X-Attachment-Id: f_i2shpa2p0 ZGlmZiAtLWdpdCBhL2xpc3Avb3JnLmVsIGIvbGlzcC9vcmcuZWwKaW5kZXggMWYzM2QyYS4uYWFh NWM5YiAxMDA3NTUKLS0tIGEvbGlzcC9vcmcuZWwKKysrIGIvbGlzcC9vcmcuZWwKQEAgLTE5MzM4 LDcgKzE5MzM4LDcgQEAgYm91bmRhcmllcy4iCiAJCQkgICAgKG5vdCAoY2RyIChvcmctZWxlbWVu dC1jb250ZW50cyBwYXJlbnQpKSkpKQogCQkgICAgICAob3JnLXN0cmluZy1tYXRjaC1wIGZpbGUt ZXh0ZW5zaW9uLXJlCiAJCQkJCSAgKG9yZy1lbGVtZW50LXByb3BlcnR5IDpwYXRoIGxpbmspKSkK LQkgICAgIChsZXQgKChmaWxlIChleHBhbmQtZmlsZS1uYW1lIChvcmctZWxlbWVudC1wcm9wZXJ0 eSA6cGF0aCBsaW5rKSkpKQorCSAgICAgKGxldCAoKGZpbGUgKHN1YnN0aXR1dGUtaW4tZmlsZS1u YW1lIChleHBhbmQtZmlsZS1uYW1lIChvcmctZWxlbWVudC1wcm9wZXJ0eSA6cGF0aCBsaW5rKSkp KSkKIAkgICAgICAgKHdoZW4gKGZpbGUtZXhpc3RzLXAgZmlsZSkKIAkJIChsZXQgKCh3aWR0aAog CQkJOzsgQXBwbHkgYG9yZy1pbWFnZS1hY3R1YWwtd2lkdGgnIHNwZWNpZmljYXRpb25zLgpAQCAt MTkzNzYsMTAgKzE5Mzc2LDI5IEBAIGJvdW5kYXJpZXMuIgogCQkJICAgICAnb3JnLWltYWdlLW92 ZXJsYXkpKSkKIAkJICAgKGlmIChhbmQgKGNhci1zYWZlIG9sZCkgcmVmcmVzaCkKIAkJICAgICAg IChpbWFnZS1yZWZyZXNoIChvdmVybGF5LWdldCAoY2RyIG9sZCkgJ2Rpc3BsYXkpKQotCQkgICAg IChsZXQgKChpbWFnZSAoY3JlYXRlLWltYWdlIGZpbGUKLQkJCQkJCSAgKGFuZCB3aWR0aCAnaW1h Z2VtYWdpY2spCi0JCQkJCQkgIG5pbAotCQkJCQkJICA6d2lkdGggd2lkdGgpKSkKKwkJICAgICAo bGV0KiAoKGltYWdlIAorCQkJICAgICAobGV0ICgobmV3bmFtZQorCQkJCSAgICAoaWYgKG9yZy1m aWxlLXJlbW90ZS1wIGZpbGUpCisJCQkJCShsZXQqICgodHJhbXAtdG1wZGlyIChjb25jYXQKKwkJ CQkJCQkgICAgICAoaWYgKGZlYXR1cmVwICd4ZW1hY3MpCisJCQkJCQkJCSAgKHRlbXAtZGlyZWN0 b3J5KQorCQkJCQkJCQl0ZW1wb3JhcnktZmlsZS1kaXJlY3RvcnkpCisJCQkJCQkJICAgICAgIi90 cmFtcCIKKwkJCQkJCQkgICAgICAob3JnLWZpbGUtcmVtb3RlLXAgZmlsZSkKKwkJCQkJCQkgICAg ICAoZmlsZS1uYW1lLWRpcmVjdG9yeQorCQkJCQkJCSAgICAgICAob3JnLWJhYmVsLWxvY2FsLWZp bGUtbmFtZSBmaWxlKSkpKQorCQkJCQkgICAgICAgKG5ld25hbWUgKGNvbmNhdAorCQkJCQkJCSB0 cmFtcC10bXBkaXIgCisJCQkJCQkJIChmaWxlLW5hbWUtbm9uZGlyZWN0b3J5IGZpbGUpKSkpCisJ CQkJCSAgKG1ha2UtZGlyZWN0b3J5IHRyYW1wLXRtcGRpciB0KQorCQkJCQkgIChpZiAodHJhbXAt aGFuZGxlLWZpbGUtbmV3ZXItdGhhbi1maWxlLXAgZmlsZSBuZXduYW1lKQorCQkJCQkJKHRyYW1w LWNvbXBhdC1jb3B5LWZpbGUgZmlsZSBuZXduYW1lIHQgdCkpCisJCQkJCSAgbmV3bmFtZSkKKwkJ CQkgICAgICBmaWxlKSkpCisJCQkgICAgICAgKGNyZWF0ZS1pbWFnZSBuZXduYW1lCisJCQkJCSAg ICAgKGFuZCB3aWR0aCAnaW1hZ2VtYWdpY2spCisJCQkJCSAgICAgbmlsCisJCQkJCSAgICAgOndp ZHRoIHdpZHRoKSkpKQogCQkgICAgICAgKHdoZW4gaW1hZ2UKIAkJCSAobGV0KiAoKGxpbmsKIAkJ CQkgOzsgSWYgaW5saW5lIGltYWdlIGlzIHRoZSBkZXNjcmlwdGlvbgo= --001a11c2531838a93805086b3389-- From mboxrd@z Thu Jan 1 00:00:00 1970 From: Nicolas Goaziou Subject: Re: [Patch] org-display-inline-images: Add support for remote images Date: Tue, 25 Nov 2014 09:32:47 +0100 Message-ID: <87vbm3of2o.fsf@selenimh.mobile.lan> References: Mime-Version: 1.0 Content-Type: text/plain Return-path: Received: from eggs.gnu.org ([2001:4830:134:3::10]:55726) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XtBY0-0004Gb-4k for emacs-orgmode@gnu.org; Tue, 25 Nov 2014 03:32:38 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1XtBXt-0002H1-Hs for emacs-orgmode@gnu.org; Tue, 25 Nov 2014 03:32:32 -0500 Received: from relay6-d.mail.gandi.net ([2001:4b98:c:538::198]:45235) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XtBXt-0002Gv-Bo for emacs-orgmode@gnu.org; Tue, 25 Nov 2014 03:32:25 -0500 In-Reply-To: (Kit-Yan Choi's message of "Fri, 21 Nov 2014 23:43:17 -0500") List-Id: "General discussions about Org-mode." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: emacs-orgmode-bounces+geo-emacs-orgmode=m.gmane.org@gnu.org Sender: emacs-orgmode-bounces+geo-emacs-orgmode=m.gmane.org@gnu.org To: Kit-Yan Choi Cc: emacs-orgmode@gnu.org 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 > > .) 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 From mboxrd@z Thu Jan 1 00:00:00 1970 From: Kit-Yan Choi Subject: Re: [Patch] org-display-inline-images: Add support for remote images Date: Tue, 25 Nov 2014 10:15:57 -0500 Message-ID: References: <87vbm3of2o.fsf@selenimh.mobile.lan> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary=001a11c3ec6c5837430508b063bc Return-path: Received: from eggs.gnu.org ([2001:4830:134:3::10]:47480) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XtHqS-00020R-TO for emacs-orgmode@gnu.org; Tue, 25 Nov 2014 10:16:07 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1XtHqQ-0001XM-I1 for emacs-orgmode@gnu.org; Tue, 25 Nov 2014 10:16:00 -0500 Received: from mail-qa0-x229.google.com ([2607:f8b0:400d:c00::229]:35250) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XtHqQ-0001XE-AR for emacs-orgmode@gnu.org; Tue, 25 Nov 2014 10:15:58 -0500 Received: by mail-qa0-f41.google.com with SMTP id f12so525352qad.14 for ; Tue, 25 Nov 2014 07:15:57 -0800 (PST) In-Reply-To: <87vbm3of2o.fsf@selenimh.mobile.lan> List-Id: "General discussions about Org-mode." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: emacs-orgmode-bounces+geo-emacs-orgmode=m.gmane.org@gnu.org Sender: emacs-orgmode-bounces+geo-emacs-orgmode=m.gmane.org@gnu.org To: Kit-Yan Choi , emacs-orgmode@gnu.org --001a11c3ec6c5837430508b063bc Content-Type: multipart/alternative; boundary=001a11c3ec6c58373b0508b063ba --001a11c3ec6c58373b0508b063ba Content-Type: text/plain; charset=UTF-8 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 > --001a11c3ec6c58373b0508b063ba Content-Type: text/html; charset=UTF-8 Content-Transfer-Encoding: quoted-printable
Thank you for your comments!=C2=A0 They are very helpful.<= div>
> Thanks for your patch. However, I wonder if we real= ly want this. Remote
> images could be slow to fetch, and it would = make buffer unusable.

I personally needed this funct= ionality.=C2=A0 I have tried to reduce the amount of time spent on fetching= the images by checking whether the images have been fetched before and whe= ther the remote files are newer.=C2=A0 Yes these communications take time a= s it should be expected if one opens an org file remotely (therefore connec= tion should have been made) or when one specifies a remote image as path an= d 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 rem= ote images or not?=C2=A0 In case the behaviour of displaying remote images = inline is not desired?=C2=A0 One scenario I can think of is that `org-start= up-with-inline-images' is set to true and the file is sometimes visited= remotely. =C2=A0
Any opinion or comment on this, please?

>> -=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 (let ((file = (expand-file-name (org-element-property :path link))))
>> +=C2=A0 = =C2=A0 =C2=A0 =C2=A0 =C2=A0 (let ((file (substitute-in-file-name (expand-fi= le-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 p= ath which makes org-file-remote-p to return nil for a remote path.
`substitute-in-file-name' corrects such path. =C2=A0`substitute-in-fi= le-name' is also used in `org-open-file'.=C2=A0 So I followed suit.=

> Are you sure the return value (a boolean, i.= e., not necessarily
> a string) should belong to the `concat'?<= div>
Good point.=C2=A0 I changed the code (see below, and att= ached).

> =C2=A0(create-image (if (org-file-rem= ote-p file) ...)
> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0(and width 'imagemagick)
> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 =C2=A0nil
> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0:width width)

I agree.=C2=A0 Thanks.=C2= =A0 I made the code cleaner now (see below, and attached).


@@ -19340,7 +19340,7 @@ boundaries= ."
=C2=A0 =C2=A0 =C2=A0(not (cdr (org-element-contents paren= t)))))
=C2=A0 =C2=A0 =C2=A0 =C2=A0(org-string-match-p file-extensi= on-re
=C2=A0 =C2=A0(org-element-property :path link)))
- =C2=A0 =C2=A0 (let ((file (expand-file-name (org-element-property :path l= ink))))
+ =C2=A0 =C2=A0 (let ((file (substitute-in-file-name (expan= d-file-name (org-element-property :path link)))))
=C2=A0 =C2=A0 =C2= =A0 =C2=A0 (when (file-exists-p file)
=C2= =A0 (let ((width
<= div class=3D"gmail_extra">=C2=A0= ;; Apply `org-image-actual-width' specifications.
@@ -19378,10 +19378,24 @@ boundaries."
=C2=A0 <= /span> =C2=A0 =C2=A0 'org-image-overlay)))
=C2=A0 =C2=A0 (if (= and (car-safe old) refresh)
=C2=A0 =C2=A0 =C2=A0 =C2=A0 (image-ref= resh (overlay-get (cdr old) 'display))
= - =C2=A0 =C2=A0 (let ((= image (create-image file
- =C2=A0(and width 'imagemagick)<= /div>
- =C2=A0nil
- =C2=A0:width width)))
+ = =C2=A0 =C2=A0 (let ((image=C2=A0
+ =C2=A0 =C2=A0(create-image (if= (org-file-remote-p file)
+ =C2=A0 =C2=A0 =C2=A0(let* ((tramp-t= mpdir (concat
+ =C2=A0 =C2=A0(if (featurep 'xemacs)
+ = (temp-directory)
+ =C2=A0 =C2=A0 =C2=A0tempora= ry-file-directory)
+ =C2=A0 =C2=A0"/tramp"))
+ = =C2=A0 =C2=A0 (newname (concat
+= =C2=A0 =C2=A0 =C2= =A0 tramp-tmpdir=C2=A0
+ =C2=A0 =C2=A0 =C2=A0 (expand-file-na= me file))))
+ (make-directory tramp-tmpdir t)
+ (= if (file-newer-than-file-p file newname)
+<= span class=3D"" style=3D"white-space:pre"> =C2=A0 =C2=A0(copy-= file file newname t t))
+ newname)
+ =C2=A0 =C2=A0f= ile)
+ =C2=A0(and width 'imagemagick)
+ =C2=A0n= il
+ =C2=A0:width width)))
=C2= =A0 =C2=A0 =C2=A0 =C2= =A0 (when image
=C2=A0 (let* ((link
=C2=A0 ;; If inlin= e image is the description



On Tue, Nov 25, 2014 at 3:32 AM, Nicolas Goaziou <= mail@nicolasgoa= ziou.fr> wrote:
Hello,

Kit-Yan Choi <kit@kychoi.org> w= rites:

> I would like to submit a patch to support displaying remote images inl= ine
> in Org-mode.=C2=A0 Attached is the formatted patch (or github branch h= ere
> <https://github.com/k= itchoi/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.=C2=A0 Thanks.

Some comments follow.

> -=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 (let ((file (expand-file-name (org= -element-property :path link))))
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 (let ((file (substitute-in-file-na= me (expand-file-name (org-element-property :path link)))))

Why is it needed?

> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 (let* = ((image
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0 =C2=A0 =C2=A0 (let ((newname
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0(if (org-file-remote-p fil= e)
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0(let* ((tram= p-tmpdir (concat
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0(if (feat= urep 'xemacs)
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0(temp-directory)
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0te= mporary-file-directory)
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0"/tr= amp"
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0(org-file= -remote-p file)
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 ^^^^^= ^^^^^^^^^^^^^^^^^^^
Are you sure the return value (a boolean, i.e., not necessarily
a string) should belong to the `concat'?

> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0(file-nam= e-directory
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 (org-bab= el-local-file-name file))))
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 (newname (concat
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 tramp-tmpdir
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 (file-name-nondirectory file)= )))
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0(make= -directory tramp-tmpdir t)
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0(if (= tramp-handle-file-newer-than-file-p file newname)
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0(tramp-compat-copy-file file newname t t))
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0newna= me)
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0file)))
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0 =C2=A0 =C2=A0 =C2=A0 (create-image newname
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 (and width 'imagemagick)
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 nil
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 :width width))))

IMO, it is clearer to use

=C2=A0 (create-image (if (org-file-remote-p file) ...)
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 (and width 'ima= gemagick)
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 nil
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 :width width)



Regards,

--
Nicolas Goaziou

--001a11c3ec6c58373b0508b063ba-- --001a11c3ec6c5837430508b063bc Content-Type: text/plain; charset=US-ASCII; name="0001-org.el.diff" Content-Disposition: attachment; filename="0001-org.el.diff" Content-Transfer-Encoding: base64 X-Attachment-Id: f_i2xeo2ys0 ZGlmZiAtLWdpdCBhL2xpc3Avb3JnLmVsIGIvbGlzcC9vcmcuZWwKaW5kZXggNmFiMTNmNC4uOTZi MDRiNiAxMDA3NTUKLS0tIGEvbGlzcC9vcmcuZWwKKysrIGIvbGlzcC9vcmcuZWwKQEAgLTE5MzQw LDcgKzE5MzQwLDcgQEAgYm91bmRhcmllcy4iCiAJCQkgICAgKG5vdCAoY2RyIChvcmctZWxlbWVu dC1jb250ZW50cyBwYXJlbnQpKSkpKQogCQkgICAgICAob3JnLXN0cmluZy1tYXRjaC1wIGZpbGUt ZXh0ZW5zaW9uLXJlCiAJCQkJCSAgKG9yZy1lbGVtZW50LXByb3BlcnR5IDpwYXRoIGxpbmspKSkK LQkgICAgIChsZXQgKChmaWxlIChleHBhbmQtZmlsZS1uYW1lIChvcmctZWxlbWVudC1wcm9wZXJ0 eSA6cGF0aCBsaW5rKSkpKQorCSAgICAgKGxldCAoKGZpbGUgKHN1YnN0aXR1dGUtaW4tZmlsZS1u YW1lIChleHBhbmQtZmlsZS1uYW1lIChvcmctZWxlbWVudC1wcm9wZXJ0eSA6cGF0aCBsaW5rKSkp KSkKIAkgICAgICAgKHdoZW4gKGZpbGUtZXhpc3RzLXAgZmlsZSkKIAkJIChsZXQgKCh3aWR0aAog CQkJOzsgQXBwbHkgYG9yZy1pbWFnZS1hY3R1YWwtd2lkdGgnIHNwZWNpZmljYXRpb25zLgpAQCAt MTkzNzgsMTAgKzE5Mzc4LDI0IEBAIGJvdW5kYXJpZXMuIgogCQkJICAgICAnb3JnLWltYWdlLW92 ZXJsYXkpKSkKIAkJICAgKGlmIChhbmQgKGNhci1zYWZlIG9sZCkgcmVmcmVzaCkKIAkJICAgICAg IChpbWFnZS1yZWZyZXNoIChvdmVybGF5LWdldCAoY2RyIG9sZCkgJ2Rpc3BsYXkpKQotCQkgICAg IChsZXQgKChpbWFnZSAoY3JlYXRlLWltYWdlIGZpbGUKLQkJCQkJCSAgKGFuZCB3aWR0aCAnaW1h Z2VtYWdpY2spCi0JCQkJCQkgIG5pbAotCQkJCQkJICA6d2lkdGggd2lkdGgpKSkKKwkJICAgICAo bGV0ICgoaW1hZ2UgCisJCQkgICAgKGNyZWF0ZS1pbWFnZSAoaWYgKG9yZy1maWxlLXJlbW90ZS1w IGZpbGUpCisJCQkJCSAgICAgIChsZXQqICgodHJhbXAtdG1wZGlyIChjb25jYXQKKwkJCQkJCQkJ ICAgIChpZiAoZmVhdHVyZXAgJ3hlbWFjcykKKwkJCQkJCQkJCSh0ZW1wLWRpcmVjdG9yeSkKKwkJ CQkJCQkJICAgICAgdGVtcG9yYXJ5LWZpbGUtZGlyZWN0b3J5KQorCQkJCQkJCQkgICAgIi90cmFt cCIpKQorCQkJCQkJICAgICAobmV3bmFtZSAoY29uY2F0CisJCQkJCQkJICAgICAgIHRyYW1wLXRt cGRpciAKKwkJCQkJCQkgICAgICAgKGV4cGFuZC1maWxlLW5hbWUgZmlsZSkpKSkKKwkJCQkJCSht YWtlLWRpcmVjdG9yeSB0cmFtcC10bXBkaXIgdCkKKwkJCQkJCShpZiAoZmlsZS1uZXdlci10aGFu LWZpbGUtcCBmaWxlIG5ld25hbWUpCisJCQkJCQkgICAgKGNvcHktZmlsZSBmaWxlIG5ld25hbWUg dCB0KSkKKwkJCQkJCW5ld25hbWUpCisJCQkJCSAgICBmaWxlKQorCQkJCQkgIChhbmQgd2lkdGgg J2ltYWdlbWFnaWNrKQorCQkJCQkgIG5pbAorCQkJCQkgIDp3aWR0aCB3aWR0aCkpKQogCQkgICAg ICAgKHdoZW4gaW1hZ2UKIAkJCSAobGV0KiAoKGxpbmsKIAkJCQkgOzsgSWYgaW5saW5lIGltYWdl IGlzIHRoZSBkZXNjcmlwdGlvbgo= --001a11c3ec6c5837430508b063bc-- From mboxrd@z Thu Jan 1 00:00:00 1970 From: Rasmus Subject: Re: [Patch] org-display-inline-images: Add support for remote images Date: Tue, 25 Nov 2014 16:23:35 +0100 Message-ID: <873897718o.fsf@gmx.us> References: <87vbm3of2o.fsf@selenimh.mobile.lan> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit Return-path: Received: from eggs.gnu.org ([2001:4830:134:3::10]:49751) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XtHyA-0004u1-O8 for emacs-orgmode@gnu.org; Tue, 25 Nov 2014 10:24:05 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1XtHy4-00053I-D9 for emacs-orgmode@gnu.org; Tue, 25 Nov 2014 10:23:58 -0500 Received: from plane.gmane.org ([80.91.229.3]:40925) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XtHy4-00053B-5V for emacs-orgmode@gnu.org; Tue, 25 Nov 2014 10:23:52 -0500 Received: from list by plane.gmane.org with local (Exim 4.69) (envelope-from ) id 1XtHy2-0007Rp-Rg for emacs-orgmode@gnu.org; Tue, 25 Nov 2014 16:23:50 +0100 Received: from 109.201.152.230 ([109.201.152.230]) by main.gmane.org with esmtp (Gmexim 0.1 (Debian)) id 1AlnuQ-0007hv-00 for ; Tue, 25 Nov 2014 16:23:50 +0100 Received: from rasmus by 109.201.152.230 with local (Gmexim 0.1 (Debian)) id 1AlnuQ-0007hv-00 for ; Tue, 25 Nov 2014 16:23:50 +0100 List-Id: "General discussions about Org-mode." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: emacs-orgmode-bounces+geo-emacs-orgmode=m.gmane.org@gnu.org Sender: emacs-orgmode-bounces+geo-emacs-orgmode=m.gmane.org@gnu.org To: emacs-orgmode@gnu.org Kit-Yan Choi writes: >> 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? I recently worked with remote pictures for html-slides. The slides were stored on my github powered website, and the pictures were hosted on my owncloud. So I definitely see the merits of Kit-Yan's proposal. Also, remote files will work in HTML, but not in latex or odt (I think), so local caching could maybe also be applied, optionally, when exporting documents. org-startup-with-inline-images should have an equivalent file-variable that takes priority, and probably org-startup-with-inline-images should default to nil. The above is of course "IMO". —Rasmus -- Got mashed potatoes. Ain't got no T-Bone. No T-Bone From mboxrd@z Thu Jan 1 00:00:00 1970 From: Kit-Yan Choi Subject: Re: [Patch] org-display-inline-images: Add support for remote images Date: Tue, 25 Nov 2014 10:29:35 -0500 Message-ID: References: <87vbm3of2o.fsf@selenimh.mobile.lan> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary=001a11349f2e147d160508b094f3 Return-path: Received: from eggs.gnu.org ([2001:4830:134:3::10]:51159) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XtI3j-0000d0-4f for emacs-orgmode@gnu.org; Tue, 25 Nov 2014 10:29:45 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1XtI3b-0007Wn-Rg for emacs-orgmode@gnu.org; Tue, 25 Nov 2014 10:29:43 -0500 Received: from mail-qa0-x22b.google.com ([2607:f8b0:400d:c00::22b]:56618) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XtI3b-0007Wb-JP for emacs-orgmode@gnu.org; Tue, 25 Nov 2014 10:29:35 -0500 Received: by mail-qa0-f43.google.com with SMTP id bm13so532932qab.16 for ; Tue, 25 Nov 2014 07:29:35 -0800 (PST) In-Reply-To: List-Id: "General discussions about Org-mode." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: emacs-orgmode-bounces+geo-emacs-orgmode=m.gmane.org@gnu.org Sender: emacs-orgmode-bounces+geo-emacs-orgmode=m.gmane.org@gnu.org To: Kit-Yan Choi , emacs-orgmode@gnu.org --001a11349f2e147d160508b094f3 Content-Type: multipart/alternative; boundary=001a11349f2e147d100508b094f1 --001a11349f2e147d100508b094f1 Content-Type: text/plain; charset=UTF-8 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 >> > > --001a11349f2e147d100508b094f1 Content-Type: text/html; charset=UTF-8 Content-Transfer-Encoding: quoted-printable
Ah my apologies.=C2=A0 I forgot I had to use `file-name-di= rectory' for creating the path to the temporary directory (too long ago= since I did this).=C2=A0 Here is the corrected version.

--- a/lisp/org.el
+++ b/lisp/org.el
@@ -19340,7 += 19340,7 @@ boundaries."
=C2=A0 =C2=A0 =C2=A0(not (cdr (org-element-contents parent= )))))
=C2=A0 = =C2=A0 =C2=A0 =C2=A0(org-string-match-p file-extension-re
=C2=A0<= span class=3D"" style=3D"white-space:pre"> =C2=A0(org-element-p= roperty :path link)))
- =C2=A0 =C2=A0 (let ((file (expand-file-name (org-element-property= :path link))))
+ =C2=A0 =C2=A0 (let ((file (substitute-in-file-name (expand-file-name (o= rg-element-property :path link)))))
=C2=A0 =C2=A0 =C2=A0 =C2=A0 (when (file-exists-p fil= e)
=C2=A0 (le= t ((width
=C2=A0 ;; Apply `org-image-actual-width' specifications.
@@ -193= 78,10 +19378,25 @@ boundaries."
=C2=A0 =C2=A0 =C2=A0 'org-image-overlay)))
=C2=A0 =C2=A0 (i= f (and (car-safe old) refresh)
=C2=A0 =C2=A0 =C2=A0 =C2=A0 (image-refresh (overlay-get (= cdr old) 'display))
- =C2=A0 =C2=A0 (let ((image (create-image file
- =C2=A0(and width 'i= magemagick)
- =C2=A0nil
- = =C2=A0:width width)))
+ =C2=A0 =C2=A0 (let ((image=C2=A0
+ =C2=A0 =C2=A0(create-image (if (= org-file-remote-p file)
+ =C2=A0 =C2=A0 =C2=A0(let* ((tramp-tmpdir (concat
= + =C2=A0 =C2=A0(i= f (featurep 'xemacs)
+ (temp-directory)
+ =C2=A0 =C2=A0 =C2=A0temporary-file-director= y)
+ = =C2=A0 =C2=A0"/tramp"
+ =C2=A0 =C2=A0(file-name-directory (expand-file-n= ame file))))
+ =C2=A0 =C2=A0 (newname (concat
+ =C2=A0 =C2=A0 =C2=A0 tramp-tmpdir=C2=A0
+ =C2=A0 =C2= =A0 =C2=A0 (file-name-nondirectory (expand-file-name file)))))
+<= span class=3D"" style=3D"white-space:pre"> (make-directory tram= p-tmpdir t)
+ (if (file-newer-than-file-p file newname)
+ =C2=A0 =C2=A0(copy-file file newname= t t))
+ n= ewname)
+ = =C2=A0 =C2=A0file)
+ = =C2=A0(and width 'imagemagick)
+ =C2=A0nil
+ =C2=A0:width width)))
=C2= =A0 =C2=A0 =C2=A0 =C2= =A0 (when image
=C2=A0= (let* ((link
=C2=A0 ;; If inline image is the description
<= br>

On= Tue, Nov 25, 2014 at 10:15 AM, Kit-Yan Choi <kit@kychoi.org> = wrote:
Thank you for you= r comments!=C2=A0 They are very helpful.

> Thanks for your patch. However, I wonder if we really want this. Rem= ote
> images could be slow to fetch, and it would make buffer unusa= ble.

I personally needed this functionality.= =C2=A0 I have tried to reduce the amount of time spent on fetching the imag= es by checking whether the images have been fetched before and whether the = remote files are newer.=C2=A0 Yes these communications take time as it shou= ld be expected if one opens an org file remotely (therefore connection shou= ld have been made) or when one specifies a remote image as path and wants t= o display it inline.
Perhaps I could add an option flag or ask a = question before fetching for user to decide whether to display remote image= s or not?=C2=A0 In case the behaviour of displaying remote images inline is= not desired?=C2=A0 One scenario I can think of is that `org-startup-with-i= nline-images' is set to true and the file is sometimes visited remotely= . =C2=A0
Any opinion or comment on this, please?

>> -=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 (let= ((file (expand-file-name (org-element-property :path link))))
>> = +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 (let ((file (substitute-in-file-name (e= xpand-file-name (org-element-property :path link)))))
> Why is it nee= ded?

Because the file path for a remote fil= e, as far as I have tested, have redundant slashes "/" at the beg= inning of the path which makes org-file-remote-p to return nil for a remote= path.
`substitute-in-file-name' corrects such path. =C2=A0`s= ubstitute-in-file-name' is also used in `org-open-file'.=C2=A0 So I= followed suit.

> Are you sure= the return value (a boolean, i.e., not necessarily
> a string) sho= uld belong to the `concat'?

Good point.=C2=A0= I changed the code (see below, and attached).
<= br>
> =C2=A0(create-image (if (org-file-remote-p file) ...)> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0(and width &= #39;imagemagick)
> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0nil
> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0:w= idth width)

I agree.=C2=A0 Thanks.=C2=A0 I made t= he code cleaner now (see below, and attached).


@@ -19340,7 +19340,7 @@ boundaries."
=C2=A0 <= /span> =C2=A0 =C2=A0(not (cdr (org-element-contents parent)))))
=C2=A0 = =C2=A0 =C2=A0 =C2=A0(org-string-match-p file-extension-re
=C2=A0 = =C2=A0(org-element-property :path link)))
- =C2=A0 =C2= =A0 (let ((file (expand-file-name (org-element-property :path link))))
+ = =C2=A0 =C2=A0 (let ((file (substitute-in-file-name (expand-file-name (org-e= lement-property :path link)))))
=C2= =A0 =C2=A0 =C2=A0 =C2=A0 (when= (file-exists-p file)
=C2=A0 (let ((width
=C2=A0 ;; Apply `org-image-= actual-width' specifications.
@@ -19378= ,10 +19378,24 @@ boundaries."
=C2=A0 =C2=A0 =C2=A0 'org-image-= overlay)))
=C2=A0 =C2=A0 (if (and (car-safe old) refresh)
=C2=A0 =C2= =A0 =C2=A0 =C2=A0 (image-refresh (overlay-get (cdr old) 'display))
- = =C2=A0 =C2=A0 (let ((image (create-image file
- =C2=A0(and width &#= 39;imagemagick)
- =C2=A0nil
- =C2=A0:width width)))
+ =C2= =A0 =C2=A0 (let ((image=C2=A0
+ =C2=A0 =C2=A0(create-image (if (org-fi= le-remote-p file)
+ =C2=A0 =C2=A0 =C2=A0(let* ((tram= p-tmpdir (concat
+ =C2=A0 =C2=A0(if (featurep 'xemacs)
+ (temp-directory)
+ =C2=A0 =C2=A0 =C2=A0temporary-file-director= y)
+ =C2=A0 =C2=A0"/tramp"))
+ =C2=A0 =C2=A0 (newname (concat
+ =C2=A0 =C2=A0 =C2=A0 tramp-t= mpdir=C2=A0
+ =C2=A0 =C2=A0 =C2=A0 (expand-file-name file)))= )
+ = (make-directory tramp-tmpdir t)
+= (if (file-newer-than-file= -p file newname)
+ =C2=A0 =C2=A0(copy-file file newname t t))
<= div class=3D"gmail_extra">+ newname)
+ =C2=A0 =C2=A0file)
+ =C2=A0(and w= idth 'imagemagick)
+ =C2=A0nil
+<= span style=3D"white-space:pre-wrap"> =C2=A0:width width)))
=C2=A0 =C2=A0 =C2=A0 =C2=A0 (when image
=C2=A0 (let* ((link
=
=C2=A0 <= /span> ;; If inline image is the description



On Tue, Nov 25, 2014 at 3:32 A= M, Nicolas Goaziou <mail@nicolasgoaziou.fr> wrote:
<= blockquote class=3D"gmail_quote" style=3D"margin:0px 0px 0px 0.8ex;border-l= eft-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;pa= dding-left:1ex">Hello,

Kit-Yan Choi <kit@ky= choi.org> writes:

> I would like to submit a patch to support displaying remote images inl= ine
> in Org-mode.=C2=A0 Attached is the formatted patch (or github branch h= ere
> <https://github.com/k= itchoi/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.=C2=A0 Thanks.

Some comments follow.

> -=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 (let ((file (expand-file-name (org= -element-property :path link))))
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 (let ((file (substitute-in-file-na= me (expand-file-name (org-element-property :path link)))))

Why is it needed?

> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 (let* = ((image
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0 =C2=A0 =C2=A0 (let ((newname
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0(if (org-file-remote-p fil= e)
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0(let* ((tram= p-tmpdir (concat
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0(if (feat= urep 'xemacs)
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0(temp-directory)
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0te= mporary-file-directory)
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0"/tr= amp"
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0(org-file= -remote-p file)
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 ^^^^^= ^^^^^^^^^^^^^^^^^^^
Are you sure the return value (a boolean, i.e., not necessarily
a string) should belong to the `concat'?

> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0(file-nam= e-directory
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 (org-bab= el-local-file-name file))))
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 (newname (concat
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 tramp-tmpdir
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 (file-name-nondirectory file)= )))
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0(make= -directory tramp-tmpdir t)
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0(if (= tramp-handle-file-newer-than-file-p file newname)
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0(tramp-compat-copy-file file newname t t))
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0newna= me)
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0file)))
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0 =C2=A0 =C2=A0 =C2=A0 (create-image newname
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 (and width 'imagemagick)
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 nil
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 :width width))))

IMO, it is clearer to use

=C2=A0 (create-image (if (org-file-remote-p file) ...)
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 (and width 'ima= gemagick)
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 nil
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 :width width)



Regards,

--
Nicolas Goaziou


--001a11349f2e147d100508b094f1-- --001a11349f2e147d160508b094f3 Content-Type: text/plain; charset=US-ASCII; name="0001-org.el.diff" Content-Disposition: attachment; filename="0001-org.el.diff" Content-Transfer-Encoding: base64 X-Attachment-Id: f_i2xf5hy01 ZGlmZiAtLWdpdCBhL2xpc3Avb3JnLmVsIGIvbGlzcC9vcmcuZWwKaW5kZXggNmFiMTNmNC4uNDRj YTdkNiAxMDA3NTUKLS0tIGEvbGlzcC9vcmcuZWwKKysrIGIvbGlzcC9vcmcuZWwKQEAgLTE5MzQw LDcgKzE5MzQwLDcgQEAgYm91bmRhcmllcy4iCiAJCQkgICAgKG5vdCAoY2RyIChvcmctZWxlbWVu dC1jb250ZW50cyBwYXJlbnQpKSkpKQogCQkgICAgICAob3JnLXN0cmluZy1tYXRjaC1wIGZpbGUt ZXh0ZW5zaW9uLXJlCiAJCQkJCSAgKG9yZy1lbGVtZW50LXByb3BlcnR5IDpwYXRoIGxpbmspKSkK LQkgICAgIChsZXQgKChmaWxlIChleHBhbmQtZmlsZS1uYW1lIChvcmctZWxlbWVudC1wcm9wZXJ0 eSA6cGF0aCBsaW5rKSkpKQorCSAgICAgKGxldCAoKGZpbGUgKHN1YnN0aXR1dGUtaW4tZmlsZS1u YW1lIChleHBhbmQtZmlsZS1uYW1lIChvcmctZWxlbWVudC1wcm9wZXJ0eSA6cGF0aCBsaW5rKSkp KSkKIAkgICAgICAgKHdoZW4gKGZpbGUtZXhpc3RzLXAgZmlsZSkKIAkJIChsZXQgKCh3aWR0aAog CQkJOzsgQXBwbHkgYG9yZy1pbWFnZS1hY3R1YWwtd2lkdGgnIHNwZWNpZmljYXRpb25zLgpAQCAt MTkzNzgsMTAgKzE5Mzc4LDI1IEBAIGJvdW5kYXJpZXMuIgogCQkJICAgICAnb3JnLWltYWdlLW92 ZXJsYXkpKSkKIAkJICAgKGlmIChhbmQgKGNhci1zYWZlIG9sZCkgcmVmcmVzaCkKIAkJICAgICAg IChpbWFnZS1yZWZyZXNoIChvdmVybGF5LWdldCAoY2RyIG9sZCkgJ2Rpc3BsYXkpKQotCQkgICAg IChsZXQgKChpbWFnZSAoY3JlYXRlLWltYWdlIGZpbGUKLQkJCQkJCSAgKGFuZCB3aWR0aCAnaW1h Z2VtYWdpY2spCi0JCQkJCQkgIG5pbAotCQkJCQkJICA6d2lkdGggd2lkdGgpKSkKKwkJICAgICAo bGV0ICgoaW1hZ2UgCisJCQkgICAgKGNyZWF0ZS1pbWFnZSAoaWYgKG9yZy1maWxlLXJlbW90ZS1w IGZpbGUpCisJCQkJCSAgICAgIChsZXQqICgodHJhbXAtdG1wZGlyIChjb25jYXQKKwkJCQkJCQkJ ICAgIChpZiAoZmVhdHVyZXAgJ3hlbWFjcykKKwkJCQkJCQkJCSh0ZW1wLWRpcmVjdG9yeSkKKwkJ CQkJCQkJICAgICAgdGVtcG9yYXJ5LWZpbGUtZGlyZWN0b3J5KQorCQkJCQkJCQkgICAgIi90cmFt cCIKKwkJCQkJCQkJICAgIChmaWxlLW5hbWUtZGlyZWN0b3J5IChleHBhbmQtZmlsZS1uYW1lIGZp bGUpKSkpCisJCQkJCQkgICAgIChuZXduYW1lIChjb25jYXQKKwkJCQkJCQkgICAgICAgdHJhbXAt dG1wZGlyIAorCQkJCQkJCSAgICAgICAoZmlsZS1uYW1lLW5vbmRpcmVjdG9yeSAoZXhwYW5kLWZp bGUtbmFtZSBmaWxlKSkpKSkKKwkJCQkJCShtYWtlLWRpcmVjdG9yeSB0cmFtcC10bXBkaXIgdCkK KwkJCQkJCShpZiAoZmlsZS1uZXdlci10aGFuLWZpbGUtcCBmaWxlIG5ld25hbWUpCisJCQkJCQkg ICAgKGNvcHktZmlsZSBmaWxlIG5ld25hbWUgdCB0KSkKKwkJCQkJCW5ld25hbWUpCisJCQkJCSAg ICBmaWxlKQorCQkJCQkgIChhbmQgd2lkdGggJ2ltYWdlbWFnaWNrKQorCQkJCQkgIG5pbAorCQkJ CQkgIDp3aWR0aCB3aWR0aCkpKQogCQkgICAgICAgKHdoZW4gaW1hZ2UKIAkJCSAobGV0KiAoKGxp bmsKIAkJCQkgOzsgSWYgaW5saW5lIGltYWdlIGlzIHRoZSBkZXNjcmlwdGlvbgo= --001a11349f2e147d160508b094f3-- From mboxrd@z Thu Jan 1 00:00:00 1970 From: Michael Albinus Subject: Re: [Patch] org-display-inline-images: Add support for remote images Date: Tue, 25 Nov 2014 16:39:02 +0100 Message-ID: <87bnnv1e95.fsf@gmx.de> References: <87vbm3of2o.fsf@selenimh.mobile.lan> Mime-Version: 1.0 Content-Type: text/plain Return-path: Received: from eggs.gnu.org ([2001:4830:134:3::10]:52917) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XtICv-0002Ws-ND for emacs-orgmode@gnu.org; Tue, 25 Nov 2014 10:39:20 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1XtICp-00041Z-Dd for emacs-orgmode@gnu.org; Tue, 25 Nov 2014 10:39:13 -0500 Received: from mout.gmx.net ([212.227.15.19]:50213) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XtICp-00041K-2x for emacs-orgmode@gnu.org; Tue, 25 Nov 2014 10:39:07 -0500 In-Reply-To: (Kit-Yan Choi's message of "Tue, 25 Nov 2014 10:29:35 -0500") List-Id: "General discussions about Org-mode." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: emacs-orgmode-bounces+geo-emacs-orgmode=m.gmane.org@gnu.org Sender: emacs-orgmode-bounces+geo-emacs-orgmode=m.gmane.org@gnu.org To: Kit-Yan Choi Cc: emacs-orgmode@gnu.org Kit-Yan Choi writes: > 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 This code looks much to complicate to me. Wouldn't a simple file-local-copy suffice? You don't need to care whether the file is remote (let Tramp do the job) or local (there won't be a copy). Best regards, Michael. From mboxrd@z Thu Jan 1 00:00:00 1970 From: Kit-Yan Choi Subject: Re: [Patch] org-display-inline-images: Add support for remote images Date: Tue, 25 Nov 2014 10:45:50 -0500 Message-ID: References: <87vbm3of2o.fsf@selenimh.mobile.lan> <87bnnv1e95.fsf@gmx.de> Mime-Version: 1.0 Content-Type: multipart/alternative; boundary=001a11c13e4a3b3e150508b0ce1c Return-path: Received: from eggs.gnu.org ([2001:4830:134:3::10]:54293) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XtIJN-0005ri-Hh for emacs-orgmode@gnu.org; Tue, 25 Nov 2014 10:46:03 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1XtIJL-0006c4-Ia for emacs-orgmode@gnu.org; Tue, 25 Nov 2014 10:45:53 -0500 Received: from mail-qc0-x230.google.com ([2607:f8b0:400d:c01::230]:50135) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XtIJL-0006c0-Ag for emacs-orgmode@gnu.org; Tue, 25 Nov 2014 10:45:51 -0500 Received: by mail-qc0-f176.google.com with SMTP id i17so608954qcy.35 for ; Tue, 25 Nov 2014 07:45:50 -0800 (PST) In-Reply-To: <87bnnv1e95.fsf@gmx.de> List-Id: "General discussions about Org-mode." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: emacs-orgmode-bounces+geo-emacs-orgmode=m.gmane.org@gnu.org Sender: emacs-orgmode-bounces+geo-emacs-orgmode=m.gmane.org@gnu.org To: Michael Albinus Cc: emacs-orgmode@gnu.org --001a11c13e4a3b3e150508b0ce1c Content-Type: text/plain; charset=UTF-8 Michael, Thanks for the suggestion. Indeed `file-local-copy' would have made the code cleaner. Yet `file-local-copy' generates a new filename each time it's run. But I wanted to allow the code to check whether the remote image has already been fetched before and so skip unnecessary file transfer, which takes time (think of it like "rsync -a"). Therefore I wanted to preserve the remote path under a temporary directory. Best, Kit On Tue, Nov 25, 2014 at 10:39 AM, Michael Albinus wrote: > Kit-Yan Choi writes: > > > 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 > > This code looks much to complicate to me. Wouldn't a simple > file-local-copy suffice? You don't need to care whether the file is > remote (let Tramp do the job) or local (there won't be a copy). > > Best regards, Michael. > --001a11c13e4a3b3e150508b0ce1c Content-Type: text/html; charset=UTF-8 Content-Transfer-Encoding: quoted-printable
Michael,

Thanks for the suggestion.=C2= =A0 Indeed `file-local-copy' would have made the code cleaner.=C2=A0 Ye= t `file-local-copy' generates a new filename each time it's run.=C2= =A0 But I wanted to allow the code to check whether the remote image has al= ready been fetched before and so skip unnecessary file transfer, which take= s time (think of it like "rsync -a").=C2=A0 Therefore I wanted to= preserve the remote path under a temporary directory.

=
Best,
Kit


On Tue, Nov 25, 2014 at 10:39 AM, Michael= Albinus <michael.albinus@gmx.de> wrote:
Kit-Yan Choi &l= t;kit@kychoi.org> writes:

> 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

This code looks much to complicate to me. Wouldn't a simple=
file-local-copy suffice? You don't need to care whether the file is
remote (let Tramp do the job) or local (there won't be a copy).

Best regards, Michael.

--001a11c13e4a3b3e150508b0ce1c-- From mboxrd@z Thu Jan 1 00:00:00 1970 From: Michael Albinus Subject: Re: [Patch] org-display-inline-images: Add support for remote images Date: Tue, 25 Nov 2014 18:04:49 +0100 Message-ID: <8761e31aa6.fsf@gmx.de> References: <87vbm3of2o.fsf@selenimh.mobile.lan> <87bnnv1e95.fsf@gmx.de> Mime-Version: 1.0 Content-Type: text/plain Return-path: Received: from eggs.gnu.org ([2001:4830:134:3::10]:48410) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XtJXv-0004YS-Vc for emacs-orgmode@gnu.org; Tue, 25 Nov 2014 12:05:06 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1XtJXp-0000Ve-Oh for emacs-orgmode@gnu.org; Tue, 25 Nov 2014 12:04:59 -0500 Received: from mout.gmx.net ([212.227.15.19]:63824) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XtJXp-0000VY-E7 for emacs-orgmode@gnu.org; Tue, 25 Nov 2014 12:04:53 -0500 In-Reply-To: (Kit-Yan Choi's message of "Tue, 25 Nov 2014 10:45:50 -0500") List-Id: "General discussions about Org-mode." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: emacs-orgmode-bounces+geo-emacs-orgmode=m.gmane.org@gnu.org Sender: emacs-orgmode-bounces+geo-emacs-orgmode=m.gmane.org@gnu.org To: Kit-Yan Choi Cc: emacs-orgmode@gnu.org Kit-Yan Choi writes: > Michael, Hi Kit, > Thanks for the suggestion. Indeed `file-local-copy' would have made > the code cleaner. Yet `file-local-copy' generates a new filename each > time it's run. But I wanted to allow the code to check whether the > remote image has already been fetched before and so skip unnecessary > file transfer, which takes time (think of it like "rsync -a"). > Therefore I wanted to preserve the remote path under a temporary > directory. After first retrieval via file-local-copy, you could cache this information somewhere for later use. Or you could write a ticket towards Tramp, that file-local-copy shall remember that a file was downloaded already, and should offer that file for reuse. Note: I'm the Tramp maintainer :-) > Best, > Kit Best regards, Michael. From mboxrd@z Thu Jan 1 00:00:00 1970 From: Kit-Yan Choi Subject: Re: [Patch] org-display-inline-images: Add support for remote images Date: Wed, 26 Nov 2014 12:13:15 -0500 Message-ID: References: <87vbm3of2o.fsf@selenimh.mobile.lan> <87bnnv1e95.fsf@gmx.de> <8761e31aa6.fsf@gmx.de> Mime-Version: 1.0 Content-Type: multipart/alternative; boundary=001a113a4eb2a8395d0508c6243c Return-path: Received: from eggs.gnu.org ([2001:4830:134:3::10]:38005) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Xtg9V-0006xi-4s for emacs-orgmode@gnu.org; Wed, 26 Nov 2014 12:13:18 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Xtg9T-0003A3-SE for emacs-orgmode@gnu.org; Wed, 26 Nov 2014 12:13:17 -0500 Received: from mail-qg0-x22e.google.com ([2607:f8b0:400d:c04::22e]:35862) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Xtg9T-00039w-Mr for emacs-orgmode@gnu.org; Wed, 26 Nov 2014 12:13:15 -0500 Received: by mail-qg0-f46.google.com with SMTP id z107so2314284qgd.19 for ; Wed, 26 Nov 2014 09:13:15 -0800 (PST) In-Reply-To: <8761e31aa6.fsf@gmx.de> List-Id: "General discussions about Org-mode." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: emacs-orgmode-bounces+geo-emacs-orgmode=m.gmane.org@gnu.org Sender: emacs-orgmode-bounces+geo-emacs-orgmode=m.gmane.org@gnu.org To: Michael Albinus Cc: emacs-orgmode@gnu.org --001a113a4eb2a8395d0508c6243c Content-Type: text/plain; charset=UTF-8 Thank you! These are interesting ideas! For both of these suggestions, I think I will need a look-up table using the remote file paths as keys, the local copy paths as values. But is it theoretically possible to have two remote file paths sharing the same local copy file name? i.e. `make-temp-file' generates the same temporary file name for two separate calls? If such a situation occurs, then: (1) the local copy of one remote image will be overwritten by another (2) it is wrong to try to update the local copy based on the time stamp of a wrong remote file. As far as org-mode is concerned, and assuming the current stable version of tramp, I think there are two options (1) the current approach which preserves the path of the remote file and therefore avoid unnecessary downloads; however we forgo the capability of compression that tramp's file-local-copy offers, (2) use `file-local-copy' but risk overwriting (albeit unlikely) downloaded local copy; this is possible with or with out a look-up table for reusing files. And yes, for those who prefer seeing an empty box instead of downloading remote images for display, I will add an option for turning this functionality ON/OFF, default off maybe. Thanks, Kit On Tue, Nov 25, 2014 at 12:04 PM, Michael Albinus wrote: > Kit-Yan Choi writes: > > > Michael, > > Hi Kit, > > > Thanks for the suggestion. Indeed `file-local-copy' would have made > > the code cleaner. Yet `file-local-copy' generates a new filename each > > time it's run. But I wanted to allow the code to check whether the > > remote image has already been fetched before and so skip unnecessary > > file transfer, which takes time (think of it like "rsync -a"). > > Therefore I wanted to preserve the remote path under a temporary > > directory. > > After first retrieval via file-local-copy, you could cache this > information somewhere for later use. > > Or you could write a ticket towards Tramp, that file-local-copy shall > remember that a file was downloaded already, and should offer that file > for reuse. > > Note: I'm the Tramp maintainer :-) > > > Best, > > Kit > > Best regards, Michael. > --001a113a4eb2a8395d0508c6243c Content-Type: text/html; charset=UTF-8 Content-Transfer-Encoding: quoted-printable
Thank you!=C2=A0 These are interesting ideas!=C2=A0 For bo= th of these suggestions, I think I will need a look-up table using the remo= te file paths as keys, the local copy paths as values. =C2=A0

But is it theoretically possible to have two remote file paths sharin= g the same local copy file name? =C2=A0i.e. `make-temp-file' generates = the same temporary file name for two separate calls?
If such a si= tuation occurs, then: (1) the local copy of one remote image will be overwr= itten by another (2) it is wrong to try to update the local copy based on t= he time stamp of a wrong remote file.

As far as or= g-mode is concerned, and assuming the current stable version of tramp, I th= ink there are two options (1) the current approach which preserves the path= of the remote file and therefore avoid unnecessary downloads; however we f= orgo the capability of compression that tramp's file-local-copy offers,= (2) use `file-local-copy' but risk overwriting (albeit unlikely) downl= oaded local copy; this is possible with or with out a look-up table for reu= sing files.

And yes, for those who prefer seeing a= n empty box instead of downloading remote images for display, I will add an= option for turning this functionality ON/OFF, default off maybe.

Thanks,
Kit


On Tue, Nov 25, 2014 at 12:= 04 PM, Michael Albinus <michael.albinus@gmx.de> wrote:<= br>
Kit-Yan Choi <kit@kychoi.org> writes:

> Michael,

Hi Kit,

> Thanks for the suggestion. Indeed `file-local-copy' would have mad= e
> the code cleaner. Yet `file-local-copy' generates a new filename e= ach
> time it's run. But I wanted to allow the code to check whether the=
> remote image has already been fetched before and so skip unnecessary > file transfer, which takes time (think of it like "rsync -a"= ).
> Therefore I wanted to preserve the remote path under a temporary
> directory.

After first retrieval via file-local-copy, you could cache this
information somewhere for later use.

Or you could write a ticket towards Tramp, that file-local-copy shall
remember that a file was downloaded already, and should offer that file
for reuse.

Note: I'm the Tramp maintainer :-)

> Best,
> Kit

Best regards, Michael.

--001a113a4eb2a8395d0508c6243c-- From mboxrd@z Thu Jan 1 00:00:00 1970 From: Michael Albinus Subject: Re: [Patch] org-display-inline-images: Add support for remote images Date: Sat, 29 Nov 2014 11:50:50 +0100 Message-ID: <8738922sc5.fsf@gmx.de> References: <87vbm3of2o.fsf@selenimh.mobile.lan> <87bnnv1e95.fsf@gmx.de> <8761e31aa6.fsf@gmx.de> Mime-Version: 1.0 Content-Type: text/plain Return-path: Received: from eggs.gnu.org ([2001:4830:134:3::10]:57843) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XufcG-0000UY-NY for emacs-orgmode@gnu.org; Sat, 29 Nov 2014 05:51:10 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Xufc7-0006oZ-Mv for emacs-orgmode@gnu.org; Sat, 29 Nov 2014 05:51:04 -0500 Received: from mout.gmx.net ([212.227.15.19]:49906) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Xufc7-0006oI-Ch for emacs-orgmode@gnu.org; Sat, 29 Nov 2014 05:50:55 -0500 In-Reply-To: (Kit-Yan Choi's message of "Wed, 26 Nov 2014 12:13:15 -0500") List-Id: "General discussions about Org-mode." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: emacs-orgmode-bounces+geo-emacs-orgmode=m.gmane.org@gnu.org Sender: emacs-orgmode-bounces+geo-emacs-orgmode=m.gmane.org@gnu.org To: Kit-Yan Choi Cc: emacs-orgmode@gnu.org Kit-Yan Choi writes: > But is it theoretically possible to have two remote file paths sharing > the same local copy file name? i.e. `make-temp-file' generates the > same temporary file name for two separate calls? No, never ever. After choosing a random file name, make-temp-file checks whether there exists already such a file. In case of yes, it chosses another random file name, and so on. > As far as org-mode is concerned, and assuming the current stable > version of tramp, I think there are two options (1) the current > approach which preserves the path of the remote file and therefore > avoid unnecessary downloads; however we forgo the capability of > compression that tramp's file-local-copy offers, (2) use > `file-local-copy' but risk overwriting (albeit unlikely) downloaded > local copy; this is possible with or with out a look-up table for > reusing files. Again, the risk of overwriting something else does not exist. > Thanks, > Kit Best regards, Michael.