[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: Displaying remote images

From: Nicolas Goaziou
Subject: Re: Displaying remote images
Date: Tue, 21 Jan 2020 17:39:45 +0100
User-agent: Gnus/5.13 (Gnus v5.13) Emacs/26.3 (gnu/linux)


Jack Kamm <address@hidden> writes:

> Apologies for the delay on this. I've now got a more complete patch for
> displaying remote images inline. Since downloading many remote images
> could potentially hang Emacs on a slow connection, I've added an option
> to control whether remote images are displayed. I've also added an
> option to cache the remote images by visiting them in Emacs buffers.

Thank you.

> The default behavior is not to display remote images, but to issue a
> message that references the option that controls remote image display.

I think displaying a message in this case can be annoying. It means the
default value is not satisfying for anyone.

> +(defcustom org-display-remote-inline-images 'skip-warn
> +  "How to display remote inline images.
> +Possible values of this option are:
> +
> +skip-warn         Don't display, and emit a message about it.
> +skip-silent       Don't display, and don't warn about it.
> +download-always   Always download and display remote images.
> +cache-in-buffer   Display remote images, and open them in separate buffers 
> for
> +                  cache'ing.  Silently update the image buffer when a file
> +                  change is detected."
> +  :type '(choice
> +       (const skip-warn)
> +       (const skip-silent)
> +       (const download-always)
> +       (const cache-in-buffers))
> +  :group 'org-appearance)

I suggest to drop the `skip-warn' value altogether, and use
`skip-silent', renamed as `skip', as the default value.

It also needs a :package-version and a :safe keyword.

> +(defun org-inline-image--buffer-unibyte ()
> +  (string-make-unibyte (buffer-substring-no-properties
> +                     (point-min) (point-max))))

I'm surprised such a function is necessary. In any case, it should be
named `org--inline-image-buffer-unibyte'.

> +(defun org-inline-image--create (file width)

It should be named `org--inline-image-create' or

> +  (let* ((remote-p (file-remote-p file))
> +      (file-or-data
> +       (if remote-p

I suggest to move the `if' within the `pcase' with an initial `guard'.

> +           (pcase org-display-remote-inline-images
> +             (`download-always (with-temp-buffer (insert-file-contents file)
> +                                                 
> (org-inline-image--buffer-unibyte)))

Wouldn't `insert-file-contents-literally' fit the bill instead?

> +             (`cache-in-buffers (let ((revert-without-query '(".*")))
> +                                  (with-current-buffer
> +                                      (find-file-noselect file)
> +                                    (org-inline-image--buffer-unibyte))))

Wouldn't the RAW argument from `find-file-noselect' prevent
`org-inline-image--buffer-unibyte' from being used?

> +             (`skip-warn (message
> +                          (concat "Set `org-display-remote-inline-images'"
> +                                  " to display remote images."))

Use continuation delimiter instead.

> +                         nil)
> +             (`skip-silent nil)
> +             (_ (message (concat "Invalid value of "
> +                                 "`org-display-remote-inline-images'"))



Nicolas Goaziou

reply via email to

[Prev in Thread] Current Thread [Next in Thread]