bug#52200: 29.0.50; image-dired-cmd-create-standard-thumbnail-options

bug#52200: 29.0.50; image-dired-cmd-create-standard-thumbnail-options

[Peter Münster]

[-- Attachment #1.1: Type: text/plain, Size: 438 bytes --]

Hi,

I'm a bit confused about
image-dired-cmd-create-standard-thumbnail-options:

- It seems, that it's not used with pngcrush or pngnq, so this test is
  perhaps useless:
  (unless (or image-dired-cmd-pngcrush-program
              image-dired-cmd-pngnq-program) ...
- If gm is used, then the "convert" command should be added.

The attached patch works for me, but probably I'm missing something...

-- 
           Peter

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1.2: my.patch --]
[-- Type: text/x-patch, Size: 1447 bytes --]

diff --git a/lisp/image-dired.el b/lisp/image-dired.el
index 47a44a4a60..159f6a3b97 100644
--- a/lisp/image-dired.el
+++ b/lisp/image-dired.el
@@ -302,19 +302,17 @@ image-dired-cmd-optipng-options
   :link '(url-link "man:optipng(1)"))
 
 (defcustom image-dired-cmd-create-standard-thumbnail-options
-  (append '("-size" "%wx%h" "%f[0]")
-          (unless (or image-dired-cmd-pngcrush-program
-                      image-dired-cmd-pngnq-program)
-            (list
-             "-set" "Thumb::MTime" "%m"
-             "-set" "Thumb::URI" "file://%f"
-             "-set" "Description" "Thumbnail of file://%f"
-             "-set" "Software" (emacs-version)))
-          '("-thumbnail" "%wx%h>" "png:%t"))
+  (let ((opts (list "-size" "%wx%h" "%f[0]"
+              "-set" "Thumb::MTime" "%m"
+              "-set" "Thumb::URI" "file://%f"
+              "-set" "Description" "Thumbnail of file://%f"
+              "-set" "Software" (emacs-version)
+              "-thumbnail" "%wx%h>" "png:%t")))
+      (if (executable-find "gm") (cons "convert" opts) opts))
   "Options for creating thumbnails according to the Thumbnail Managing Standard.
 Available format specifiers are the same as in
 `image-dired-cmd-create-thumbnail-options', with %m for file modification time."
-  :version "26.1"
+  :version "29.1"
   :type '(repeat (string :tag "Argument")))
 
 (defcustom image-dired-cmd-rotate-original-program

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 211 bytes --]

bug#52200: 29.0.50; image-dired-cmd-create-standard-thumbnail-options

[Stefan Kangas]

Peter Münster <pm@a16n.net> writes:

> I'm a bit confused about
> image-dired-cmd-create-standard-thumbnail-options:
>
> - It seems, that it's not used with pngcrush or pngnq, so this test is
>   perhaps useless:
>   (unless (or image-dired-cmd-pngcrush-program
>               image-dired-cmd-pngnq-program) ...

I think the intention is that we shouldn't insert those text fields
twice.  See the removed code in commit 36b9973dec65 and also
`image-dired-cmd-pngcrush-options'.

Perhaps we could make this unconditional, but then what should we do
about `image-dired-cmd-pngcrush-options'?  Should the options be removed
from there?

> - If gm is used, then the "convert" command should be added.

Yes, I think you are right.

bug#52200: 29.0.50; image-dired-cmd-create-standard-thumbnail-options

[Peter Münster]

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

On Wed, Dec 08 2021, Stefan Kangas wrote:

> Perhaps we could make this unconditional, but then what should we do
> about `image-dired-cmd-pngcrush-options'?  Should the options be removed
> from there?

I don't know...


>> - If gm is used, then the "convert" command should be added.
>
> Yes, I think you are right.

Ok, could you add this part of the patch then please?

TIA,
-- 
           Peter

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 211 bytes --]

bug#52200: 29.0.50; image-dired-cmd-create-standard-thumbnail-options

[Peter Münster]

And here the same patch for the latest master:

diff --git a/lisp/image/image-dired-external.el b/lisp/image/image-dired-external.el
index 223d881bcf..fcf2e2edb6 100644
--- a/lisp/image/image-dired-external.el
+++ b/lisp/image/image-dired-external.el
@@ -128,15 +128,13 @@ image-dired-cmd-optipng-options
   :link '(url-link "man:optipng(1)"))
 
 (defcustom image-dired-cmd-create-standard-thumbnail-options
-  (append '("-size" "%wx%h" "%f[0]")
-          (unless (or image-dired-cmd-pngcrush-program
-                      image-dired-cmd-pngnq-program)
-            (list
-             "-set" "Thumb::MTime" "%m"
-             "-set" "Thumb::URI" "file://%f"
-             "-set" "Description" "Thumbnail of file://%f"
-             "-set" "Software" (emacs-version)))
-          '("-thumbnail" "%wx%h>" "png:%t"))
+  (let ((opts (list "-size" "%wx%h" "%f[0]"
+                    "-set" "Thumb::MTime" "%m"
+                    "-set" "Thumb::URI" "file://%f"
+                    "-set" "Description" "Thumbnail of file://%f"
+                    "-set" "Software" (emacs-version)
+                    "-thumbnail" "%wx%h>" "png:%t")))
+    (if (executable-find "gm") (cons "convert" opts) opts))
   "Options for creating thumbnails according to the Thumbnail Managing Standard.
 Available format specifiers are the same as in
 `image-dired-cmd-create-thumbnail-options', with %m for file modification time."

-- 
           Peter

bug#52200: 29.0.50; image-dired-cmd-create-standard-thumbnail-options

[Stefan Kangas]

close 52200 29.1
thanks

Peter Münster <pm@a16n.net> writes:

> And here the same patch for the latest master:

Sorry for the delay here.  Your patch LGTM and works for me, so I've
pushed it to master (commit 7ec31d3222).  Thanks.