Package: emacs;
Reported by: David Ponce <da_vid <at> orange.fr>
Date: Thu, 30 Apr 2020 08:54:01 UTC
Severity: normal
Merged with 47039
Found in versions 27.1, 28.0.50
Fixed in version 29.1
Done: Lars Ingebrigtsen <larsi <at> gnus.org>
Bug is archived. No further changes may be made.
Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):
From: David Ponce <da_vid <at> orange.fr> To: bug-gnu-emacs <at> gnu.org Subject: 28.0.50; create-image and find-image consistency Date: Thu, 30 Apr 2020 10:53:26 +0200
Hello dear Emacs developers,
I am using Emacs latest master, and noticed some inconsistency between the
results of function `create-image' and `find-image'.
Here is an example:
(create-image "[EMACS-DIR]/etc/images/icons/hicolor/scalable/apps/emacs.svg")
==> (image :type svg :file "[EMACS-DIR]/etc/images/icons/hicolor/scalable/apps/emacs.svg" :scale 1.2)
(find-image '((:type svg :file "[EMACS-DIR]/etc/images/icons/hicolor/scalable/apps/emacs.svg")))
==> (image :type svg :file "[EMACS-DIR]/etc/images/icons/hicolor/scalable/apps/emacs.svg")
Contrary to images returned by `create-image', which are automatically scaled
when no :scale property is provided, those returned by `find-image' are not.
I propose the below patch to have `find-image' use `create-image' in order to get
consistent results from both functions. Another advantage of using `create-image' is
that it is no longer necessary to provide the :type property to `find-image'.
The correct type is determined by `create-image' with the proper default scaling.
Here is the result of previous example with patched `find-image':
(find-image '((:file "[EMACS-DIR]/etc/images/icons/hicolor/scalable/apps/emacs.svg")))
==> (image :type svg :file "[EMACS-DIR]/etc/images/icons/hicolor/scalable/apps/emacs.svg" :scale 1.2)
Please eventually consider this enhancement to Emacs, if it makes sense.
Thanks & regards,
David Ponce
diff --git a/installs/emacs/lisp/image.el b/emacs.d/image.el
index 4ea8594..046af95 100644
--- a/installs/emacs/lisp/image.el
+++ b/emacs.d/image.el
@@ -687,13 +687,15 @@ SPECS is a list of image specifications.
Each image specification in SPECS is a property list. The contents of
a specification are image type dependent. All specifications must at
-least contain the properties `:type TYPE' and either `:file FILE' or
-`:data DATA', where TYPE is a symbol specifying the image type,
-e.g. `xbm', FILE is the file to load the image from, and DATA is a
-string containing the actual image data. The specification whose TYPE
-is supported, and FILE exists, is used to construct the image
-specification to be returned. Return nil if no specification is
-satisfied.
+least contain the either the property `:file FILE' or `:data DATA',
+where FILE is the file to load the image from, and DATA is a string
+containing the actual image data. If the property `:type TYPE' is
+omitted or nil, try to determine the image type from its first few
+bytes of image data. If that doesn’t work, and the property `:file
+FILE' provide a file name, use its file extension as image type. If
+the property `:type TYPE' is provided, it must match the actual type
+determined for FILE or DATA by `create-image'. Return nil if no
+specification is satisfied.
The image is looked for in `image-load-path'.
@@ -703,16 +705,39 @@ Image files should not be larger than specified by `max-image-size'."
(let* ((spec (car specs))
(type (plist-get spec :type))
(data (plist-get spec :data))
- (file (plist-get spec :file))
- found)
- (when (image-type-available-p type)
- (cond ((stringp file)
- (if (setq found (image-search-load-path file))
- (setq image
- (cons 'image (plist-put (copy-sequence spec)
- :file found)))))
- ((not (null data))
- (setq image (cons 'image spec)))))
+ (file (plist-get spec :file)))
+ (cond
+ ((stringp file)
+ (when (setq file (image-search-load-path file))
+ ;; At this point, remove the :type and :file properties.
+ ;; `create-image' will set them depending on image file.
+ (setq image (cons 'image (copy-sequence spec)))
+ (setf (image-property image :type) nil)
+ (setf (image-property image :file) nil)
+ (and (setq image (ignore-errors
+ (apply #'create-image file nil nil
+ (cdr image))))
+ ;; Ensure, if a type has been provided, it is
+ ;; consistent with the type returned by
+ ;; `create-image'. If not, return nil.
+ (not (null type))
+ (not (eq type (image-property image :type)))
+ (setq image nil))))
+ ((not (null data))
+ ;; At this point, remove the :type and :data properties.
+ ;; `create-image' will set them depending on image data.
+ (setq image (cons 'image (copy-sequence spec)))
+ (setf (image-property image :type) nil)
+ (setf (image-property image :data) nil)
+ (and (setq image (ignore-errors
+ (apply #'create-image data nil t
+ (cdr image))))
+ ;; Ensure, if a type has been provided, it is
+ ;; consistent with the type returned by
+ ;; `create-image'. If not, return nil.
+ (not (null type))
+ (not (eq type (image-property image :type)))
+ (setq image nil))))
(setq specs (cdr specs))))
image))
-------- Forwarded Message --------
From: Juri Linkov <juri <at> linkov.net>
To: David Ponce <da_vid <at> orange.fr>
Subject: Re: create-image and find-image consistency
Date: Mon, 20 Apr 2020 02:24:14 +0300
> I propose the below patch to have `find-image' use `create-image' in order to get
> consistent results from both functions. Another advantage of using `create-image' is
> that it is no longer necessary to provide the :type property to `find-image'.
> The correct type is determined by `create-image' with the proper default scaling.
>
> Here is the result of previous example with patched `find-image':
>
> (find-image '((:file "[EMACS-DIR]/etc/images/icons/hicolor/scalable/apps/emacs.svg")))
> ==> (image :type svg :file "[EMACS-DIR]/etc/images/icons/hicolor/scalable/apps/emacs.svg" :scale 1.2)
>
> Please eventually consider this enhancement to Emacs, if it makes sense.
I haven't tested it yet, but it makes sense indeed.
-------- Forwarded Message --------
From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: David Ponce <da_vid <at> orange.fr>
Cc: emacs-devel <at> gnu.org
Subject: Re: create-image and find-image consistency
Date: Thu, 30 Apr 2020 07:08:54 +0200
David Ponce <da_vid <at> orange.fr> writes:
> I am using Emacs latest master, and noticed some inconsistency between the
> results of function `create-image' and `find-image'.
>
> Here is an example:
>
> (create-image "[EMACS-DIR]/etc/images/icons/hicolor/scalable/apps/emacs.svg")
> ==> (image :type svg :file "[EMACS-DIR]/etc/images/icons/hicolor/scalable/apps/emacs.svg" :scale 1.2)
>
> (find-image '((:type svg :file "[EMACS-DIR]/etc/images/icons/hicolor/scalable/apps/emacs.svg")))
> ==> (image :type svg :file "[EMACS-DIR]/etc/images/icons/hicolor/scalable/apps/emacs.svg")
>
> Contrary to images returned by `create-image', which are automatically scaled
> when no :scale property is provided, those returned by `find-image' are not.
>
> I propose the below patch to have `find-image' use `create-image' in
> order to get consistent results from both functions.
I think it conceptually makes sense to have find-image use create-image,
but looking at the code, and where find-image is used, it looks like the
use case it for creating toolbars and the like, where you want to pick
out one of the (built-in) image formats that Emacs supports...
Looking at your patch, you remove the (image-type-available-p type), and
instead rely on create-image not bugging out instead? That feels like a
less obvious way to do the test (and more breakable; there may be other
reasons create-image fails).
And I'm not 100% sure that we want to auto-scale toolbars and the like.
I'm pretty sure we do, but perhaps somebody else has an opinion here?
Anyway, I think you should file this as a bug report so that the patch
doesn't get lost, because I think you're basically correct that the
find-image behaviour should be changed.
--
(domestic pets only, the antidote for overdose, milk.)
bloggy blog: http://lars.ingebrigtsen.no
GNU bug tracking system
Copyright (C) 1999 Darren O. Benham,
1997,2003 nCipher Corporation Ltd,
1994-97 Ian Jackson.