From: Nicolas Goaziou <mail@nicolasgoaziou.fr>
To: Kit-Yan Choi <kit@kychoi.org>
Cc: emacs-orgmode@gnu.org
Subject: Re: [Patch] org-display-inline-images: Add support for remote images
Date: Tue, 25 Nov 2014 09:32:47 +0100 [thread overview]
Message-ID: <87vbm3of2o.fsf@selenimh.mobile.lan> (raw)
In-Reply-To: <CAKHbGXV-iY9ucXohWrMBBWHF5dfJrgp8237Vk6JXO743Q-LFOQ@mail.gmail.com> (Kit-Yan Choi's message of "Fri, 21 Nov 2014 23:43:17 -0500")
Hello,
Kit-Yan Choi <kit@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
next prev parent reply other threads:[~2014-11-25 8:32 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-11-22 4:43 [Patch] org-display-inline-images: Add support for remote images Kit-Yan Choi
2014-11-25 8:32 ` Nicolas Goaziou [this message]
2014-11-25 15:15 ` Kit-Yan Choi
2014-11-25 15:23 ` Rasmus
2014-11-25 15:29 ` Kit-Yan Choi
2014-11-25 15:39 ` Michael Albinus
2014-11-25 15:45 ` Kit-Yan Choi
2014-11-25 17:04 ` Michael Albinus
2014-11-26 17:13 ` Kit-Yan Choi
2014-11-29 10:50 ` Michael Albinus
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
List information: https://www.orgmode.org/
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=87vbm3of2o.fsf@selenimh.mobile.lan \
--to=mail@nicolasgoaziou.fr \
--cc=emacs-orgmode@gnu.org \
--cc=kit@kychoi.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this public inbox
https://git.savannah.gnu.org/cgit/emacs/org-mode.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).