emacs-orgmode@gnu.org archives
 help / color / mirror / code / Atom feed
* [PATCH] Persistently save downloaded inline remote images
@ 2020-09-29 17:15 Ferdinand Pieper
  2020-12-07  5:54 ` Kyle Meyer
  0 siblings, 1 reply; 7+ messages in thread
From: Ferdinand Pieper @ 2020-09-29 17:15 UTC (permalink / raw)
  To: emacs-orgmode

[-- Attachment #1: Type: text/plain, Size: 272 bytes --]

Currently remote images are downloaded upon each display.
As most of the time the images do not change in between redisplays, we can instead buffer the images locally and only update the local copy when the remote image is updated.

Attached is a proposed patch.


Best,


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-org.el-Persistently-save-downloaded-inline-remote-im.patch --]
[-- Type: text/x-diff, Size: 1663 bytes --]

From aa34ad1176f4599c5a3c2678806644f16a3d22a2 Mon Sep 17 00:00:00 2001
From: fpi <git@pie.tf>
Date: Tue, 23 Jun 2020 15:59:28 +0200
Subject: [PATCH] org.el: Persistently save downloaded inline remote images

* lisp/org.el (org--create-inline-image): Save downloaded inline
remote images to temporary directory to persist them for future
`org-display-inline-images' calls.
---
 lisp/org.el | 20 ++++++++++++++++----
 1 file changed, 16 insertions(+), 4 deletions(-)

diff --git a/lisp/org.el b/lisp/org.el
index 4d46b4173..7b649d6d0 100644
--- a/lisp/org.el
+++ b/lisp/org.el
@@ -16277,10 +16277,22 @@ according to the value of `org-display-remote-inline-images'."
 	 (file-or-data
 	  (pcase org-display-remote-inline-images
 	    ((guard (not remote?)) file)
-	    (`download (with-temp-buffer
-			 (set-buffer-multibyte nil)
-			 (insert-file-contents-literally file)
-			 (buffer-string)))
+	    (`download (let ((new (concat temporary-file-directory
+					  "tramp/"
+					  (file-remote-p file 'host)
+					  (file-local-name file))))
+			 ;; dont download file if local copy exists & is newer than remote
+			 (if (and (file-exists-p new)
+				  (file-newer-than-file-p new file))
+			     (with-temp-buffer
+			       (set-buffer-multibyte nil)
+			       (insert-file-contents-literally new)
+			       (buffer-string))
+			   (with-temp-file new
+			     (make-directory (file-name-directory new) t)
+			     (set-buffer-multibyte nil)
+			     (insert-file-contents-literally file)
+			     (buffer-string)))))
 	    (`cache (let ((revert-without-query '(".")))
 		      (with-current-buffer (find-file-noselect file)
 			(buffer-string))))
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH] Persistently save downloaded inline remote images
  2020-09-29 17:15 [PATCH] Persistently save downloaded inline remote images Ferdinand Pieper
@ 2020-12-07  5:54 ` Kyle Meyer
  2020-12-07 15:17   ` Jack Kamm
  0 siblings, 1 reply; 7+ messages in thread
From: Kyle Meyer @ 2020-12-07  5:54 UTC (permalink / raw)
  To: Ferdinand Pieper; +Cc: Jack Kamm, emacs-orgmode


Thanks for the patch, and sorry about the slow response.

Ferdinand Pieper writes:

> Currently remote images are downloaded upon each display.  As most of
> the time the images do not change in between redisplays, we can
> instead buffer the images locally and only update the local copy when
> the remote image is updated.

Hmm, I'm guessing this was left as a simple download given that there is
also a `cache' option that keeps the image around in a separate buffer.
Does that value help for your use case?

Jack, as the author of the org-display-remote-inline-images commit, what
do you think about this patch?


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] Persistently save downloaded inline remote images
  2020-12-07  5:54 ` Kyle Meyer
@ 2020-12-07 15:17   ` Jack Kamm
  2021-04-27 20:39     ` Bastien
  2021-04-27 20:40     ` Bastien
  0 siblings, 2 replies; 7+ messages in thread
From: Jack Kamm @ 2020-12-07 15:17 UTC (permalink / raw)
  To: Kyle Meyer, Ferdinand Pieper; +Cc: michael.albinus, emacs-orgmode, kit

> Jack, as the author of the org-display-remote-inline-images commit, what
> do you think about this patch?

This patch takes the same approach as an earlier attempt by Kit in 2014
[1].  That patch was never merged; Michael objected that the approach
seemed too complex, and suggested using file-local-copy and cacheing the
filename instead; he also suggested implementing this cacheing
functionality in Tramp instead of Org.

So, when I submitted a new attempt at this, I chose to cache by simply
opening the image files in Emacs instead, since the implementation is
much simpler, and would hopefully sidestep the objections from the 2014
approach.

While cacheing via tempfiles is more complex, it does have some
benefits: it allows cacheing across Emacs sessions, and it avoids the
side effect of opening the image files in Emacs.

I'm agnostic whether tempfiles or Emacs buffers are ultimately
better. My inclination is to be conservative and stick with the status
quo unless there's a compelling reason to switch.

So, I suggest Ferdinand confirm he's tested the existing "cache" option,
and articulate why he found it unsuitable, before merging this.

If we do proceed in this direction, I'd suggest replacing the current
cache mechanism entirely -- I don't see a good reason to maintain 2
solutions to the same problem. Also, we could combine the "download" and
"cache" options into a single implementation, making them synonyms for
backwards compatibility.

[1] https://lists.gnu.org/archive/html/emacs-orgmode/2014-11/msg00690.html
[2] https://lists.gnu.org/archive/html/emacs-orgmode/2020-01/msg00177.html


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] Persistently save downloaded inline remote images
  2020-12-07 15:17   ` Jack Kamm
@ 2021-04-27 20:39     ` Bastien
  2021-04-27 20:40     ` Bastien
  1 sibling, 0 replies; 7+ messages in thread
From: Bastien @ 2021-04-27 20:39 UTC (permalink / raw)
  To: Jack Kamm
  Cc: emacs-orgmode, Kyle Meyer, michael.albinus, Ferdinand Pieper, kit

Hi,

Jack Kamm <jackkamm@gmail.com> writes:

> So, I suggest Ferdinand confirm he's tested the existing "cache" option,
> and articulate why he found it unsuitable, before merging this.

As no feedback was received, I'm discarding this patch.




^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] Persistently save downloaded inline remote images
  2020-12-07 15:17   ` Jack Kamm
  2021-04-27 20:39     ` Bastien
