bug#61394: 30.0.50; [PATCH] Image-dired thumb name based on content

bug#61394: 30.0.50; [PATCH] Image-dired thumb name based on content

[Manuel Giraud via Bug reports for GNU Emacs, the Swiss army knife of text editors]

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


Hi,

Here is a proposal for a modification of how image-dired generates thumb
names.  With this patch, a thumb name will depend on the content of the
associated image.

Pros:
        - the thumb name is unique even if the file is moved or renamed
        - you keep a centralized thumb directory
Cons:
        - none (as it fits my usage :-)


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Image-dired-thumb-name-based-on-content.patch --]
[-- Type: text/x-patch, Size: 2255 bytes --]

From 017bb36bdff40383b170e8ee5cff00034595a6ce Mon Sep 17 00:00:00 2001
From: Manuel Giraud <manuel@ledu-giraud.fr>
Date: Thu, 9 Feb 2023 19:56:37 +0100
Subject: [PATCH] Image-dired thumb name based on content

* lisp/image/image-dired-util.el (image-dired-content-sha1): New
utility that compute the SHA-1 of a part of the content of a file.
(image-dired-thumb-name): Use it.
---
 lisp/image/image-dired-util.el | 14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/lisp/image/image-dired-util.el b/lisp/image/image-dired-util.el
index c03f9d2e3d3..58c96f7f20b 100644
--- a/lisp/image/image-dired-util.el
+++ b/lisp/image/image-dired-util.el
@@ -57,13 +57,19 @@ image-dired-dir
       (message "Thumbnail directory created: %s" image-dired-dir))
     image-dired-dir))
 
