From mboxrd@z Thu Jan  1 00:00:00 1970
From: Nicolas Goaziou <mail@nicolasgoaziou.fr>
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: <CAKHbGXV-iY9ucXohWrMBBWHF5dfJrgp8237Vk6JXO743Q-LFOQ@mail.gmail.com>
Mime-Version: 1.0
Content-Type: text/plain
Return-path: <emacs-orgmode-bounces+geo-emacs-orgmode=m.gmane.org@gnu.org>
Received: from eggs.gnu.org ([2001:4830:134:3::10]:55726)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <mail@nicolasgoaziou.fr>) 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 <mail@nicolasgoaziou.fr>) 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 <mail@nicolasgoaziou.fr>) id 1XtBXt-0002Gv-Bo
	for emacs-orgmode@gnu.org; Tue, 25 Nov 2014 03:32:25 -0500
In-Reply-To: <CAKHbGXV-iY9ucXohWrMBBWHF5dfJrgp8237Vk6JXO743Q-LFOQ@mail.gmail.com>
	(Kit-Yan Choi's message of "Fri, 21 Nov 2014 23:43:17 -0500")
List-Id: "General discussions about Org-mode." <emacs-orgmode.gnu.org>
List-Unsubscribe: <https://lists.gnu.org/mailman/options/emacs-orgmode>,
	<mailto:emacs-orgmode-request@gnu.org?subject=unsubscribe>
List-Archive: <http://lists.gnu.org/archive/html/emacs-orgmode>
List-Post: <mailto:emacs-orgmode@gnu.org>
List-Help: <mailto:emacs-orgmode-request@gnu.org?subject=help>
List-Subscribe: <https://lists.gnu.org/mailman/listinfo/emacs-orgmode>,
	<mailto:emacs-orgmode-request@gnu.org?subject=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 <kit@kychoi.org>
Cc: emacs-orgmode@gnu.org

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