@ 2021-04-27 20:40     ` Bastien
  2022-06-20 21:39       ` Jack Kamm
  1 sibling, 1 reply; 7+ messages in thread
From: Bastien @ 2021-04-27 20:40 UTC (permalink / raw)
  To: Jack Kamm
  Cc: emacs-orgmode, Kyle Meyer, michael.albinus, Ferdinand Pieper, kit

Hi Jack,

Jack Kamm <jackkamm@gmail.com> writes:

> If we do proceed in this direction, I'd suggest replacing the current
> cache mechanism entirely -- I don't see a good reason to maintain 2
> solutions to the same problem. Also, we could combine the "download" and
> "cache" options into a single implementation, making them synonyms for
> backwards compatibility.

I just added a call for help about this.  Thanks!


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] Persistently save downloaded inline remote images
  2021-04-27 20:40     ` Bastien
@ 2022-06-20 21:39       ` Jack Kamm
  2022-06-21  3:39         ` Ihor Radchenko
  0 siblings, 1 reply; 7+ messages in thread
From: Jack Kamm @ 2022-06-20 21:39 UTC (permalink / raw)
  To: Bastien; +Cc: Kyle Meyer, Ferdinand Pieper, michael.albinus, emacs-orgmode, kit

Canceled.

> I just added a call for help about this.  Thanks!

This help request is old, and also it was never clear what concrete
enhancement was desirable, or what specific problem there was with the
existing cacheing mechanism. So I think it just adds noise to
updates.orgmode.org.

Therefore, I am canceling the help request (or rather, attempting to
cancel it with Woof -- this is my first time using Woof like this, so
apologies in advance if it doesn't work).


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] Persistently save downloaded inline remote images
  2022-06-20 21:39       ` Jack Kamm
@ 2022-06-21  3:39         ` Ihor Radchenko
  0 siblings, 0 replies; 7+ messages in thread
From: Ihor Radchenko @ 2022-06-21  3:39 UTC (permalink / raw)
  To: Jack Kamm
  Cc: Bastien, Kyle Meyer, Ferdinand Pieper, michael.albinus,
	emacs-orgmode, kit

Jack Kamm <jackkamm@gmail.com> writes:

> Therefore, I am canceling the help request (or rather, attempting to
> cancel it with Woof -- this is my first time using Woof like this, so
> apologies in advance if it doesn't work).

Thanks for taking care of this!

> This help request is old, and also it was never clear what concrete
> enhancement was desirable, or what specific problem there was with the
> existing cacheing mechanism. So I think it just adds noise to
> updates.orgmode.org.

As an update from the time this patch has been submitted, we now have
org-persist library that allow caching downloaded files, as one of the
possibilities. Such caching is already being used during export. See
6ee45518f.

Best,
Ihor



^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2022-06-21  3:39 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-09-29 17:15 [PATCH] Persistently save downloaded inline remote images Ferdinand Pieper
2020-12-07  5:54 ` Kyle Meyer
2020-12-07 15:17   ` Jack Kamm
2021-04-27 20:39     ` Bastien
2021-04-27 20:40     ` Bastien
2022-06-20 21:39       ` Jack Kamm
2022-06-21  3:39         ` Ihor Radchenko

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).