+(defun image-dired-content-sha1 (filename)
+  "Compute the SHA-1 of a part of FILENAME."
+  (with-temp-buffer
+    (let ((file-size (file-attribute-size (file-attributes filename)))
+	  (chunk-size 4096))
+      (insert-file-contents filename nil 0 (min chunk-size file-size))
+      (sha1 (current-buffer)))))
+
 (defun image-dired-thumb-name (file)
   "Return absolute file name for thumbnail FILE.
 Depending on the value of `image-dired-thumbnail-storage', the
 file name of the thumbnail will vary:
-- For `use-image-dired-dir', make a SHA1-hash of the image file's
-  directory name and add that to make the thumbnail file name
-  unique.
+- For `image-dired', make a SHA1-hash of some of the image file.
 - For `per-directory' storage, just add a subdirectory.
 - For `standard' storage, produce the file name according to the
   Thumbnail Managing Standard.  Among other things, an MD5-hash
@@ -85,7 +91,7 @@ image-dired-thumb-name
           ((or (eq 'image-dired image-dired-thumbnail-storage)
                ;; Maintained for backwards compatibility:
                (eq 'use-image-dired-dir image-dired-thumbnail-storage))
-           (expand-file-name (format "%s.jpg" (sha1 file))
+           (expand-file-name (format "%s.jpg" (image-dired-content-sha1 file))
                              (image-dired-dir)))
           ((eq 'per-directory image-dired-thumbnail-storage)
            (expand-file-name (format "%s.thumb.jpg"
-- 
2.39.0


[-- Attachment #3: Type: text/plain, Size: 8072 bytes --]



In GNU Emacs 30.0.50 (build 1, x86_64-unknown-openbsd7.2, cairo version
 1.17.6) of 2023-02-09 built on computer
Repository revision: 1518fc5d7c5bedbbe35053696c7ec06020c81b05
Repository branch: master
Windowing system distributor 'The X.Org Foundation', version 11.0.12101004
System Description: OpenBSD computer 7.2 GENERIC.MP#933 amd64

Configured using:
 'configure --prefix=/home/manuel/emacs --bindir=/home/manuel/bin
 --with-x-toolkit=no --without-sound --without-compress-install
 CPPFLAGS=-I/usr/local/include LDFLAGS=-L/usr/local/lib'

Configured features:
CAIRO DBUS FREETYPE GIF GLIB GMP GNUTLS GSETTINGS HARFBUZZ JPEG JSON
LCMS2 LIBOTF LIBXML2 MODULES NOTIFY KQUEUE OLDXMENU PDUMPER PNG RSVG
SQLITE3 THREADS TIFF TREE_SITTER WEBP X11 XDBE XIM XINPUT2 XPM ZLIB

Important settings:
  value of $LC_ALL: en_US.UTF-8
  locale-coding-system: utf-8-unix

Major mode: Dired by name

Minor modes in effect:
  global-git-commit-mode: t
  magit-auto-revert-mode: t
  gnus-dired-mode: t
  display-time-mode: t
  display-battery-mode: t
  server-mode: t
  shell-dirtrack-mode: t
  repeat-mode: t
  desktop-save-mode: t
  global-eldoc-mode: t
  show-paren-mode: t
  electric-indent-mode: t
  mouse-wheel-mode: t
  menu-bar-mode: t
  file-name-shadow-mode: t
  global-font-lock-mode: t
  font-lock-mode: t
  blink-cursor-mode: t
  buffer-read-only: t
  line-number-mode: t
  indent-tabs-mode: t
  transient-mark-mode: t
  auto-composition-mode: t
  auto-encryption-mode: t
  auto-compression-mode: t

Load-path shadows:
/home/manuel/.emacs.d/elpa/ef-themes-0.10.0/theme-loaddefs hides /home/manuel/emacs/share/emacs/30.0.50/lisp/theme-loaddefs
/home/manuel/.emacs.d/elpa/transient-0.3.7/transient hides /home/manuel/emacs/share/emacs/30.0.50/lisp/transient

Features:
(shadow sort mail-extr macros emacsbug whitespace magit-patch wdired
magit-extras face-remap magit-bookmark magit-submodule magit-obsolete
magit-blame magit-stash magit-reflog magit-bisect magit-push magit-pull
magit-fetch magit-clone magit-remote magit-commit magit-sequence
magit-notes magit-worktree magit-tag magit-merge magit-branch
magit-reset magit-files magit-refs magit-status magit magit-repos
magit-apply magit-wip magit-log which-func magit-diff smerge-mode diff
git-commit log-edit pcvs-util magit-core magit-autorevert magit-margin
magit-transient magit-process with-editor magit-mode transient magit-git
magit-section benchmark shortdoc executable dabbrev find-dired ffap
tabify cus-start image-dired-dired pulse misearch multi-isearch cl-print
help-fns radix-tree magit-utils dash image-file image-converter
image-dired image-dired-tags image-dired-external image-dired-util
org-indent org-element org-persist org-id org-refile avl-tree oc-basic
ol-eww ol-rmail ol-mhe ol-irc ol-info ol-gnus nnselect ol-docview
ol-bibtex bibtex ol-bbdb ol-w3m ol-doi org-link-doi texinfo
texinfo-loaddefs reveal rng-xsd xsd-regexp rng-cmpct rng-nxml rng-valid
rng-loc rng-uri rng-parse nxml-parse rng-match rng-dt rng-util rng-pttrn
nxml-ns nxml-mode nxml-outln nxml-rap nxml-util nxml-enc xmltok css-mode
treesit smie sgml-mode facemenu imenu eww xdg url-queue mm-url doc-view
jka-compr image-mode exif view pascal bug-reference paredit edmacro
vc-hg vc-svn conf-mode vc-dir ewoc vc autorevert filenotify vc-git
diff-mode vc-dispatcher add-log gnus-dired time battery exwm-randr
xcb-randr exwm-config exwm exwm-input xcb-keysyms xcb-xkb exwm-manage
exwm-floating xcb-cursor xcb-render exwm-layout exwm-workspace exwm-core
xcb-ewmh xcb-icccm xcb xcb-xproto xcb-types xcb-debug kmacro server
stimmung-themes modus-operandi-theme modus-themes ytdious osm mingus
libmpdee reporter edebug debug backtrace transmission diary-lib
diary-loaddefs color calc-bin calc-ext calc calc-loaddefs rect calc-macs
w3m-load mu4e mu4e-org mu4e-main mu4e-view mu4e-headers mu4e-compose
mu4e-draft mu4e-actions smtpmail mu4e-search mu4e-lists mu4e-bookmarks
mu4e-mark mu4e-message flow-fill mule-util hl-line mu4e-contacts
mu4e-update mu4e-folders mu4e-server mu4e-context mu4e-vars mu4e-helpers
mu4e-config bookmark ido supercite regi ebdb-message ebdb-gnus gnus-msg
gnus-art mm-uu mml2015 mm-view mml-smime smime gnutls dig gnus-sum shr
pixel-fill kinsoku url-file svg dom gnus-group gnus-undo gnus-start
gnus-dbus gnus-cloud nnimap nnmail mail-source utf7 nnoo gnus-spec
gnus-int gnus-range message sendmail yank-media puny rfc822 mml mml-sec
epa epg rfc6068 epg-config mm-decode mm-bodies mm-encode mail-parse
rfc2231 rfc2047 rfc2045 ietf-drums gmm-utils mailheader gnus-win gnus
nnheader gnus-util mail-utils range mm-util mail-prsvr ebdb-mua ebdb-com
crm ebdb-format ebdb mailabbrev eieio-opt cl-extra help-mode speedbar
ezimage dframe eieio-base pcase timezone org ob ob-tangle ob-ref ob-lob
ob-table ob-exp org-macro org-src ob-comint org-pcomplete org-list
org-footnote org-faces org-entities ob-emacs-lisp ob-core ob-eval
org-cycle org-table ol org-fold org-fold-core org-keys oc org-loaddefs
find-func cal-menu calendar cal-loaddefs org-version org-compat org-macs
visual-basic-mode cl web-mode derived disp-table erlang-start
smart-tabs-mode skeleton cc-mode cc-fonts cc-guess cc-menus cc-cmds
cc-styles cc-align cc-engine cc-vars cc-defs slime-asdf grep slime-tramp
tramp rx tramp-loaddefs trampver tramp-integration cus-edit cus-load
wid-edit files-x tramp-compat shell pcomplete parse-time iso8601
time-date ls-lisp format-spec slime-fancy slime-indentation
slime-cl-indent cl-indent slime-trace-dialog slime-fontifying-fu
slime-package-fu slime-references slime-compiler-notes-tree advice
slime-scratch slime-presentations bridge slime-macrostep macrostep
slime-mdot-fu slime-enclosing-context slime-fuzzy slime-fancy-trace
slime-fancy-inspector slime-c-p-c slime-editing-commands slime-autodoc
slime-repl slime-parse slime apropos compile text-property-search etags
fileloop generator xref project arc-mode archive-mode noutline outline
icons pp comint ansi-osc ansi-color ring hyperspec thingatpt
slime-autoloads dired-aux dired-x dired dired-loaddefs notifications
dbus xml repeat easy-mmode desktop frameset stimmung-themes-autoloads
rust-mode-autoloads ebdb-autoloads magit-autoloads debbugs-autoloads
git-commit-autoloads magit-section-autoloads ef-themes-autoloads
with-editor-autoloads paredit-autoloads dash-autoloads ytdious-autoloads
transmission-autoloads transient-autoloads exwm-autoloads
hyperbole-autoloads detached-autoloads info package browse-url url
url-proxy url-privacy url-expand url-methods url-history url-cookie
generate-lisp-file url-domsuf url-util mailcap url-handlers url-parse
auth-source cl-seq eieio eieio-core cl-macs password-cache json subr-x
map byte-opt gv bytecomp byte-compile url-vars cl-loaddefs cl-lib rmc
iso-transl tooltip cconv eldoc paren electric uniquify ediff-hook
vc-hooks lisp-float-type elisp-mode mwheel term/x-win x-win
term/common-win x-dnd tool-bar dnd fontset image regexp-opt fringe
tabulated-list replace newcomment text-mode lisp-mode prog-mode register
page tab-bar menu-bar rfn-eshadow isearch easymenu timer select
scroll-bar mouse jit-lock font-lock syntax font-core term/tty-colors
frame minibuffer nadvice seq simple cl-generic indonesian philippine
cham georgian utf-8-lang misc-lang vietnamese tibetan thai tai-viet lao
korean japanese eucjp-ms cp51932 hebrew greek romanian slovak czech
european ethiopic indian cyrillic chinese composite emoji-zwj charscript
charprop case-table epa-hook jka-cmpr-hook help abbrev obarray oclosure
cl-preloaded button loaddefs theme-loaddefs faces cus-face macroexp
files window text-properties overlay sha1 md5 base64 format env
code-pages mule custom widget keymap hashtable-print-readable backquote
threads dbusbind kqueue lcms2 dynamic-setting system-font-setting
font-render-setting cairo xinput2 x multi-tty make-network-process
emacs)

Memory information:
((conses 16 890208 177320)
 (symbols 48 60888 5)
 (strings 32 208408 17024)
 (string-bytes 1 6407691)
 (vectors 16 117900)
 (vector-slots 8 2299033 117223)
 (floats 8 645 483)
 (intervals 56 40488 2510)
 (buffers 984 144))

-- 
Manuel Giraud

bug#61394: 30.0.50; [PATCH] Image-dired thumb name based on content

[Basil Contovounesios via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Manuel Giraud via "Bug reports for GNU Emacs, the Swiss army knife of text editors" [2023-02-09 20:06 +0100] wrote:

> +(defun image-dired-content-sha1 (filename)
> +  "Compute the SHA-1 of a part of FILENAME."
> +  (with-temp-buffer
> +    (let ((file-size (file-attribute-size (file-attributes filename)))
> +	  (chunk-size 4096))
> +      (insert-file-contents filename nil 0 (min chunk-size file-size))

Can't we unconditionally pass END=chunk-size to insert-file-contents,
even for smaller files?

Thanks,

-- 
Basil

bug#61394: 30.0.50; [PATCH] Image-dired thumb name based on content

[Manuel Giraud via Bug reports for GNU Emacs, the Swiss army knife of text editors]

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

Basil Contovounesios <contovob@tcd.ie> writes:

> Manuel Giraud via "Bug reports for GNU Emacs, the Swiss army knife of
> text editors" [2023-02-09 20:06 +0100] wrote:
>
>> +(defun image-dired-content-sha1 (filename)
>> +  "Compute the SHA-1 of a part of FILENAME."
>> +  (with-temp-buffer
>> +    (let ((file-size (file-attribute-size (file-attributes filename)))
>> +	  (chunk-size 4096))
>> +      (insert-file-contents filename nil 0 (min chunk-size file-size))
>
> Can't we unconditionally pass END=chunk-size to insert-file-contents,
> even for smaller files?

From fileio.c:4076, it seems that you are right:

--8<---------------cut here---------------start------------->8---
      /* The likely offset where we will stop reading.  We could read
	 more (or less), if the file grows (or shrinks) as we read it.  */
      off_t likely_end = min (end_offset, st.st_size);
--8<---------------cut here---------------end--------------->8---

So here is an update version of this patch.  I've tested it on small 400
bytes icons and it works also.

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Image-dired-thumb-name-based-on-content.patch --]
[-- Type: text/x-patch, Size: 2130 bytes --]

From f568717f7492c57fbf248ac4dd8d16cd22a24443 Mon Sep 17 00:00:00 2001
From: Manuel Giraud <manuel@ledu-giraud.fr>
Date: Thu, 9 Feb 2023 19:56:37 +0100
Subject: [PATCH] Image-dired thumb name based on content

* lisp/image/image-dired-util.el (image-dired-content-sha1): New
utility that compute the SHA-1 of a part of the content of a file.
(image-dired-thumb-name): Use it.
---
 lisp/image/image-dired-util.el | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/lisp/image/image-dired-util.el b/lisp/image/image-dired-util.el
index c03f9d2e3d3..e54879ce033 100644
--- a/lisp/image/image-dired-util.el
+++ b/lisp/image/image-dired-util.el
@@ -57,13 +57,17 @@ image-dired-dir
       (message "Thumbnail directory created: %s" image-dired-dir))
     image-dired-dir))
 
+(defun image-dired-content-sha1 (filename)
+  "Compute the SHA-1 of a part of FILENAME."
+  (with-temp-buffer
+    (insert-file-contents filename nil 0 4096)
+    (sha1 (current-buffer))))
+
 (defun image-dired-thumb-name (file)
   "Return absolute file name for thumbnail FILE.
 Depending on the value of `image-dired-thumbnail-storage', the
 file name of the thumbnail will vary:
-- For `use-image-dired-dir', make a SHA1-hash of the image file's
-  directory name and add that to make the thumbnail file name
-  unique.
+- For `image-dired', make a SHA1-hash of some of the image file.
 - For `per-directory' storage, just add a subdirectory.
 - For `standard' storage, produce the file name according to the
   Thumbnail Managing Standard.  Among other things, an MD5-hash
@@ -85,7 +89,7 @@ image-dired-thumb-name
           ((or (eq 'image-dired image-dired-thumbnail-storage)
                ;; Maintained for backwards compatibility:
                (eq 'use-image-dired-dir image-dired-thumbnail-storage))
-           (expand-file-name (format "%s.jpg" (sha1 file))
+           (expand-file-name (format "%s.jpg" (image-dired-content-sha1 file))
                              (image-dired-dir)))
           ((eq 'per-directory image-dired-thumbnail-storage)
            (expand-file-name (format "%s.thumb.jpg"
-- 
2.39.1


[-- Attachment #3: Type: text/plain, Size: 27 bytes --]


Thanks,
-- 
Manuel Giraud

bug#61394: 30.0.50; [PATCH] Image-dired thumb name based on content

[Eli Zaretskii]

> Cc: 61394@debbugs.gnu.org
> Date: Fri, 10 Feb 2023 19:46:02 +0100
> From:  Manuel Giraud via "Bug reports for GNU Emacs,
>  the Swiss army knife of text editors" <bug-gnu-emacs@gnu.org>
> 
> +(defun image-dired-content-sha1 (filename)
> +  "Compute the SHA-1 of a part of FILENAME."

Not "part of FILENAME", but "the first 4KiB of FILENAME's contents".

Btw, using only the first 4KiB would mean a collision is still
possible, albeit rarely, right?  So your use case of having all the
thumbnails in the same directory could sometimes fail, right?

> +  (with-temp-buffer
> +    (insert-file-contents filename nil 0 4096)

Please use insert-file-contents-literally here.  It should be much
faster, and we only care about the file's bytestream anyway.

>  (defun image-dired-thumb-name (file)
>    "Return absolute file name for thumbnail FILE.
>  Depending on the value of `image-dired-thumbnail-storage', the
>  file name of the thumbnail will vary:
> -- For `use-image-dired-dir', make a SHA1-hash of the image file's
> -  directory name and add that to make the thumbnail file name
> -  unique.
> +- For `image-dired', make a SHA1-hash of some of the image file.
>  - For `per-directory' storage, just add a subdirectory.
>  - For `standard' storage, produce the file name according to the
>    Thumbnail Managing Standard.  Among other things, an MD5-hash

This doc string "needs work".  Could you please fix it as part of the
patch, even though most of the problems are not due to this patch?  In
any case, please either say here that only the first 4KiB of the file's
contents are SHA1-hashed or include a link to the new function.

bug#61394: 30.0.50; [PATCH] Image-dired thumb name based on content

[Manuel Giraud via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Eli Zaretskii <eliz@gnu.org> writes:

>> Cc: 61394@debbugs.gnu.org
>> Date: Fri, 10 Feb 2023 19:46:02 +0100
>> From:  Manuel Giraud via "Bug reports for GNU Emacs,
>>  the Swiss army knife of text editors" <bug-gnu-emacs@gnu.org>
>> 
>> +(defun image-dired-content-sha1 (filename)
>> +  "Compute the SHA-1 of a part of FILENAME."
>
> Not "part of FILENAME", but "the first 4KiB of FILENAME's contents".

Yes, I'll fix that.

> Btw, using only the first 4KiB would mean a collision is still
> possible, albeit rarely, right?  So your use case of having all the
> thumbnails in the same directory could sometimes fail, right?

The 4KiB was "quite large but not so much" guess.  I've made tests with
the following code:

--8<---------------cut here---------------start------------->8---
(defun sha1-test (filename size)
  (with-temp-buffer
    (insert-file-contents-literally filename nil 0 size)
    (sha1 (current-buffer))))

;; From 1KiB to 64KiB
(list
 (benchmark-run-compiled 1000
   (sha1-test "/tmp/a-5MiB-photo.jpg" (expt 2 10)))
 (benchmark-run-compiled 1000
   (sha1-test "/tmp/a-5MiB-photo.jpg" (expt 2 11)))
 (benchmark-run-compiled 1000
   (sha1-test "/tmp/a-5MiB-photo.jpg" (expt 2 12)))
 (benchmark-run-compiled 1000
   (sha1-test "/tmp/a-5MiB-photo.jpg" (expt 2 13)))
 (benchmark-run-compiled 1000
   (sha1-test "/tmp/a-5MiB-photo.jpg" (expt 2 14)))
 (benchmark-run-compiled 1000
   (sha1-test "/tmp/a-5MiB-photo.jpg" (expt 2 15)))
 (benchmark-run-compiled 1000
   (sha1-test "/tmp/a-5MiB-photo.jpg" (expt 2 16))))
--8<---------------cut here---------------end--------------->8---

And here are the results on my machine:
((0.664336771 1 0.14466495299998883)
 (0.707937024 2 0.28811983400001395)
 (0.940229304 3 0.44037704100000497) ;; <- 4KiB
 (1.672118528 4 0.7672738199999856)
 (2.6194289370000003 6 1.046699996000001)
 (3.169999951 11 1.5916382949999957)
 (6.547043287 21 3.195145416999992))

So this 4KiB seems practical: about 1 second for one thousand run.
WDYT?

About collision, my wild guess here is that, as we are considering
images, most of the modifications on these images we'll have an impact
on those first 4KiB anyway.  But you're that collision is still possible
and the thumb could be wrong.  I'll try to find out what is the
probability of a SHA-1 collision on 4KiB of data.

>> +  (with-temp-buffer
>> +    (insert-file-contents filename nil 0 4096)
>
> Please use insert-file-contents-literally here.  It should be much
> faster, and we only care about the file's bytestream anyway.

Thanks, I'll do that too.

>>  (defun image-dired-thumb-name (file)
>>    "Return absolute file name for thumbnail FILE.
>>  Depending on the value of `image-dired-thumbnail-storage', the
>>  file name of the thumbnail will vary:
>> -- For `use-image-dired-dir', make a SHA1-hash of the image file's
>> -  directory name and add that to make the thumbnail file name
>> -  unique.
>> +- For `image-dired', make a SHA1-hash of some of the image file.
>>  - For `per-directory' storage, just add a subdirectory.
>>  - For `standard' storage, produce the file name according to the
>>    Thumbnail Managing Standard.  Among other things, an MD5-hash
>
> This doc string "needs work".  Could you please fix it as part of the
> patch, even though most of the problems are not due to this patch?  In
> any case, please either say here that only the first 4KiB of the file's
> contents are SHA1-hashed or include a link to the new function.

You're right it is even not complete for `per-directory'.  I could try
to come up with a fix.  Thanks.
-- 
Manuel Giraud

bug#61394: 30.0.50; [PATCH] Image-dired thumb name based on content

[Eli Zaretskii]

> From: Manuel Giraud <manuel@ledu-giraud.fr>
> Cc: contovob@tcd.ie,  61394@debbugs.gnu.org
> Date: Sat, 11 Feb 2023 13:30:48 +0100
> 
> > Btw, using only the first 4KiB would mean a collision is still
> > possible, albeit rarely, right?  So your use case of having all the
> > thumbnails in the same directory could sometimes fail, right?
> 
> The 4KiB was "quite large but not so much" guess.  I've made tests with
> the following code:
> 
> --8<---------------cut here---------------start------------->8---
> (defun sha1-test (filename size)
>   (with-temp-buffer
>     (insert-file-contents-literally filename nil 0 size)
>     (sha1 (current-buffer))))
> 
> ;; From 1KiB to 64KiB
> (list
>  (benchmark-run-compiled 1000
>    (sha1-test "/tmp/a-5MiB-photo.jpg" (expt 2 10)))
>  (benchmark-run-compiled 1000
>    (sha1-test "/tmp/a-5MiB-photo.jpg" (expt 2 11)))
>  (benchmark-run-compiled 1000
>    (sha1-test "/tmp/a-5MiB-photo.jpg" (expt 2 12)))
>  (benchmark-run-compiled 1000
>    (sha1-test "/tmp/a-5MiB-photo.jpg" (expt 2 13)))
>  (benchmark-run-compiled 1000
>    (sha1-test "/tmp/a-5MiB-photo.jpg" (expt 2 14)))
>  (benchmark-run-compiled 1000
>    (sha1-test "/tmp/a-5MiB-photo.jpg" (expt 2 15)))
>  (benchmark-run-compiled 1000
>    (sha1-test "/tmp/a-5MiB-photo.jpg" (expt 2 16))))
> --8<---------------cut here---------------end--------------->8---
> 
> And here are the results on my machine:
> ((0.664336771 1 0.14466495299998883)
>  (0.707937024 2 0.28811983400001395)
>  (0.940229304 3 0.44037704100000497) ;; <- 4KiB
>  (1.672118528 4 0.7672738199999856)
>  (2.6194289370000003 6 1.046699996000001)
>  (3.169999951 11 1.5916382949999957)
>  (6.547043287 21 3.195145416999992))
> 
> So this 4KiB seems practical: about 1 second for one thousand run.
> WDYT?

I only commented on that because you said it allows you to use a
single directory.  Otherwise, I have no problems with using just 4KiB.

bug#61394: 30.0.50; [PATCH] Image-dired thumb name based on content

[Manuel Giraud via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Eli Zaretskii <eliz@gnu.org> writes:

[...]

> I only commented on that because you said it allows you to use a
> single directory.  Otherwise, I have no problems with using just 4KiB.

A single directory is already what is used now when
image-dired-thumbnail-storage is set to 'image-dired (the default).
Only the SHA-1 is taken from the file path.  I'll fix my patch with your
suggestions then.
-- 
Manuel Giraud

bug#61394: 30.0.50; [PATCH] Image-dired thumb name based on content

[Manuel Giraud via Bug reports for GNU Emacs, the Swiss army knife of text editors]

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

So, here is another version of the patch.  I'm not quite sure for the
docstring of image-dired-thumb-name.

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Image-dired-thumb-name-based-on-contents.patch --]
[-- Type: text/x-patch, Size: 2765 bytes --]

From 5cf749b1b321c5f1bd80ae4bb3e1dcf2ba5c02e1 Mon Sep 17 00:00:00 2001
From: Manuel Giraud <manuel@ledu-giraud.fr>
Date: Thu, 9 Feb 2023 19:56:37 +0100
Subject: [PATCH] Image-dired thumb name based on contents

* lisp/image/image-dired-util.el (image-dired-contents-sha1): New
utility that compute the SHA-1 of the first 4KiB of the contents of a
file.
(image-dired-thumb-name): Fix the docstring and use
image-dired-contents-sha1.
---
 lisp/image/image-dired-util.el | 23 ++++++++++++++++++-----
 1 file changed, 18 insertions(+), 5 deletions(-)

diff --git a/lisp/image/image-dired-util.el b/lisp/image/image-dired-util.el
index c03f9d2e3d3..cde9d0be8d8 100644
--- a/lisp/image/image-dired-util.el
+++ b/lisp/image/image-dired-util.el
@@ -57,18 +57,31 @@ image-dired-dir
       (message "Thumbnail directory created: %s" image-dired-dir))
     image-dired-dir))
 
+(defun image-dired-contents-sha1 (filename)
+  "Compute the SHA-1 of the first 4KiB of FILENAME's contents."
+  (with-temp-buffer
+    (insert-file-contents-literally filename nil 0 4096)
+    (sha1 (current-buffer))))
+
 (defun image-dired-thumb-name (file)
   "Return absolute file name for thumbnail FILE.
 Depending on the value of `image-dired-thumbnail-storage', the
 file name of the thumbnail will vary:
-- For `use-image-dired-dir', make a SHA1-hash of the image file's
-  directory name and add that to make the thumbnail file name
-  unique.
-- For `per-directory' storage, just add a subdirectory.
+
+- For `image-dired', make a SHA1-hash the contents of the image
+  FILE with `image-dired-contents-sha1' and append a \".jpg\"
+  extension.  This thumbnail will be stored in the
+  `image-dired-dir' directory.
+
+- For `per-directory', append a \".thumb.jpg\" extension to the
+  FILE's name.  This thumbnail will be stored in a
+  \".image-dired\" subdirectory into the FILE's directory.
+
 - For `standard' storage, produce the file name according to the
   Thumbnail Managing Standard.  Among other things, an MD5-hash
   of the image file's directory name will be added to the
   filename.
+
 See also `image-dired-thumbnail-storage'."
   (let ((file (expand-file-name file)))
     (cond ((memq image-dired-thumbnail-storage
@@ -85,7 +98,7 @@ image-dired-thumb-name
           ((or (eq 'image-dired image-dired-thumbnail-storage)
                ;; Maintained for backwards compatibility:
                (eq 'use-image-dired-dir image-dired-thumbnail-storage))
-           (expand-file-name (format "%s.jpg" (sha1 file))
+           (expand-file-name (format "%s.jpg" (image-dired-contents-sha1 file))
                              (image-dired-dir)))
           ((eq 'per-directory image-dired-thumbnail-storage)
            (expand-file-name (format "%s.thumb.jpg"
-- 
2.39.1


[-- Attachment #3: Type: text/plain, Size: 18 bytes --]

-- 
Manuel Giraud

bug#61394: 30.0.50; [PATCH] Image-dired thumb name based on content

[Stefan Kangas]

Manuel Giraud via "Bug reports for GNU Emacs, the Swiss army knife of
text editors" <bug-gnu-emacs@gnu.org> writes:

> So, here is another version of the patch.

What is the performance impact of this?  Could we see some benchmarks?
I routinely open folders with hundreds of files that are several
megabytes each, so I think this is the type of benchmark I would be
interested in.  This is common when working with images from a digital
camera.

If it has too much of a performance impact, we could consider making it
optional (and disable it by default).

bug#61394: 30.0.50; [PATCH] Image-dired thumb name based on content

[Manuel Giraud via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Stefan Kangas <stefankangas@gmail.com> writes:

> Manuel Giraud via "Bug reports for GNU Emacs, the Swiss army knife of
> text editors" <bug-gnu-emacs@gnu.org> writes:
>
>> So, here is another version of the patch.
>
> What is the performance impact of this?  Could we see some benchmarks?

The performance impact is important.  Here are the results from a list
of images of mine:
--8<---------------cut here---------------start------------->8---
(length *images*) -> 3664

(benchmark-run-compiled 10 (dolist (im *images*) (sha1 im)))
 -> (0.367976492 1 0.2809483390000196)
 
(benchmark-run-compiled 10 (dolist (im *images*) (image-dired-contents-sha1 im)))
 -> (72.115512605 84 26.079076938000014)
--8<---------------cut here---------------end--------------->8---

OTOH, using image-dired on a directory of 245 photos before and after
this patch I cannot feel any difference (after the thumbnails are done
of course).

> I routinely open folders with hundreds of files that are several
> megabytes each, so I think this is the type of benchmark I would be
> interested in.  This is common when working with images from a digital
> camera.
>
> If it has too much of a performance impact, we could consider making it
> optional (and disable it by default).

Maybe we could have this in another option for
`image-dired-thumbnail-storage'?  What do you think of
'image-dired-contents?
-- 
Manuel Giraud

bug#61394: 30.0.50; [PATCH] Image-dired thumb name based on content

[Stefan Kangas]

Manuel Giraud <manuel@ledu-giraud.fr> writes:

> The performance impact is important.  Here are the results from a list
> of images of mine:
> --8<---------------cut here---------------start------------->8---
> (length *images*) -> 3664
>
> (benchmark-run-compiled 10 (dolist (im *images*) (sha1 im)))
>  -> (0.367976492 1 0.2809483390000196)
>
> (benchmark-run-compiled 10 (dolist (im *images*) (image-dired-contents-sha1 im)))
>  -> (72.115512605 84 26.079076938000014)
> --8<---------------cut here---------------end--------------->8---

Thanks.  That's a slowdown by a factor close to 100, so while I think
the feature sounds useful, it should indeed be made optional.

> Maybe we could have this in another option for
> `image-dired-thumbnail-storage'?

I think 'image-dired-thumbnail-storage' is already complicated enough,
and I'd rather not complicate it further.

> What do you think of 'image-dired-contents?

Hmm, it sounds a bit too nondescript.  How about something like
'image-dired-thumbnail-naming'?

bug#61394: 30.0.50; [PATCH] Image-dired thumb name based on content

[Manuel Giraud via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Stefan Kangas <stefankangas@gmail.com> writes:

[...]

> Thanks.  That's a slowdown by a factor close to 100, so while I think
> the feature sounds useful, it should indeed be made optional.

I'm ok with that.

>> Maybe we could have this in another option for
>> `image-dired-thumbnail-storage'?
>
> I think 'image-dired-thumbnail-storage' is already complicated enough,
> and I'd rather not complicate it further.

I don't understand here.  Do you suggest introducing another
custom/variable?  'image-dired-thumbnail-storage' has currently only 3
possible values.  I was suggesting adding one more.

>> What do you think of 'image-dired-contents?
>
> Hmm, it sounds a bit too nondescript.  How about something like
> 'image-dired-thumbnail-naming'?

Do you mean as a name for a new variable? (I was talking about a new
possible value for image-dired-thumbnail-storage)
-- 
Manuel Giraud

bug#61394: 30.0.50; [PATCH] Image-dired thumb name based on content

[Stefan Kangas]

Manuel Giraud <manuel@ledu-giraud.fr> writes:

>> I think 'image-dired-thumbnail-storage' is already complicated enough,
>> and I'd rather not complicate it further.
>
> I don't understand here.  Do you suggest introducing another
> custom/variable?  'image-dired-thumbnail-storage' has currently only 3
> possible values.  I was suggesting adding one more.

That variable relates to *where* to store the thumbnail files, not how
to name the thumbnail files.  I might want to use `image-dired' or
`per-directory' using either thumbnail naming scheme.

So a new variable makes more sense, as it is orthogonal from
`image-dired-thumbnail-storage'.

>> Hmm, it sounds a bit too nondescript.  How about something like
>> 'image-dired-thumbnail-naming'?
>
> Do you mean as a name for a new variable? (I was talking about a new
> possible value for image-dired-thumbnail-storage)

Yes.  However, we currently have `image-dired-thumb-size', so perhaps
`image-dired-thumb-naming' is better.

bug#61394: 30.0.50; [PATCH] Image-dired thumb name based on content

[Manuel Giraud via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Stefan Kangas <stefankangas@gmail.com> writes:

> Manuel Giraud <manuel@ledu-giraud.fr> writes:
>
>>> I think 'image-dired-thumbnail-storage' is already complicated enough,
>>> and I'd rather not complicate it further.
>>
>> I don't understand here.  Do you suggest introducing another
>> custom/variable?  'image-dired-thumbnail-storage' has currently only 3
>> possible values.  I was suggesting adding one more.
>
> That variable relates to *where* to store the thumbnail files, not how
> to name the thumbnail files.  I might want to use `image-dired' or
> `per-directory' using either thumbnail naming scheme.
>
> So a new variable makes more sense, as it is orthogonal from
> `image-dired-thumbnail-storage'.
>
>>> Hmm, it sounds a bit too nondescript.  How about something like
>>> 'image-dired-thumbnail-naming'?
>>
>> Do you mean as a name for a new variable? (I was talking about a new
>> possible value for image-dired-thumbnail-storage)
>
> Yes.  However, we currently have `image-dired-thumb-size', so perhaps
> `image-dired-thumb-naming' is better.

Ok.  I like that.  I'll try to come up with something but I think I'll
do some cleaning in the process as currently
'image-dired-thumbnail-storage' decides more than just the "where"
(eg. add "thumb" somewhere).
-- 
Manuel Giraud

bug#61394: 30.0.50; [PATCH] Image-dired thumb name based on content

[Stefan Kangas]

Manuel Giraud <manuel@ledu-giraud.fr> writes:

> Ok.  I like that.  I'll try to come up with something but I think I'll
> do some cleaning in the process as currently
> 'image-dired-thumbnail-storage' decides more than just the "where"
> (eg. add "thumb" somewhere).

Sounds good, thanks.

bug#61394: 30.0.50; [PATCH] Image-dired thumb name based on content

[Manuel Giraud via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Hi,

While at that, I see that for an `image-dired-thumbnail-storage' other
than 'standard* the thumbnail name is hardcoded to "jpg" ("png" for
'standard* but it seems to be mandated by said standard).  Should it
stay that way or could we extract the extension from the image filename?
-- 
Manuel Giraud

bug#61394: 30.0.50; [PATCH] Image-dired thumb name based on content

[Manuel Giraud via Bug reports for GNU Emacs, the Swiss army knife of text editors]

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

Hi,

Here is a new version of the patch that, I think, addresses all the
issues raised by Stefan.

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-New-option-image-dired-thumb-naming-bug-61394.patch --]
[-- Type: text/x-patch, Size: 8552 bytes --]

From 3003c1f7bd5b65c076b33fe5ae986eeebdee6cce Mon Sep 17 00:00:00 2001
From: Manuel Giraud <manuel@ledu-giraud.fr>
Date: Sat, 25 Feb 2023 19:27:07 +0100
Subject: [PATCH] New option 'image-dired-thumb-naming' (bug#61394)

* lisp/image/image-dired.el (image-dired-thumb-naming): New user
option to control thumbnail name.

* lisp/image/image-dired-util.el (image-dired-thumb-name): Update
to use new user option and compute contents SHA-1 if needed.
(image-dired-contents-sha1): New function to compute the SHA-1 of the
first 4KiB of a file.
---
 etc/NEWS                       |  4 ++
 lisp/image/image-dired-util.el | 84 +++++++++++++++++++++-------------
 lisp/image/image-dired.el      | 26 ++++++++++-
 3 files changed, 79 insertions(+), 35 deletions(-)

diff --git a/etc/NEWS b/etc/NEWS
index 4b0e4e6bd46..eca4fdaafe8 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -192,6 +192,10 @@ This command adds a docstring comment to the current defun.  If a
 comment already exists, point is only moved to the comment.  It is
 bound to 'C-c C-d' in 'go-ts-mode'.
 
+** Image Dired
+*** New user option 'image-dired-thumb-naming'.
+You can now configure how a thumbnail is named using this option.
+
 \f
 * New Modes and Packages in Emacs 30.1
 
diff --git a/lisp/image/image-dired-util.el b/lisp/image/image-dired-util.el
index a80b3afc0f3..3f6880fc807 100644
--- a/lisp/image/image-dired-util.el
+++ b/lisp/image/image-dired-util.el
@@ -30,6 +30,7 @@
 (eval-when-compile (require 'cl-lib))
 
 (defvar image-dired-dir)
+(defvar image-dired-thumb-naming)
 (defvar image-dired-thumbnail-storage)
 
 (defconst image-dired--thumbnail-standard-sizes
@@ -57,42 +58,59 @@ image-dired-dir
       (message "Thumbnail directory created: %s" image-dired-dir))
     image-dired-dir))
 
+(defun image-dired-contents-sha1 (filename)
+  "Compute the SHA-1 of the first 4KiB of FILENAME's contents."
+  (with-temp-buffer
+    (insert-file-contents-literally filename nil 0 4096)
+    (sha1 (current-buffer))))
+
 (defun image-dired-thumb-name (file)
   "Return absolute file name for thumbnail FILE.
-Depending on the value of `image-dired-thumbnail-storage', the
-file name of the thumbnail will vary:
-- For `use-image-dired-dir', make a SHA1-hash of the image file's
-  directory name and add that to make the thumbnail file name
-  unique.
-- For `per-directory' storage, just add a subdirectory.
-- For `standard' storage, produce the file name according to the
-  Thumbnail Managing Standard.  Among other things, an MD5-hash
-  of the image file's directory name will be added to the
-  filename.
-See also `image-dired-thumbnail-storage'."
+Depending on the value of `image-dired-thumbnail-storage' and
+`image-dired-thumb-naming', the file name of the thumbnail will
+vary:
+
+- If `image-dired-thumbnail-storage' is set to one of the value
+  of `image-dired--thumbnail-standard-sizes', produce the file
+  name according to the Thumbnail Managing Standard.  Among other
+  things, an MD5-hash of the image file's directory name will be
+  added to the file name.
+
+- Otherwise `image-dired-thumbnail-storage' is used to set the
+  directory where to store the thumbnail.  In this latter case
+  the name given to the thumbnail depends on the value of
+  `image-dired-thumb-naming'.
+
+See also `image-dired-thumbnail-storage' and
+`image-dired-thumb-naming'."
   (let ((file (expand-file-name file)))
-    (cond ((memq image-dired-thumbnail-storage
-                 image-dired--thumbnail-standard-sizes)
-           (let ((thumbdir (cl-case image-dired-thumbnail-storage
-                             (standard "thumbnails/normal")
-                             (standard-large "thumbnails/large")
-                             (standard-x-large "thumbnails/x-large")
-                             (standard-xx-large "thumbnails/xx-large"))))
-             (expand-file-name
-              ;; MD5 is mandated by the Thumbnail Managing Standard.
-              (concat (md5 (concat "file://" file)) ".png")
-              (expand-file-name thumbdir (xdg-cache-home)))))
-          ((or (eq 'image-dired image-dired-thumbnail-storage)
-               ;; Maintained for backwards compatibility:
-               (eq 'use-image-dired-dir image-dired-thumbnail-storage))
-           (expand-file-name (format "%s.jpg" (sha1 file))
-                             (image-dired-dir)))
-          ((eq 'per-directory image-dired-thumbnail-storage)
-           (expand-file-name (format "%s.thumb.jpg"
-                                     (file-name-nondirectory file))
-                             (expand-file-name
-                              ".image-dired"
-                              (file-name-directory file)))))))
+    (if (memq image-dired-thumbnail-storage
+              image-dired--thumbnail-standard-sizes)
+        (let ((thumbdir (cl-case image-dired-thumbnail-storage
+                          (standard "thumbnails/normal")
+                          (standard-large "thumbnails/large")
+                          (standard-x-large "thumbnails/x-large")
+                          (standard-xx-large "thumbnails/xx-large"))))
+          (expand-file-name
+           ;; MD5 and PNG is mandated by the Thumbnail Managing
+           ;; Standard.
+           (concat (md5 (concat "file://" file)) ".png")
+           (expand-file-name thumbdir (xdg-cache-home))))
+      (let ((name (if (eq 'sha1-contents image-dired-thumb-naming)
+                      (image-dired-contents-sha1 file)
+                    ;; Defaults to SHA-1 of file name
+                    (if (eq 'per-directory image-dired-thumbnail-storage)
+                        (sha1 (file-name-nondirectory file))
+                      (sha1 file)))))
+        (cond ((or (eq 'image-dired image-dired-thumbnail-storage)
+                   ;; Maintained for backwards compatibility:
+                   (eq 'use-image-dired-dir image-dired-thumbnail-storage))
+               (expand-file-name (format "%s.jpg" name) (image-dired-dir)))
+              ((eq 'per-directory image-dired-thumbnail-storage)
+               (expand-file-name (format "%s.jpg" name)
+                                 (expand-file-name
+                                  ".image-dired"
+                                  (file-name-directory file)))))))))
 
 (defvar image-dired-thumbnail-buffer "*image-dired*"
   "Image-Dired's thumbnail buffer.")
diff --git a/lisp/image/image-dired.el b/lisp/image/image-dired.el
index 8e2a75a418f..8d56fd1e462 100644
--- a/lisp/image/image-dired.el
+++ b/lisp/image/image-dired.el
@@ -162,8 +162,27 @@ image-dired-dir
 `image-dired-thumbnail-storage'."
   :type 'directory)
 
+(defcustom image-dired-thumb-naming 'sha1-filename
+  "How `image-dired' names thumbnail files.
+When set to `sha1-filename' the name of thumbnail is built by
+computing the SHA-1 of the full file name of the image.
+
+When set to `sha1-contents' the name of thumbnail is built by
+computing the SHA-1 of first 4KiB of the image contents (See
+`image-dired-contents-sha1').
+
+In both case, a \"jpg\" extension is appended to save as JPEG.
+
+The value of this option is ignored if Image-Dired is customized
+to use the Thumbnail Managing Standard.  See
+`image-dired-thumbnail-storage'."
+  :type '(choice :tag "How to name thumbnail files"
+                 (const :tag "SHA-1 of the image file name" sha1-filename)
+                 (const :tag "SHA-1 of the image contents" sha1-contents))
+  :version "30.1")
+
 (defcustom image-dired-thumbnail-storage 'image-dired
-  "How `image-dired' stores thumbnail files.
+  "Where `image-dired' stores thumbnail files.
 There are three ways that Image-Dired can store and generate
 thumbnails:
 
@@ -189,6 +208,9 @@ image-dired-thumbnail-storage
 
     Set this user option to `per-directory'.
 
+To control the naming of thumbnails for alternatives (2) and (3)
+above, customize the value of `image-dired-thumb-naming'.
+
 To control the default size of thumbnails for alternatives (2)
 and (3) above, customize the value of `image-dired-thumb-size'.
 
@@ -197,7 +219,7 @@ image-dired-thumbnail-storage
 
 For more information on the Thumbnail Managing Standard, see:
 https://specifications.freedesktop.org/thumbnail-spec/thumbnail-spec-latest.html"
-  :type '(choice :tag "How to store thumbnail files"
+  :type '(choice :tag "Where to store thumbnail files"
                  (const :tag "Use image-dired-dir" image-dired)
                  (const :tag "Thumbnail Managing Standard (normal 128x128)"
                         standard)
-- 
2.39.1


[-- Attachment #3: Type: text/plain, Size: 18 bytes --]

-- 
Manuel Giraud

bug#61394: 30.0.50; [PATCH] Image-dired thumb name based on content

[Manuel Giraud via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Hi Stefan,

Just a "ping" on this.  Do you think it could go in Emacs 30?
-- 
Manuel Giraud

bug#61394: 30.0.50; [PATCH] Image-dired thumb name based on content

[Eli Zaretskii]

> From: Manuel Giraud <manuel@ledu-giraud.fr>
> Cc: contovob@tcd.ie,  Eli Zaretskii <eliz@gnu.org>,  61394@debbugs.gnu.org
> Date: Wed, 26 Jul 2023 21:18:12 +0200
> 
> Hi Stefan,
> 
> Just a "ping" on this.  Do you think it could go in Emacs 30?

Thanks for the reminder.  I think we should install this now.  So
please rebase on the current master branch and repost, and I will
install it then.

bug#61394: 30.0.50; [PATCH] Image-dired thumb name based on content

[Manuel Giraud via Bug reports for GNU Emacs, the Swiss army knife of text editors]

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

Eli Zaretskii <eliz@gnu.org> writes:

> Thanks for the reminder.  I think we should install this now.  So
> please rebase on the current master branch and repost, and I will
> install it then.

Thanks.  Here is the rebased version of the patch.
-- 
Manuel Giraud

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-New-option-image-dired-thumb-naming-bug-61394.patch --]
[-- Type: text/x-patch, Size: 8573 bytes --]

From 83002e2cda2c3c8c28b2e37837625709fc4ca233 Mon Sep 17 00:00:00 2001
From: Manuel Giraud <manuel@ledu-giraud.fr>
Date: Sat, 25 Feb 2023 19:27:07 +0100
Subject: [PATCH] New option 'image-dired-thumb-naming' (bug#61394)

* lisp/image/image-dired.el (image-dired-thumb-naming): New user
option to control thumbnail name.

* lisp/image/image-dired-util.el (image-dired-thumb-name): Update
to use new user option and compute contents SHA-1 if needed.
(image-dired-contents-sha1): New function to compute the SHA-1 of the
first 4KiB of a file.
---
 etc/NEWS                       |  5 ++
 lisp/image/image-dired-util.el | 84 +++++++++++++++++++++-------------
 lisp/image/image-dired.el      | 26 ++++++++++-
 3 files changed, 80 insertions(+), 35 deletions(-)

diff --git a/etc/NEWS b/etc/NEWS
index d0dab755212..a8e7d97df7e 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -567,6 +567,11 @@ Similarly to buffer restoration by Desktop, 'recentf-mode' checking
 of the accessibility of remote files can now time out if
 'remote-file-name-access-timeout' is set to a positive number.
 
+** Image Dired
+
+*** New user option 'image-dired-thumb-naming'.
+You can now configure how a thumbnail is named using this option.
+
 \f
 * New Modes and Packages in Emacs 30.1
 
diff --git a/lisp/image/image-dired-util.el b/lisp/image/image-dired-util.el
index a80b3afc0f3..3f6880fc807 100644
--- a/lisp/image/image-dired-util.el
+++ b/lisp/image/image-dired-util.el
@@ -30,6 +30,7 @@
 (eval-when-compile (require 'cl-lib))
 
 (defvar image-dired-dir)
+(defvar image-dired-thumb-naming)
 (defvar image-dired-thumbnail-storage)
 
 (defconst image-dired--thumbnail-standard-sizes
@@ -57,42 +58,59 @@ image-dired-dir
       (message "Thumbnail directory created: %s" image-dired-dir))
     image-dired-dir))
 
+(defun image-dired-contents-sha1 (filename)
+  "Compute the SHA-1 of the first 4KiB of FILENAME's contents."
+  (with-temp-buffer
+    (insert-file-contents-literally filename nil 0 4096)
+    (sha1 (current-buffer))))
+
 (defun image-dired-thumb-name (file)
   "Return absolute file name for thumbnail FILE.
-Depending on the value of `image-dired-thumbnail-storage', the
-file name of the thumbnail will vary:
-- For `use-image-dired-dir', make a SHA1-hash of the image file's
-  directory name and add that to make the thumbnail file name
-  unique.
-- For `per-directory' storage, just add a subdirectory.
-- For `standard' storage, produce the file name according to the
-  Thumbnail Managing Standard.  Among other things, an MD5-hash
-  of the image file's directory name will be added to the
-  filename.
-See also `image-dired-thumbnail-storage'."
+Depending on the value of `image-dired-thumbnail-storage' and
+`image-dired-thumb-naming', the file name of the thumbnail will
+vary:
+
+- If `image-dired-thumbnail-storage' is set to one of the value
+  of `image-dired--thumbnail-standard-sizes', produce the file
+  name according to the Thumbnail Managing Standard.  Among other
+  things, an MD5-hash of the image file's directory name will be
+  added to the file name.
+
+- Otherwise `image-dired-thumbnail-storage' is used to set the
+  directory where to store the thumbnail.  In this latter case
+  the name given to the thumbnail depends on the value of
+  `image-dired-thumb-naming'.
+
+See also `image-dired-thumbnail-storage' and
+`image-dired-thumb-naming'."
   (let ((file (expand-file-name file)))
-    (cond ((memq image-dired-thumbnail-storage
-                 image-dired--thumbnail-standard-sizes)
-           (let ((thumbdir (cl-case image-dired-thumbnail-storage
-                             (standard "thumbnails/normal")
-                             (standard-large "thumbnails/large")
-                             (standard-x-large "thumbnails/x-large")
-                             (standard-xx-large "thumbnails/xx-large"))))
-             (expand-file-name
-              ;; MD5 is mandated by the Thumbnail Managing Standard.
-              (concat (md5 (concat "file://" file)) ".png")
-              (expand-file-name thumbdir (xdg-cache-home)))))
-          ((or (eq 'image-dired image-dired-thumbnail-storage)
-               ;; Maintained for backwards compatibility:
-               (eq 'use-image-dired-dir image-dired-thumbnail-storage))
-           (expand-file-name (format "%s.jpg" (sha1 file))
-                             (image-dired-dir)))
-          ((eq 'per-directory image-dired-thumbnail-storage)
-           (expand-file-name (format "%s.thumb.jpg"
-                                     (file-name-nondirectory file))
-                             (expand-file-name
-                              ".image-dired"
-                              (file-name-directory file)))))))
+    (if (memq image-dired-thumbnail-storage
+              image-dired--thumbnail-standard-sizes)
+        (let ((thumbdir (cl-case image-dired-thumbnail-storage
+                          (standard "thumbnails/normal")
+                          (standard-large "thumbnails/large")
+                          (standard-x-large "thumbnails/x-large")
+                          (standard-xx-large "thumbnails/xx-large"))))
+          (expand-file-name
+           ;; MD5 and PNG is mandated by the Thumbnail Managing
+           ;; Standard.
+           (concat (md5 (concat "file://" file)) ".png")
+           (expand-file-name thumbdir (xdg-cache-home))))
+      (let ((name (if (eq 'sha1-contents image-dired-thumb-naming)
+                      (image-dired-contents-sha1 file)
+                    ;; Defaults to SHA-1 of file name
+                    (if (eq 'per-directory image-dired-thumbnail-storage)
+                        (sha1 (file-name-nondirectory file))
+                      (sha1 file)))))
+        (cond ((or (eq 'image-dired image-dired-thumbnail-storage)
+                   ;; Maintained for backwards compatibility:
+                   (eq 'use-image-dired-dir image-dired-thumbnail-storage))
+               (expand-file-name (format "%s.jpg" name) (image-dired-dir)))
+              ((eq 'per-directory image-dired-thumbnail-storage)
+               (expand-file-name (format "%s.jpg" name)
+                                 (expand-file-name
+                                  ".image-dired"
+                                  (file-name-directory file)))))))))
 
 (defvar image-dired-thumbnail-buffer "*image-dired*"
   "Image-Dired's thumbnail buffer.")
diff --git a/lisp/image/image-dired.el b/lisp/image/image-dired.el
index b13b3e08ce2..96a0c2ef9bc 100644
--- a/lisp/image/image-dired.el
+++ b/lisp/image/image-dired.el
@@ -162,8 +162,27 @@ image-dired-dir
 `image-dired-thumbnail-storage'."
   :type 'directory)
 
+(defcustom image-dired-thumb-naming 'sha1-filename
+  "How `image-dired' names thumbnail files.
+When set to `sha1-filename' the name of thumbnail is built by
+computing the SHA-1 of the full file name of the image.
+
+When set to `sha1-contents' the name of thumbnail is built by
+computing the SHA-1 of first 4KiB of the image contents (See
+`image-dired-contents-sha1').
+
+In both case, a \"jpg\" extension is appended to save as JPEG.
+
+The value of this option is ignored if Image-Dired is customized
+to use the Thumbnail Managing Standard.  See
+`image-dired-thumbnail-storage'."
+  :type '(choice :tag "How to name thumbnail files"
+                 (const :tag "SHA-1 of the image file name" sha1-filename)
+                 (const :tag "SHA-1 of the image contents" sha1-contents))
+  :version "30.1")
+
 (defcustom image-dired-thumbnail-storage 'image-dired
-  "How `image-dired' stores thumbnail files.
+  "Where `image-dired' stores thumbnail files.
 There are three ways that Image-Dired can store and generate
 thumbnails:
 
@@ -189,6 +208,9 @@ image-dired-thumbnail-storage
 
     Set this user option to `per-directory'.
 
+To control the naming of thumbnails for alternatives (2) and (3)
+above, customize the value of `image-dired-thumb-naming'.
+
 To control the default size of thumbnails for alternatives (2)
 and (3) above, customize the value of `image-dired-thumb-size'.
 
@@ -197,7 +219,7 @@ image-dired-thumbnail-storage
 
 For more information on the Thumbnail Managing Standard, see:
 https://specifications.freedesktop.org/thumbnail-spec/thumbnail-spec-latest.html"
-  :type '(choice :tag "How to store thumbnail files"
+  :type '(choice :tag "Where to store thumbnail files"
                  (const :tag "Use image-dired-dir" image-dired)
                  (const :tag "Thumbnail Managing Standard (normal 128x128)"
                         standard)
-- 
2.40.0

bug#61394: 30.0.50; [PATCH] Image-dired thumb name based on content

[Eli Zaretskii]

> From: Manuel Giraud <manuel@ledu-giraud.fr>
> Cc: stefankangas@gmail.com,  contovob@tcd.ie,  61394@debbugs.gnu.org
> Date: Thu, 27 Jul 2023 15:52:55 +0200
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> > Thanks for the reminder.  I think we should install this now.  So
> > please rebase on the current master branch and repost, and I will
> > install it then.
> 
> Thanks.  Here is the rebased version of the patch.

Thanks, installed.  But now 1 set in image-dired-util-tests fails; it
sounds like the method of producing per-directory thumbnails has
changed, and now produces a SHA-1 hash?  Can you please look into
fixing that test?

bug#61394: 30.0.50; [PATCH] Image-dired thumb name based on content

[Manuel Giraud via Bug reports for GNU Emacs, the Swiss army knife of text editors]

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

Eli Zaretskii <eliz@gnu.org> writes:

[...]

> Thanks, installed.  But now 1 set in image-dired-util-tests fails; it
> sounds like the method of producing per-directory thumbnails has
> changed, and now produces a SHA-1 hash?  Can you please look into
> fixing that test?

Ok, I think this patch should do.
-- 
Manuel Giraud

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Fix-test-after-83b6a8a5147.patch --]
[-- Type: text/x-patch, Size: 2318 bytes --]

From 1460fb462300aac70faf99724efe126c72ad5fe6 Mon Sep 17 00:00:00 2001
From: Manuel Giraud <manuel@ledu-giraud.fr>
Date: Thu, 27 Jul 2023 23:26:30 +0200
Subject: [PATCH] Fix test after 83b6a8a5147

---
 test/lisp/image/image-dired-util-tests.el | 21 ++++++++++++---------
 1 file changed, 12 insertions(+), 9 deletions(-)

diff --git a/test/lisp/image/image-dired-util-tests.el b/test/lisp/image/image-dired-util-tests.el
index 1f3747a82b1..273a32d5dbb 100644
--- a/test/lisp/image/image-dired-util-tests.el
+++ b/test/lisp/image/image-dired-util-tests.el
@@ -57,20 +57,23 @@ image-dired-thumb-name/image-dired
                      "jpg")))))
 
 (ert-deftest image-dired-thumb-name/per-directory ()
-  (let ((image-dired-thumbnail-storage 'per-directory))
-    (should (file-name-absolute-p (image-dired-thumb-name "foo.jpg")))
-    (should (file-name-absolute-p (image-dired-thumb-name "/tmp/foo.jpg")))
+  (let ((image-dired-thumbnail-storage 'per-directory)
+        (rel-path "foo.jpg")
+        (abs-path "/tmp/foo.jpg")
+        (hash-name (concat (sha1 "foo.jpg") ".jpg")))
+    (should (file-name-absolute-p (image-dired-thumb-name rel-path)))
+    (should (file-name-absolute-p (image-dired-thumb-name abs-path)))
     (should (equal
-             (file-name-nondirectory (image-dired-thumb-name "foo.jpg"))
-             (file-name-nondirectory (image-dired-thumb-name "/tmp/foo.jpg"))))
+             (file-name-nondirectory (image-dired-thumb-name rel-path))
+             (file-name-nondirectory (image-dired-thumb-name abs-path))))
     ;; The cdr below avoids the system dependency in the car of the
     ;; list returned by 'file-name-split': it's "" on Posix systems,
     ;; but the drive letter on MS-Windows.
     (should (equal (cdr (file-name-split
-                         (image-dired-thumb-name "/tmp/foo.jpg")))
-                   '("tmp" ".image-dired" "foo.jpg.thumb.jpg")))
+                         (image-dired-thumb-name abs-path)))
+                   (list "tmp" ".image-dired" hash-name)))
     (should (equal (file-name-nondirectory
-                    (image-dired-thumb-name "foo.jpg"))
-                   "foo.jpg.thumb.jpg"))))
+                    (image-dired-thumb-name rel-path))
+                   hash-name))))
 
 ;;; image-dired-util-tests.el ends here
-- 
2.40.0

bug#61394: 30.0.50; [PATCH] Image-dired thumb name based on content

[Eli Zaretskii]

> From: Manuel Giraud <manuel@ledu-giraud.fr>
> Cc: stefankangas@gmail.com,  contovob@tcd.ie,  61394@debbugs.gnu.org
> Date: Thu, 27 Jul 2023 23:30:29 +0200
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> [...]
> 
> > Thanks, installed.  But now 1 set in image-dired-util-tests fails; it
> > sounds like the method of producing per-directory thumbnails has
> > changed, and now produces a SHA-1 hash?  Can you please look into
> > fixing that test?
> 
> Ok, I think this patch should do.

Thanks, the tests now pass, but I wonder about this part:

>      (should (equal (cdr (file-name-split
> -                         (image-dired-thumb-name "/tmp/foo.jpg")))
> -                   '("tmp" ".image-dired" "foo.jpg.thumb.jpg")))
> +                         (image-dired-thumb-name abs-path)))
> +                   (list "tmp" ".image-dired" hash-name)))

Does this mean that thumbnail naming under 'per-directory' has
changed, and it now uses the SHA-1 hash of the base-name?  IOW, does
this mean your changes for bug#61394 included incompatible changes in
behavior?

bug#61394: 30.0.50; [PATCH] Image-dired thumb name based on content

[Manuel Giraud via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Eli Zaretskii <eliz@gnu.org> writes:

[...]

> Thanks, the tests now pass, but I wonder about this part:
>
>>      (should (equal (cdr (file-name-split
>> -                         (image-dired-thumb-name "/tmp/foo.jpg")))
>> -                   '("tmp" ".image-dired" "foo.jpg.thumb.jpg")))
>> +                         (image-dired-thumb-name abs-path)))
>> +                   (list "tmp" ".image-dired" hash-name)))
>
> Does this mean that thumbnail naming under 'per-directory' has
> changed, and it now uses the SHA-1 hash of the base-name?  IOW, does
> this mean your changes for bug#61394 included incompatible changes in
> behavior?

Yes I think it does.  My patch for bug#61394 changes the previous
behaviour of 'image-dired-thumbnail-storage'.  Now
'image-dired-thumbnail-storage' defines where (ie. in which directory)
the thumbnails are stored and I introduce 'image-dired-thumb-naming'
which tells how thumbnail file ared named (ie. the file name part sans
directory).

'image-dired-thumb-naming' is meaning less if
'image-dired-thumbnail-storage' is one of the "standard*" method because
those methods already define storage locations, file names and even
sizes.  But for the "per-directory" method, I'm using
'image-dired-thumb-naming'.  As we are talking about thumbnail I did not
think it was a big deal but if it is I can prepare a patch, on top of
the one in place, and then 'image-dired-thumb-naming' will be used only
for the "image-dired" storage method.  WDYT?
-- 
Manuel Giraud

bug#61394: 30.0.50; [PATCH] Image-dired thumb name based on content

[Eli Zaretskii]

> From: Manuel Giraud <manuel@ledu-giraud.fr>
> Cc: stefankangas@gmail.com,  contovob@tcd.ie,  61394@debbugs.gnu.org
> Date: Fri, 28 Jul 2023 11:33:19 +0200
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> [...]
> 
> > Thanks, the tests now pass, but I wonder about this part:
> >
> >>      (should (equal (cdr (file-name-split
> >> -                         (image-dired-thumb-name "/tmp/foo.jpg")))
> >> -                   '("tmp" ".image-dired" "foo.jpg.thumb.jpg")))
> >> +                         (image-dired-thumb-name abs-path)))
> >> +                   (list "tmp" ".image-dired" hash-name)))
> >
> > Does this mean that thumbnail naming under 'per-directory' has
> > changed, and it now uses the SHA-1 hash of the base-name?  IOW, does
> > this mean your changes for bug#61394 included incompatible changes in
> > behavior?
> 
> Yes I think it does.  My patch for bug#61394 changes the previous
> behaviour of 'image-dired-thumbnail-storage'.  Now
> 'image-dired-thumbnail-storage' defines where (ie. in which directory)
> the thumbnails are stored and I introduce 'image-dired-thumb-naming'
> which tells how thumbnail file ared named (ie. the file name part sans
> directory).
> 
> 'image-dired-thumb-naming' is meaning less if
> 'image-dired-thumbnail-storage' is one of the "standard*" method because
> those methods already define storage locations, file names and even
> sizes.  But for the "per-directory" method, I'm using
> 'image-dired-thumb-naming'.  As we are talking about thumbnail I did not
> think it was a big deal but if it is I can prepare a patch, on top of
> the one in place, and then 'image-dired-thumb-naming' will be used only
> for the "image-dired" storage method.  WDYT?

I don't think I understand all the aspects of this, as I use neither
image-dired nor the thumbnails.  But it sounds like an incompatible
change in behavior wrt what we have in Emacs 29?  If so, how do we
expect this to work for users who will have configured their Emacs for
some particular storage type, and then upgrade to Emacs 30 when that
is released?  Will the existing thumbnails still work for them?  Will
Emacs 30 now start storing thumbnails under differently-formatted
names, even though the user didn't change his/her configuration?

In general, any incompatible change in behavior (if there is indeed
such a change caused by this changeset) is undesirable.  So I'd like
first to discuss why there was a need for the behavior change in the
first place.

(I'm sorry that I didn't realize there was a change in behavior before
installing the changeset.  It seems we never discussed that aspect in
the bug#61394 discussion thread.)

bug#61394: 30.0.50; [PATCH] Image-dired thumb name based on content

[Manuel Giraud via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Eli Zaretskii <eliz@gnu.org> writes:

[...]

>> Yes I think it does.  My patch for bug#61394 changes the previous
>> behaviour of 'image-dired-thumbnail-storage'.  Now
>> 'image-dired-thumbnail-storage' defines where (ie. in which directory)
>> the thumbnails are stored and I introduce 'image-dired-thumb-naming'
>> which tells how thumbnail file ared named (ie. the file name part sans
>> directory).
>> 
>> 'image-dired-thumb-naming' is meaning less if
>> 'image-dired-thumbnail-storage' is one of the "standard*" method because
>> those methods already define storage locations, file names and even
>> sizes.  But for the "per-directory" method, I'm using
>> 'image-dired-thumb-naming'.  As we are talking about thumbnail I did not
>> think it was a big deal but if it is I can prepare a patch, on top of
>> the one in place, and then 'image-dired-thumb-naming' will be used only
>> for the "image-dired" storage method.  WDYT?
>
> I don't think I understand all the aspects of this, as I use neither
> image-dired nor the thumbnails.  But it sounds like an incompatible
> change in behavior wrt what we have in Emacs 29?  If so, how do we
> expect this to work for users who will have configured their Emacs for
> some particular storage type, and then upgrade to Emacs 30 when that
> is released?  Will the existing thumbnails still work for them?  Will
> Emacs 30 now start storing thumbnails under differently-formatted
> names, even though the user didn't change his/her configuration?

AFAIU image-dired thumbnails' generation, this will automatically work
for users from Emacs 29... but you are right that new thumbnails (with
new names) will be generated.  This will be true for users of the
"per-directory" storage method.  This will also be true when one user
customize the 'image-dired-thumb-naming' to 'sha1-contents'.

> In general, any incompatible change in behavior (if there is indeed
> such a change caused by this changeset) is undesirable.  So I'd like
> first to discuss why there was a need for the behavior change in the
> first place.

If you talk about the behaviour change for the "per-directory" method,
as I said, I can restore it back to what it was.
-- 
Manuel Giraud

bug#61394: 30.0.50; [PATCH] Image-dired thumb name based on content

[Eli Zaretskii]

> From: Manuel Giraud <manuel@ledu-giraud.fr>
> Cc: stefankangas@gmail.com,  contovob@tcd.ie,  61394@debbugs.gnu.org
> Date: Fri, 28 Jul 2023 18:00:52 +0200
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> > In general, any incompatible change in behavior (if there is indeed
> > such a change caused by this changeset) is undesirable.  So I'd like
> > first to discuss why there was a need for the behavior change in the
> > first place.
> 
> If you talk about the behaviour change for the "per-directory" method,
> as I said, I can restore it back to what it was.

Yes, I think we should do that, thanks.

bug#61394: 30.0.50; [PATCH] Image-dired thumb name based on content

[Manuel Giraud via Bug reports for GNU Emacs, the Swiss army knife of text editors]

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

Eli Zaretskii <eliz@gnu.org> writes:

[...]

>> If you talk about the behaviour change for the "per-directory" method,
>> as I said, I can restore it back to what it was.
>
> Yes, I think we should do that, thanks.

Hi Eli,

So I think that this patch does that but then 5efc7b22cec should be
reverted.

I'm not so sure about the 'image-dired-thumb-name' docstring.

BTW, I'm not running "make check" often but I have some failures here
and there (2 in TRAMP, 2 in EPG...): is it expected?
-- 
Manuel Giraud

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Revert-thumbnail-naming-for-per-directory-storage-me.patch --]
[-- Type: text/x-patch, Size: 3087 bytes --]

From f4c3260e982c0ff5f8afc0d80f634d5d3a3f991b Mon Sep 17 00:00:00 2001
From: Manuel Giraud <manuel@ledu-giraud.fr>
Date: Sat, 29 Jul 2023 10:59:32 +0200
Subject: [PATCH] Revert thumbnail naming for 'per-directory storage method

* lisp/image/image-dired-util.el (image-dired-thumb-name): Revert
to "filename.thumb.jpg" for 'per-directory storage.

* lisp/image/image-dired.el (image-dired-thumbnail-storage): Fix
documentation.
---
 lisp/image/image-dired-util.el | 12 ++++++------
 lisp/image/image-dired.el      |  4 ++--
 2 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/lisp/image/image-dired-util.el b/lisp/image/image-dired-util.el
index 3f6880fc807..70911bce45a 100644
--- a/lisp/image/image-dired-util.el
+++ b/lisp/image/image-dired-util.el
@@ -77,8 +77,9 @@ image-dired-thumb-name
   added to the file name.
 
 - Otherwise `image-dired-thumbnail-storage' is used to set the
-  directory where to store the thumbnail.  In this latter case
-  the name given to the thumbnail depends on the value of
+  directory where to store the thumbnail.  In this latter case,
+  if `image-dired-thumbnail-storage' is set to `image-dired' the
+  file name given to the thumbnail depends on the value of
   `image-dired-thumb-naming'.
 
 See also `image-dired-thumbnail-storage' and
@@ -99,15 +100,14 @@ image-dired-thumb-name
       (let ((name (if (eq 'sha1-contents image-dired-thumb-naming)
                       (image-dired-contents-sha1 file)
                     ;; Defaults to SHA-1 of file name
-                    (if (eq 'per-directory image-dired-thumbnail-storage)
-                        (sha1 (file-name-nondirectory file))
-                      (sha1 file)))))
+                    (sha1 file))))
         (cond ((or (eq 'image-dired image-dired-thumbnail-storage)
                    ;; Maintained for backwards compatibility:
                    (eq 'use-image-dired-dir image-dired-thumbnail-storage))
                (expand-file-name (format "%s.jpg" name) (image-dired-dir)))
               ((eq 'per-directory image-dired-thumbnail-storage)
-               (expand-file-name (format "%s.jpg" name)
+               (expand-file-name (format "%s.thumb.jpg"
+                                         (file-name-nondirectory file))
                                  (expand-file-name
                                   ".image-dired"
                                   (file-name-directory file)))))))))
diff --git a/lisp/image/image-dired.el b/lisp/image/image-dired.el
index 96a0c2ef9bc..c5b317e6b2a 100644
--- a/lisp/image/image-dired.el
+++ b/lisp/image/image-dired.el
@@ -208,8 +208,8 @@ image-dired-thumbnail-storage
 
     Set this user option to `per-directory'.
 
-To control the naming of thumbnails for alternatives (2) and (3)
-above, customize the value of `image-dired-thumb-naming'.
+To control the naming of thumbnails for alternative 2 above,
+customize the value of `image-dired-thumb-naming'.
 
 To control the default size of thumbnails for alternatives (2)
 and (3) above, customize the value of `image-dired-thumb-size'.
-- 
2.40.0

bug#61394: 30.0.50; [PATCH] Image-dired thumb name based on content

[Eli Zaretskii]

> From: Manuel Giraud <manuel@ledu-giraud.fr>
> Cc: stefankangas@gmail.com,  contovob@tcd.ie,  61394@debbugs.gnu.org
> Date: Sat, 29 Jul 2023 11:51:19 +0200
> 
> So I think that this patch does that but then 5efc7b22cec should be
> reverted.
> 
> I'm not so sure about the 'image-dired-thumb-name' docstring.

Thanks, will look into that soon.

> BTW, I'm not running "make check" often but I have some failures here
> and there (2 in TRAMP, 2 in EPG...): is it expected?

Please report them as separate bugs.

bug#61394: 30.0.50; [PATCH] Image-dired thumb name based on content

[Michael Albinus]

Manuel Giraud via "Bug reports for GNU Emacs, the Swiss army knife of
text editors" <bug-gnu-emacs@gnu.org> writes:

Hi Manuel,

> BTW, I'm not running "make check" often but I have some failures here
> and there (2 in TRAMP, 2 in EPG...): is it expected?

No (speaking for Tramp). Could you pls show them?

Best regards, Michael.

bug#61394: 30.0.50; [PATCH] Image-dired thumb name based on content

[Manuel Giraud via Bug reports for GNU Emacs, the Swiss army knife of text editors]

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

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Manuel Giraud <manuel@ledu-giraud.fr>
>> Cc: stefankangas@gmail.com,  contovob@tcd.ie,  61394@debbugs.gnu.org
>> Date: Sat, 29 Jul 2023 11:51:19 +0200
>> 
>> So I think that this patch does that but then 5efc7b22cec should be
>> reverted.
>> 
>> I'm not so sure about the 'image-dired-thumb-name' docstring.
>
> Thanks, will look into that soon.

Here is a new version of this patch.  I missed some precision in a
docstring in the previous one (sorry).

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Revert-thumbnail-naming-for-per-directory-storage-me.patch --]
[-- Type: text/x-patch, Size: 3679 bytes --]

From 9db8988412a8d2463d3d44ff23b80e545e9e5b19 Mon Sep 17 00:00:00 2001
From: Manuel Giraud <manuel@ledu-giraud.fr>
Date: Sat, 29 Jul 2023 10:59:32 +0200
Subject: [PATCH] Revert thumbnail naming for 'per-directory storage method

* lisp/image/image-dired-util.el (image-dired-thumb-name): Revert
to "filename.thumb.jpg" for 'per-directory storage.

* lisp/image/image-dired.el (image-dired-thumbnail-storage): Fix
documentation.
---
 lisp/image/image-dired-util.el | 12 ++++++------
 lisp/image/image-dired.el      |  8 ++++----
 2 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/lisp/image/image-dired-util.el b/lisp/image/image-dired-util.el
index 3f6880fc807..70911bce45a 100644
--- a/lisp/image/image-dired-util.el
+++ b/lisp/image/image-dired-util.el
@@ -77,8 +77,9 @@ image-dired-thumb-name
   added to the file name.
 
 - Otherwise `image-dired-thumbnail-storage' is used to set the
-  directory where to store the thumbnail.  In this latter case
-  the name given to the thumbnail depends on the value of
+  directory where to store the thumbnail.  In this latter case,
+  if `image-dired-thumbnail-storage' is set to `image-dired' the
+  file name given to the thumbnail depends on the value of
   `image-dired-thumb-naming'.
 
 See also `image-dired-thumbnail-storage' and
@@ -99,15 +100,14 @@ image-dired-thumb-name
       (let ((name (if (eq 'sha1-contents image-dired-thumb-naming)
                       (image-dired-contents-sha1 file)
                     ;; Defaults to SHA-1 of file name
-                    (if (eq 'per-directory image-dired-thumbnail-storage)
-                        (sha1 (file-name-nondirectory file))
-                      (sha1 file)))))
+                    (sha1 file))))
         (cond ((or (eq 'image-dired image-dired-thumbnail-storage)
                    ;; Maintained for backwards compatibility:
                    (eq 'use-image-dired-dir image-dired-thumbnail-storage))
                (expand-file-name (format "%s.jpg" name) (image-dired-dir)))
               ((eq 'per-directory image-dired-thumbnail-storage)
-               (expand-file-name (format "%s.jpg" name)
+               (expand-file-name (format "%s.thumb.jpg"
+                                         (file-name-nondirectory file))
                                  (expand-file-name
                                   ".image-dired"
                                   (file-name-directory file)))))))))
diff --git a/lisp/image/image-dired.el b/lisp/image/image-dired.el
index 96a0c2ef9bc..98596510ec1 100644
--- a/lisp/image/image-dired.el
+++ b/lisp/image/image-dired.el
@@ -174,8 +174,8 @@ image-dired-thumb-naming
 In both case, a \"jpg\" extension is appended to save as JPEG.
 
 The value of this option is ignored if Image-Dired is customized
-to use the Thumbnail Managing Standard.  See
-`image-dired-thumbnail-storage'."
+to use the Thumbnail Managing Standard or the per-directory
+thumbnails setting.  See `image-dired-thumbnail-storage'."
   :type '(choice :tag "How to name thumbnail files"
                  (const :tag "SHA-1 of the image file name" sha1-filename)
                  (const :tag "SHA-1 of the image contents" sha1-contents))
@@ -208,8 +208,8 @@ image-dired-thumbnail-storage
 
     Set this user option to `per-directory'.
 
-To control the naming of thumbnails for alternatives (2) and (3)
-above, customize the value of `image-dired-thumb-naming'.
+To control the naming of thumbnails for alternative (2) above,
+customize the value of `image-dired-thumb-naming'.
 
 To control the default size of thumbnails for alternatives (2)
 and (3) above, customize the value of `image-dired-thumb-size'.
-- 
2.40.0


[-- Attachment #3: Type: text/plain, Size: 237 bytes --]


>> BTW, I'm not running "make check" often but I have some failures here
>> and there (2 in TRAMP, 2 in EPG...): is it expected?
>
> Please report them as separate bugs.

Ok, I've created one for Tramp for the moment.
-- 
Manuel Giraud

bug#61394: 30.0.50; [PATCH] Image-dired thumb name based on content

[Manuel Giraud via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Hi,

FTR, there is a thing that does not work with this "thumb name based on
content": upon rotation the thumb is not updated properly.

This comes from the fact that the image content has changed (obviously)
so the new thumb will have a new name and is not displayed in the
*image-dired* buffer 😅.  I'd have to find a way around it.
-- 
Manuel Giraud

bug#61394: 30.0.50; [PATCH] Image-dired thumb name based on content

[Manuel Giraud via Bug reports for GNU Emacs, the Swiss army knife of text editors]

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

Manuel Giraud <manuel@ledu-giraud.fr> writes:

> Hi,
>
> FTR, there is a thing that does not work with this "thumb name based on
> content": upon rotation the thumb is not updated properly.
>
> This comes from the fact that the image content has changed (obviously)
> so the new thumb will have a new name and is not displayed in the
> *image-dired* buffer 😅.  I'd have to find a way around it.

Hi,

So here is a patch that fixes this issue.  Thanks.
-- 
Manuel Giraud

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Fix-thumbnail-update-when-thumb-name-is-based-on-ima.patch --]
[-- Type: text/x-patch, Size: 3111 bytes --]

From 5716e77a61a7a7459a00e266c4d6236be48c02c4 Mon Sep 17 00:00:00 2001
From: Manuel Giraud <manuel@ledu-giraud.fr>
Date: Tue, 1 Aug 2023 18:56:33 +0200
Subject: [PATCH] Fix thumbnail update when thumb name is based on image
 content

* lisp/image/image-dired-util.el
(image-dired-update-thumbnail-at-point): New function to update
thumbnail when original image contents changed.

* lisp/image/image-dired-external.el
(image-dired-rotate-original): Use it.

* lisp/image/image-dired.el (image-dired-display-thumbs): Fix
spacing while here.
---
 lisp/image/image-dired-external.el |  3 ++-
 lisp/image/image-dired-util.el     | 15 +++++++++++++++
 lisp/image/image-dired.el          |  2 +-
 3 files changed, 18 insertions(+), 2 deletions(-)

diff --git a/lisp/image/image-dired-external.el b/lisp/image/image-dired-external.el
index 9f35e17a7e6..77352c25a3b 100644
--- a/lisp/image/image-dired-external.el
+++ b/lisp/image/image-dired-external.el
@@ -405,7 +405,8 @@ image-dired-rotate-original
                 (not image-dired-rotate-original-ask-before-overwrite))
             (progn
               (copy-file image-dired-temp-rotate-image-file file t)
-              (image-dired-refresh-thumb))
+              (image-dired-refresh-thumb)
+              (image-dired-update-thumbnail-at-point))
           (image-dired-display-image file))))))
 
 \f
diff --git a/lisp/image/image-dired-util.el b/lisp/image/image-dired-util.el
index 3f6880fc807..1114f50477c 100644
--- a/lisp/image/image-dired-util.el
+++ b/lisp/image/image-dired-util.el
@@ -190,6 +190,21 @@ image-dired-image-at-point-p
   "Return non-nil if there is an `image-dired' thumbnail at point."
   (get-text-property (point) 'image-dired-thumbnail))
 
+(defun image-dired-update-thumbnail-at-point ()
+  "Update the thumbnail at point if the original image file has been
+modified.  Take care of uncaching and removing the old thumbnail
+file."
+  (when (image-dired-image-at-point-p)
+    (let* ((file (image-dired-original-file-name))
+           (thumb (expand-file-name (image-dired-thumb-name file)))
+           (image (get-text-property (point) 'display)))
+      (when image
+        (let ((old-thumb (plist-get (cdr image) :file)))
+          (unless (string= thumb old-thumb)
+            (clear-image-cache old-thumb)
+            (delete-file old-thumb)
+            (setf (plist-get (cdr image) :file) thumb)))))))
+
 (defun image-dired-window-width-pixels (window)
   "Calculate WINDOW width in pixels."
   (declare (obsolete window-body-width "29.1"))
diff --git a/lisp/image/image-dired.el b/lisp/image/image-dired.el
index 96a0c2ef9bc..7b0059e2f7a 100644
--- a/lisp/image/image-dired.el
+++ b/lisp/image/image-dired.el
@@ -590,7 +590,7 @@ image-dired-display-thumbs
 `image-dired-previous-line-and-display' where we do not want the
 thumbnail buffer to be selected."
   (interactive "P" nil dired-mode)
-  (setq image-dired--generate-thumbs-start  (current-time))
+  (setq image-dired--generate-thumbs-start (current-time))
   (let ((buf (image-dired-create-thumbnail-buffer))
         files dired-buf)
     (if arg
-- 
2.40.0

bug#61394: 30.0.50; [PATCH] Image-dired thumb name based on content

[Eli Zaretskii]

> From: Manuel Giraud <manuel@ledu-giraud.fr>
> Cc: stefankangas@gmail.com,  contovob@tcd.ie,  61394@debbugs.gnu.org
> Date: Tue, 01 Aug 2023 19:05:52 +0200
> 
> Manuel Giraud <manuel@ledu-giraud.fr> writes:
> 
> > Hi,
> >
> > FTR, there is a thing that does not work with this "thumb name based on
> > content": upon rotation the thumb is not updated properly.
> >
> > This comes from the fact that the image content has changed (obviously)
> > so the new thumb will have a new name and is not displayed in the
> > *image-dired* buffer 😅.  I'd have to find a way around it.
> 
> Hi,
> 
> So here is a patch that fixes this issue.  Thanks.

Thanks.  Maybe I'm missing something, but isn't the problem limited to
just one method of naming the thumbnail files?  If so, shouldn't these
changes be limited to that method alone?

bug#61394: 30.0.50; [PATCH] Image-dired thumb name based on content

[Manuel Giraud via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Manuel Giraud <manuel@ledu-giraud.fr>
>> Cc: stefankangas@gmail.com,  contovob@tcd.ie,  61394@debbugs.gnu.org
>> Date: Tue, 01 Aug 2023 19:05:52 +0200
>> 
>> Manuel Giraud <manuel@ledu-giraud.fr> writes:
>> 
>> > Hi,
>> >
>> > FTR, there is a thing that does not work with this "thumb name based on
>> > content": upon rotation the thumb is not updated properly.
>> >
>> > This comes from the fact that the image content has changed (obviously)
>> > so the new thumb will have a new name and is not displayed in the
>> > *image-dired* buffer 😅.  I'd have to find a way around it.
>> 
>> Hi,
>> 
>> So here is a patch that fixes this issue.  Thanks.
>
> Thanks.  Maybe I'm missing something, but isn't the problem limited to
> just one method of naming the thumbnail files?  If so, shouldn't these
> changes be limited to that method alone?

Yes.  First I had a test about the method around the call to
'image-dired-update-thumbnail-at-point' but then I realize that nothing
will be done for others methods as I'm doing a test on the thumb name
change:
        ... (unless (string= thumb old-thumb) ...
-- 
Manuel Giraud

bug#61394: 30.0.50; [PATCH] Image-dired thumb name based on content

[Eli Zaretskii]

> From: Manuel Giraud <manuel@ledu-giraud.fr>
> Cc: stefankangas@gmail.com,  contovob@tcd.ie,  61394@debbugs.gnu.org
> Date: Wed, 02 Aug 2023 20:00:19 +0200
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> > Thanks.  Maybe I'm missing something, but isn't the problem limited to
> > just one method of naming the thumbnail files?  If so, shouldn't these
> > changes be limited to that method alone?
> 
> Yes.  First I had a test about the method around the call to
> 'image-dired-update-thumbnail-at-point' but then I realize that nothing
> will be done for others methods as I'm doing a test on the thumb name
> change:
>         ... (unless (string= thumb old-thumb) ...

This at least warrants a comment, so that others won't have to be
bothered by the same questions.

bug#61394: 30.0.50; [PATCH] Image-dired thumb name based on content

[Eli Zaretskii]

> From: Manuel Giraud <manuel@ledu-giraud.fr>
> Cc: stefankangas@gmail.com,  contovob@tcd.ie,  61394@debbugs.gnu.org
> Date: Sat, 29 Jul 2023 18:50:27 +0200
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> >> From: Manuel Giraud <manuel@ledu-giraud.fr>
> >> Cc: stefankangas@gmail.com,  contovob@tcd.ie,  61394@debbugs.gnu.org
> >> Date: Sat, 29 Jul 2023 11:51:19 +0200
> >> 
> >> So I think that this patch does that but then 5efc7b22cec should be
> >> reverted.
> >> 
> >> I'm not so sure about the 'image-dired-thumb-name' docstring.
> >
> > Thanks, will look into that soon.
> 
> Here is a new version of this patch.  I missed some precision in a
> docstring in the previous one (sorry).

Thanks, installed on the master branch.

bug#61394: 30.0.50; [PATCH] Image-dired thumb name based on content

[Manuel Giraud via Bug reports for GNU Emacs, the Swiss army knife of text editors]

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

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Manuel Giraud <manuel@ledu-giraud.fr>
>> Cc: stefankangas@gmail.com,  contovob@tcd.ie,  61394@debbugs.gnu.org
>> Date: Wed, 02 Aug 2023 20:00:19 +0200
>> 
>> Eli Zaretskii <eliz@gnu.org> writes:
>> 
>> > Thanks.  Maybe I'm missing something, but isn't the problem limited to
>> > just one method of naming the thumbnail files?  If so, shouldn't these
>> > changes be limited to that method alone?
>> 
>> Yes.  First I had a test about the method around the call to
>> 'image-dired-update-thumbnail-at-point' but then I realize that nothing
>> will be done for others methods as I'm doing a test on the thumb name
>> change:
>>         ... (unless (string= thumb old-thumb) ...
>
> This at least warrants a comment, so that others won't have to be
> bothered by the same questions.

What about this one?  I've also permuted some instructions to avoid a
flicker when updating the thumbnail.
-- 
Manuel Giraud

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Fix-thumbnail-update-when-thumb-name-is-based-on-ima.patch --]
[-- Type: text/x-patch, Size: 3303 bytes --]

From 4759066712e9cec4906f95a96b3c6b023149eade Mon Sep 17 00:00:00 2001
From: Manuel Giraud <manuel@ledu-giraud.fr>
Date: Tue, 1 Aug 2023 18:56:33 +0200
Subject: [PATCH] Fix thumbnail update when thumb name is based on image
 content

* lisp/image/image-dired-util.el
(image-dired-update-thumbnail-at-point): New function to update
thumbnail when original image contents changed.

* lisp/image/image-dired-external.el
(image-dired-rotate-original): Use it.

* lisp/image/image-dired.el (image-dired-display-thumbs): Fix
spacing while here.
---
 lisp/image/image-dired-external.el |  3 ++-
 lisp/image/image-dired-util.el     | 18 ++++++++++++++++++
 lisp/image/image-dired.el          |  2 +-
 3 files changed, 21 insertions(+), 2 deletions(-)

diff --git a/lisp/image/image-dired-external.el b/lisp/image/image-dired-external.el
index 9f35e17a7e6..77352c25a3b 100644
--- a/lisp/image/image-dired-external.el
+++ b/lisp/image/image-dired-external.el
@@ -405,7 +405,8 @@ image-dired-rotate-original
                 (not image-dired-rotate-original-ask-before-overwrite))
             (progn
               (copy-file image-dired-temp-rotate-image-file file t)
-              (image-dired-refresh-thumb))
+              (image-dired-refresh-thumb)
+              (image-dired-update-thumbnail-at-point))
           (image-dired-display-image file))))))
 
 \f
diff --git a/lisp/image/image-dired-util.el b/lisp/image/image-dired-util.el
index 70911bce45a..40e90e6dcb8 100644
--- a/lisp/image/image-dired-util.el
+++ b/lisp/image/image-dired-util.el
@@ -190,6 +190,24 @@ image-dired-image-at-point-p
   "Return non-nil if there is an `image-dired' thumbnail at point."
   (get-text-property (point) 'image-dired-thumbnail))
 
+(defun image-dired-update-thumbnail-at-point ()
+  "Update the thumbnail at point if the original image file has been
+modified.  Take care of uncaching and removing the old thumbnail
+file."
+  (when (image-dired-image-at-point-p)
+    (let* ((file (image-dired-original-file-name))
+           (thumb (expand-file-name (image-dired-thumb-name file)))
+           (image (get-text-property (point) 'display)))
+      (when image
+        (let ((old-thumb (plist-get (cdr image) :file)))
+          ;; If 'thumb' and 'old-thumb' are the same file names, do
+          ;; nothing.  This would be the case when
+          ;; 'image-dired-thumb-naming' is set to 'sha1-filename'.
+          (unless (string= thumb old-thumb)
+            (setf (plist-get (cdr image) :file) thumb)
+            (clear-image-cache old-thumb)
+            (delete-file old-thumb)))))))
+
 (defun image-dired-window-width-pixels (window)
   "Calculate WINDOW width in pixels."
   (declare (obsolete window-body-width "29.1"))
diff --git a/lisp/image/image-dired.el b/lisp/image/image-dired.el
index 98596510ec1..33beb5b3e49 100644
--- a/lisp/image/image-dired.el
+++ b/lisp/image/image-dired.el
@@ -590,7 +590,7 @@ image-dired-display-thumbs
 `image-dired-previous-line-and-display' where we do not want the
 thumbnail buffer to be selected."
   (interactive "P" nil dired-mode)
-  (setq image-dired--generate-thumbs-start  (current-time))
+  (setq image-dired--generate-thumbs-start (current-time))
   (let ((buf (image-dired-create-thumbnail-buffer))
         files dired-buf)
     (if arg
-- 
2.40.0

bug#61394: 30.0.50; [PATCH] Image-dired thumb name based on content

[Eli Zaretskii]

> From: Manuel Giraud <manuel@ledu-giraud.fr>
> Cc: stefankangas@gmail.com,  contovob@tcd.ie,  61394@debbugs.gnu.org
> Date: Thu, 03 Aug 2023 13:10:43 +0200
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> >> Yes.  First I had a test about the method around the call to
> >> 'image-dired-update-thumbnail-at-point' but then I realize that nothing
> >> will be done for others methods as I'm doing a test on the thumb name
> >> change:
> >>         ... (unless (string= thumb old-thumb) ...
> >
> > This at least warrants a comment, so that others won't have to be
> > bothered by the same questions.
> 
> What about this one?  I've also permuted some instructions to avoid a
> flicker when updating the thumbnail.

Thanks, a few minor comments below:

> +(defun image-dired-update-thumbnail-at-point ()
> +  "Update the thumbnail at point if the original image file has been
> +modified.  Take care of uncaching and removing the old thumbnail
> +file."

The first line of the doc string should be a single complete sentence.
(Just move the "modified" part to the first line, I think there's
enough space there.)

> +  (when (image-dired-image-at-point-p)
> +    (let* ((file (image-dired-original-file-name))
> +           (thumb (expand-file-name (image-dired-thumb-name file)))
> +           (image (get-text-property (point) 'display)))
> +      (when image
> +        (let ((old-thumb (plist-get (cdr image) :file)))
> +          ;; If 'thumb' and 'old-thumb' are the same file names, do
> +          ;; nothing.  This would be the case when
> +          ;; 'image-dired-thumb-naming' is set to 'sha1-filename'.

Isn't that true for _any_ method except the single one for which you
need this patch?  I thought only names based on SHA1 of the contents
are affected and need this additional renaming?  The comment seems to
say something else.

bug#61394: 30.0.50; [PATCH] Image-dired thumb name based on content

[Manuel Giraud via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Eli Zaretskii <eliz@gnu.org> writes:

[...]

>> +(defun image-dired-update-thumbnail-at-point ()
>> +  "Update the thumbnail at point if the original image file has been
>> +modified.  Take care of uncaching and removing the old thumbnail
>> +file."
>
> The first line of the doc string should be a single complete sentence.
> (Just move the "modified" part to the first line, I think there's
> enough space there.)

I don't understand.  "Update the thumbnail at point." does not seem
sufficient because the fact that the original image might be modified is
the main point of this function.

>> +  (when (image-dired-image-at-point-p)
>> +    (let* ((file (image-dired-original-file-name))
>> +           (thumb (expand-file-name (image-dired-thumb-name file)))
>> +           (image (get-text-property (point) 'display)))
>> +      (when image
>> +        (let ((old-thumb (plist-get (cdr image) :file)))
>> +          ;; If 'thumb' and 'old-thumb' are the same file names, do
>> +          ;; nothing.  This would be the case when
>> +          ;; 'image-dired-thumb-naming' is set to 'sha1-filename'.
>
> Isn't that true for _any_ method except the single one for which you
> need this patch?  I thought only names based on SHA1 of the contents
> are affected and need this additional renaming?  The comment seems to
> say something else.

What about:

"When 'image-dired-thumb-naming' is set to 'sha1-contents', 'thumb' and
'old-thumb' could be different file names.  Update the thumbnail then."
-- 
Manuel Giraud

bug#61394: 30.0.50; [PATCH] Image-dired thumb name based on content

[Eli Zaretskii]

> From: Manuel Giraud <manuel@ledu-giraud.fr>
> Cc: stefankangas@gmail.com,  contovob@tcd.ie,  61394@debbugs.gnu.org
> Date: Thu, 03 Aug 2023 18:51:28 +0200
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> >> +(defun image-dired-update-thumbnail-at-point ()
> >> +  "Update the thumbnail at point if the original image file has been
> >> +modified.  Take care of uncaching and removing the old thumbnail
> >> +file."
> >
> > The first line of the doc string should be a single complete sentence.
> > (Just move the "modified" part to the first line, I think there's
> > enough space there.)
> 
> I don't understand.  "Update the thumbnail at point." does not seem
> sufficient because the fact that the original image might be modified is
> the main point of this function.

What I had in mind is this:

 (defun image-dired-update-thumbnail-at-point ()
   "Update the thumbnail at point if the original image file has been modified.
 This function uncaches and removes the thumbnail file under the old name."

OK?

> What about:
> 
> "When 'image-dired-thumb-naming' is set to 'sha1-contents', 'thumb' and
> 'old-thumb' could be different file names.  Update the thumbnail then."

Much better, thanks.

bug#61394: 30.0.50; [PATCH] Image-dired thumb name based on content

[Manuel Giraud via Bug reports for GNU Emacs, the Swiss army knife of text editors]

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

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Manuel Giraud <manuel@ledu-giraud.fr>
>> Cc: stefankangas@gmail.com,  contovob@tcd.ie,  61394@debbugs.gnu.org
>> Date: Thu, 03 Aug 2023 18:51:28 +0200
>> 
>> Eli Zaretskii <eliz@gnu.org> writes:
>> 
>> >> +(defun image-dired-update-thumbnail-at-point ()
>> >> +  "Update the thumbnail at point if the original image file has been
>> >> +modified.  Take care of uncaching and removing the old thumbnail
>> >> +file."
>> >
>> > The first line of the doc string should be a single complete sentence.
>> > (Just move the "modified" part to the first line, I think there's
>> > enough space there.)
>> 
>> I don't understand.  "Update the thumbnail at point." does not seem
>> sufficient because the fact that the original image might be modified is
>> the main point of this function.
>
> What I had in mind is this:
>
>  (defun image-dired-update-thumbnail-at-point ()
>    "Update the thumbnail at point if the original image file has been modified.
>  This function uncaches and removes the thumbnail file under the old name."
>
> OK?

Ok.  Sorry I misread your reply ("move" not "remove").  BTW, I'm using
'M-q' on docstring and my fill-column is set to 70 (the default I
guess): is there a better way?

>> What about:
>> 
>> "When 'image-dired-thumb-naming' is set to 'sha1-contents', 'thumb' and
>> 'old-thumb' could be different file names.  Update the thumbnail then."
>
> Much better, thanks.

Ok, so here is a new patch.

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Fix-thumbnail-update-when-thumb-name-is-based-on-ima.patch --]
[-- Type: text/x-patch, Size: 3309 bytes --]

From b67f344f148dbabf8f2c0099aa9002af4570c5dd Mon Sep 17 00:00:00 2001
From: Manuel Giraud <manuel@ledu-giraud.fr>
Date: Tue, 1 Aug 2023 18:56:33 +0200
Subject: [PATCH] Fix thumbnail update when thumb name is based on image
 content

* lisp/image/image-dired-util.el
(image-dired-update-thumbnail-at-point): New function to update
thumbnail when original image contents changed.

* lisp/image/image-dired-external.el
(image-dired-rotate-original): Use it.

* lisp/image/image-dired.el (image-dired-display-thumbs): Fix
spacing while here.
---
 lisp/image/image-dired-external.el |  3 ++-
 lisp/image/image-dired-util.el     | 17 +++++++++++++++++
 lisp/image/image-dired.el          |  2 +-
 3 files changed, 20 insertions(+), 2 deletions(-)

diff --git a/lisp/image/image-dired-external.el b/lisp/image/image-dired-external.el
index 9f35e17a7e6..77352c25a3b 100644
--- a/lisp/image/image-dired-external.el
+++ b/lisp/image/image-dired-external.el
@@ -405,7 +405,8 @@ image-dired-rotate-original
                 (not image-dired-rotate-original-ask-before-overwrite))
             (progn
               (copy-file image-dired-temp-rotate-image-file file t)
-              (image-dired-refresh-thumb))
+              (image-dired-refresh-thumb)
+              (image-dired-update-thumbnail-at-point))
           (image-dired-display-image file))))))
 
 \f
diff --git a/lisp/image/image-dired-util.el b/lisp/image/image-dired-util.el
index 70911bce45a..53a5e274175 100644
--- a/lisp/image/image-dired-util.el
+++ b/lisp/image/image-dired-util.el
@@ -190,6 +190,23 @@ image-dired-image-at-point-p
   "Return non-nil if there is an `image-dired' thumbnail at point."
   (get-text-property (point) 'image-dired-thumbnail))
 
+(defun image-dired-update-thumbnail-at-point ()
+  "Update the thumbnail at point if the original image file has been modified.
+This function uncaches and removes the thumbnail file under the old name."
+  (when (image-dired-image-at-point-p)
+    (let* ((file (image-dired-original-file-name))
+           (thumb (expand-file-name (image-dired-thumb-name file)))
+           (image (get-text-property (point) 'display)))
+      (when image
+        (let ((old-thumb (plist-get (cdr image) :file)))
+          ;; When 'image-dired-thumb-naming' is set to
+          ;; 'sha1-contents', 'thumb' and 'old-thumb' could be
+          ;; different file names.  Update the thumbnail then.
+          (unless (string= thumb old-thumb)
+            (setf (plist-get (cdr image) :file) thumb)
+            (clear-image-cache old-thumb)
+            (delete-file old-thumb)))))))
+
 (defun image-dired-window-width-pixels (window)
   "Calculate WINDOW width in pixels."
   (declare (obsolete window-body-width "29.1"))
diff --git a/lisp/image/image-dired.el b/lisp/image/image-dired.el
index 98596510ec1..33beb5b3e49 100644
--- a/lisp/image/image-dired.el
+++ b/lisp/image/image-dired.el
@@ -590,7 +590,7 @@ image-dired-display-thumbs
 `image-dired-previous-line-and-display' where we do not want the
 thumbnail buffer to be selected."
   (interactive "P" nil dired-mode)
-  (setq image-dired--generate-thumbs-start  (current-time))
+  (setq image-dired--generate-thumbs-start (current-time))
   (let ((buf (image-dired-create-thumbnail-buffer))
         files dired-buf)
     (if arg
-- 
2.40.0


[-- Attachment #3: Type: text/plain, Size: 18 bytes --]

-- 
Manuel Giraud

bug#61394: 30.0.50; [PATCH] Image-dired thumb name based on content

[Eli Zaretskii]

> From: Manuel Giraud <manuel@ledu-giraud.fr>
> Cc: stefankangas@gmail.com,  contovob@tcd.ie,  61394@debbugs.gnu.org
> Date: Fri, 04 Aug 2023 09:44:41 +0200
> 
> > What I had in mind is this:
> >
> >  (defun image-dired-update-thumbnail-at-point ()
> >    "Update the thumbnail at point if the original image file has been modified.
> >  This function uncaches and removes the thumbnail file under the old name."
> >
> > OK?
> 
> Ok.  Sorry I misread your reply ("move" not "remove").  BTW, I'm using
> 'M-q' on docstring and my fill-column is set to 70 (the default I
> guess): is there a better way?

70 is okay as the default value, but when you cannot squeeze the first
line into 70 columns, it is okay to use up to 79.

> >> What about:
> >> 
> >> "When 'image-dired-thumb-naming' is set to 'sha1-contents', 'thumb' and
> >> 'old-thumb' could be different file names.  Update the thumbnail then."
> >
> > Much better, thanks.
> 
> Ok, so here is a new patch.

Thanks, installed.

Should this bug now be closed, or is there something else to do here?

bug#61394: 30.0.50; [PATCH] Image-dired thumb name based on content

[Manuel Giraud via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Eli Zaretskii <eliz@gnu.org> writes:

[...]

> Thanks, installed.
>
> Should this bug now be closed, or is there something else to do here?

Thanks.  I think it can be closed this feature is complete AFAICT.
-- 
Manuel Giraud

bug#61394: 30.0.50; [PATCH] Image-dired thumb name based on content

[Eli Zaretskii]

> From: Manuel Giraud <manuel@ledu-giraud.fr>
> Cc: stefankangas@gmail.com,  contovob@tcd.ie,  61394@debbugs.gnu.org
> Date: Fri, 04 Aug 2023 15:37:29 +0200
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> [...]
> 
> > Should this bug now be closed, or is there something else to do here?
> 
> Thanks.  I think it can be closed this feature is complete AFAICT.

Thanks, done.