Hello, Jack Kamm <jackk...@gmail.com> 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 `org--create-inline-image'. > + (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'")) Ditto. Regards, -- Nicolas Goaziou