all messages for Emacs-related lists mirrored at yhetil.org
 help / color / mirror / code / Atom feed
* bug#61639: 30.0.50; [PATCH] Do not error out on non image file in image-dired
@ 2023-02-19 20:09 Manuel Giraud via Bug reports for GNU Emacs, the Swiss army knife of text editors
  2023-02-20 12:49 ` Eli Zaretskii
  0 siblings, 1 reply; 12+ messages in thread
From: Manuel Giraud via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2023-02-19 20:09 UTC (permalink / raw)
  To: 61639

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


Hi,

This patch prevent errors when using image-dired on non image files.

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Do-not-error-out-on-non-image-file.patch --]
[-- Type: text/x-patch, Size: 3260 bytes --]

From 72f4dcd72d20fc2c0ead21300c0e2bff1795e8ee Mon Sep 17 00:00:00 2001
From: Manuel Giraud <manuel@ledu-giraud.fr>
Date: Sun, 19 Feb 2023 21:03:57 +0100
Subject: [PATCH] Do not error out on non image file

* lisp/image/image-dired.el
(image-dired--get-create-thumbnail-file): Do not error out but return
NIL for a non image file.
(image-dired-display-thumbs): Do not insert non image file and do
not display image-dired buffer if it is empty.
---
 lisp/image/image-dired.el | 37 ++++++++++++++++++++-----------------
 1 file changed, 20 insertions(+), 17 deletions(-)

diff --git a/lisp/image/image-dired.el b/lisp/image/image-dired.el
index cfcd1851188..60f2abd982a 100644
--- a/lisp/image/image-dired.el
+++ b/lisp/image/image-dired.el
@@ -411,19 +411,20 @@ image-dired-insert-image
 
 (defun image-dired--get-create-thumbnail-file (file)
   "Return the image descriptor for a thumbnail of image file FILE."
-  (unless (string-match-p (image-dired--file-name-regexp) file)
-    (error "%s is not a valid image file" file))
-  (let* ((thumb-file (image-dired-thumb-name file))
-         (thumb-attr (file-attributes thumb-file)))
-    (if (or (not thumb-attr)
-            (time-less-p (file-attribute-modification-time thumb-attr)
-                         (file-attribute-modification-time
-                          (file-attributes file))))
-        (image-dired-create-thumb file thumb-file)
-      (image-dired-debug "Found thumb for %s: %s"
-                         (file-name-nondirectory file)
-                         (file-name-nondirectory thumb-file)))
-    thumb-file))
+  (if (string-match-p (image-dired--file-name-regexp) file)
+      (let* ((thumb-file (image-dired-thumb-name file))
+             (thumb-attr (file-attributes thumb-file)))
+        (if (or (not thumb-attr)
+                (time-less-p (file-attribute-modification-time thumb-attr)
+                             (file-attribute-modification-time
+                              (file-attributes file))))
+            (image-dired-create-thumb file thumb-file)
+          (image-dired-debug "Found thumb for %s: %s"
+                             (file-name-nondirectory file)
+                             (file-name-nondirectory thumb-file)))
+        thumb-file)
+    (message "%s is not a valid image file" file)
+    (values)))
 
 (defun image-dired-insert-thumbnail ( file original-file-name
                            associated-dired-buffer image-number)
@@ -586,13 +587,15 @@ image-dired-display-thumbs
               (erase-buffer))
           (goto-char (point-max)))
         (dolist (file files)
-          (let ((thumb (image-dired--get-create-thumbnail-file file)))
+          (when-let ((thumb (image-dired--get-create-thumbnail-file file)))
             (image-dired-insert-thumbnail
              thumb file dired-buf
              (cl-incf image-dired--number-of-thumbnails)))))
-      (if do-not-pop
-          (display-buffer buf)
-        (pop-to-buffer buf))
+      (if (plusp image-dired--number-of-thumbnails)
+          (if do-not-pop
+              (display-buffer buf)
+            (pop-to-buffer buf))
+        (message "No images selected"))
       (image-dired--line-up-with-method)
       (image-dired--update-header-line))))
 
-- 
2.39.1


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



In GNU Emacs 30.0.50 (build 1, x86_64-unknown-openbsd7.2, cairo version
 1.17.8) of 2023-02-19 built on computer
Repository revision: ff2109d7202ad2bbad00edd2a2ab21e4917ca7a3
Repository branch: mgi/image-dired-doc
Windowing system distributor 'The X.Org Foundation', version 11.0.12101006
System Description: OpenBSD computer 7.2 GENERIC.MP#1052 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:
(find-dired ffap net-utils cl-print image-file image-converter
image-dired-dired image-dired image-dired-tags image-dired-external
image-dired-util dabbrev misearch multi-isearch shortdoc help-fns
radix-tree shr-color flow-fill gnus-cite gnus-async gnus-bcklg gnus-ml
gnus-topic mm-archive url-http url-gw url-cache url-auth qp utf-7 imap
rfc2104 nndoc nndraft nnmh nnfolder nnml gnus-agent gnus-srvr gnus-score
score-mode nnvirtual nntp gnus-cache nnrss w3m w3m-hist w3m-fb
bookmark-w3m w3m-ems w3m-favicon w3m-image tab-line w3m-proc w3m-util
network-stream nsm smtpmail textsec uni-scripts idna-mapping
ucs-normalize uni-confusable textsec-check mailalias shadow sort
mail-extr emacsbug whitespace magit-extras magit-patch pulse 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
add-log magit-core magit-autorevert magit-margin magit-transient
magit-process with-editor magit-mode transient magit-git magit-section
magit-utils dash org-indent vc-dir ewoc vc mhtml-mode js c-ts-common
reveal pascal vc-hg conf-mode css-mode sgml-mode facemenu imenu
sh-script smie treesit executable oc-basic ol-eww eww url-queue mm-url
ol-rmail ol-mhe ol-irc ol-info ol-gnus nnselect ol-docview doc-view
jka-compr image-mode exif ol-bibtex bibtex ol-bbdb ol-w3m ol-doi
org-link-doi paredit edmacro gnus-dired autorevert filenotify vc-git
diff-mode bug-reference texinfo texinfo-loaddefs time battery exwm-randr
xcb-randr exwm-config ido 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 bookmark mingus libmpdee reporter edebug debug
backtrace transmission color calc-bin calc-ext calc calc-loaddefs rect
calc-macs w3m-load 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 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 tramp-loaddefs trampver
tramp-integration cus-edit cus-load wid-edit files-x tramp-compat shell
parse-time iso8601 ls-lisp 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 xref project arc-mode archive-mode pp hyperspec thingatpt
slime-autoloads org-agenda org-element org-persist xdg org-id avl-tree
generator org-refile org ob ob-tangle ob-ref ob-lob ob-table ob-exp
org-macro org-src ob-comint org-pcomplete pcomplete comint ansi-osc
ansi-color ring org-list org-footnote org-faces org-entities time-date
noutline outline icons ob-emacs-lisp ob-core ob-eval org-cycle org-table
org-keys oc org-loaddefs find-func ol rx org-fold org-fold-core
org-compat org-version org-macs format-spec view mule-util cal-china
lunar solar cal-dst cal-bahai cal-islam cal-hebrew holidays
holiday-loaddefs vc-dispatcher vc-svn appt diary-lib diary-loaddefs
cal-menu calendar cal-loaddefs 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 1065525 388148)
 (symbols 48 64082 42)
 (strings 32 316403 24225)
 (string-bytes 1 9903231)
 (vectors 16 191008)
 (vector-slots 8 3234332 186563)
 (floats 8 1224 465)
 (intervals 56 21213 5652)
 (buffers 984 129))

-- 
Manuel Giraud

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* bug#61639: 30.0.50; [PATCH] Do not error out on non image file in image-dired
  2023-02-19 20:09 bug#61639: 30.0.50; [PATCH] Do not error out on non image file in image-dired Manuel Giraud via Bug reports for GNU Emacs, the Swiss army knife of text editors
@ 2023-02-20 12:49 ` Eli Zaretskii
  2023-02-20 13:35   ` Manuel Giraud via Bug reports for GNU Emacs, the Swiss army knife of text editors
  0 siblings, 1 reply; 12+ messages in thread
From: Eli Zaretskii @ 2023-02-20 12:49 UTC (permalink / raw)
  To: Manuel Giraud; +Cc: 61639

> Date: Sun, 19 Feb 2023 21:09:47 +0100
> From:  Manuel Giraud via "Bug reports for GNU Emacs,
>  the Swiss army knife of text editors" <bug-gnu-emacs@gnu.org>
> 
> This patch prevent errors when using image-dired on non image files.

Why is it better to show a message than to signal an error?  The
former could go unnoticed, especially if some other message is shown
in the echo-area soon enough.





^ permalink raw reply	[flat|nested] 12+ messages in thread

* bug#61639: 30.0.50; [PATCH] Do not error out on non image file in image-dired
  2023-02-20 12:49 ` Eli Zaretskii
@ 2023-02-20 13:35   ` Manuel Giraud via Bug reports for GNU Emacs, the Swiss army knife of text editors
  2023-02-20 13:46     ` Eli Zaretskii
  0 siblings, 1 reply; 12+ messages in thread
From: Manuel Giraud via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2023-02-20 13:35 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 61639

Eli Zaretskii <eliz@gnu.org> writes:

>> Date: Sun, 19 Feb 2023 21:09:47 +0100
>> From:  Manuel Giraud via "Bug reports for GNU Emacs,
>>  the Swiss army knife of text editors" <bug-gnu-emacs@gnu.org>
>> 
>> This patch prevent errors when using image-dired on non image files.
>
> Why is it better to show a message than to signal an error?  The
> former could go unnoticed, especially if some other message is shown
> in the echo-area soon enough.

I don't think it is better.  But for some image-dired usage, I do not
find it convenient.  Example: you carefully select some images from a
dired buffer and hit `C-t d' to see them in image-dired.  But your
selection was not correct and one of those file is not an image: you
receive an error (and have to correct your selection) and don't get to
see any of the correctly selected images.

So my idea was: yes, it is an error but it should not be one that
unwinds all.  Maybe it is a job for `warn' but having a *Warnings*
buffer that pop up is a bit too much too IMO.
-- 
Manuel Giraud





^ permalink raw reply	[flat|nested] 12+ messages in thread

* bug#61639: 30.0.50; [PATCH] Do not error out on non image file in image-dired
  2023-02-20 13:35   ` Manuel Giraud via Bug reports for GNU Emacs, the Swiss army knife of text editors
@ 2023-02-20 13:46     ` Eli Zaretskii
  2023-02-20 14:13       ` Manuel Giraud via Bug reports for GNU Emacs, the Swiss army knife of text editors
  0 siblings, 1 reply; 12+ messages in thread
From: Eli Zaretskii @ 2023-02-20 13:46 UTC (permalink / raw)
  To: Manuel Giraud; +Cc: 61639

> From: Manuel Giraud <manuel@ledu-giraud.fr>
> Cc: 61639@debbugs.gnu.org
> Date: Mon, 20 Feb 2023 14:35:42 +0100
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> >> Date: Sun, 19 Feb 2023 21:09:47 +0100
> >> From:  Manuel Giraud via "Bug reports for GNU Emacs,
> >>  the Swiss army knife of text editors" <bug-gnu-emacs@gnu.org>
> >> 
> >> This patch prevent errors when using image-dired on non image files.
> >
> > Why is it better to show a message than to signal an error?  The
> > former could go unnoticed, especially if some other message is shown
> > in the echo-area soon enough.
> 
> I don't think it is better.  But for some image-dired usage, I do not
> find it convenient.  Example: you carefully select some images from a
> dired buffer and hit `C-t d' to see them in image-dired.  But your
> selection was not correct and one of those file is not an image: you
> receive an error (and have to correct your selection) and don't get to
> see any of the correctly selected images.

So maybe "C-t d" should filter the selected "images" before it calls
the function which errors out?

IOW, if the application doesn't want an API to fail for reasons
specific to the application, the onus of avoiding the error is on the
application, no?





^ permalink raw reply	[flat|nested] 12+ messages in thread

* bug#61639: 30.0.50; [PATCH] Do not error out on non image file in image-dired
  2023-02-20 13:46     ` Eli Zaretskii
@ 2023-02-20 14:13       ` Manuel Giraud via Bug reports for GNU Emacs, the Swiss army knife of text editors
  2023-02-20 14:45         ` Eli Zaretskii
  0 siblings, 1 reply; 12+ messages in thread
From: Manuel Giraud via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2023-02-20 14:13 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 61639

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Manuel Giraud <manuel@ledu-giraud.fr>
>> Cc: 61639@debbugs.gnu.org
>> Date: Mon, 20 Feb 2023 14:35:42 +0100
>> 
>> Eli Zaretskii <eliz@gnu.org> writes:
>> 
>> >> Date: Sun, 19 Feb 2023 21:09:47 +0100
>> >> From:  Manuel Giraud via "Bug reports for GNU Emacs,
>> >>  the Swiss army knife of text editors" <bug-gnu-emacs@gnu.org>
>> >> 
>> >> This patch prevent errors when using image-dired on non image files.
>> >
>> > Why is it better to show a message than to signal an error?  The
>> > former could go unnoticed, especially if some other message is shown
>> > in the echo-area soon enough.
>> 
>> I don't think it is better.  But for some image-dired usage, I do not
>> find it convenient.  Example: you carefully select some images from a
>> dired buffer and hit `C-t d' to see them in image-dired.  But your
>> selection was not correct and one of those file is not an image: you
>> receive an error (and have to correct your selection) and don't get to
>> see any of the correctly selected images.
>
> So maybe "C-t d" should filter the selected "images" before it calls
> the function which errors out?

That is what `image-dired-show-all-from-dir' is doing: it selects files
from the given directory with the correct "image files" regexp and so no
non image will be present.

So your idea is to keep `image-dired--get-create-thumbnail-file' as is
and filter its input in `image-dired-display-thumbs'?  But then we won't
get any message or error that something was not an image (this could go
unnoticed as well ;-)

> IOW, if the application doesn't want an API to fail for reasons
> specific to the application, the onus of avoiding the error is on the
> application, no?

Yes, but maybe `image-dired--get-create-thumbnail-file' is not really an
established API.  It is called twice in Emacs (and the other place it is
called non images are already filtered out).

Do you think it could have been used in third-party package?
-- 
Manuel Giraud





^ permalink raw reply	[flat|nested] 12+ messages in thread

* bug#61639: 30.0.50; [PATCH] Do not error out on non image file in image-dired
  2023-02-20 14:13       ` Manuel Giraud via Bug reports for GNU Emacs, the Swiss army knife of text editors
@ 2023-02-20 14:45         ` Eli Zaretskii
  2023-02-20 16:41           ` Manuel Giraud via Bug reports for GNU Emacs, the Swiss army knife of text editors
  0 siblings, 1 reply; 12+ messages in thread
From: Eli Zaretskii @ 2023-02-20 14:45 UTC (permalink / raw)
  To: Manuel Giraud; +Cc: 61639

> From: Manuel Giraud <manuel@ledu-giraud.fr>
> Cc: 61639@debbugs.gnu.org
> Date: Mon, 20 Feb 2023 15:13:37 +0100
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> > So maybe "C-t d" should filter the selected "images" before it calls
> > the function which errors out?
> 
> That is what `image-dired-show-all-from-dir' is doing: it selects files
> from the given directory with the correct "image files" regexp and so no
> non image will be present.
> 
> So your idea is to keep `image-dired--get-create-thumbnail-file' as is
> and filter its input in `image-dired-display-thumbs'?  But then we won't
> get any message or error that something was not an image (this could go
> unnoticed as well ;-)

I don't necessarily see a reason to alert the user in this case, but
if you think we had better done that, we could show a message when we
find a non-image file in the list of the selected ones.

> > IOW, if the application doesn't want an API to fail for reasons
> > specific to the application, the onus of avoiding the error is on the
> > application, no?
> 
> Yes, but maybe `image-dired--get-create-thumbnail-file' is not really an
> established API.  It is called twice in Emacs (and the other place it is
> called non images are already filtered out).
> 
> Do you think it could have been used in third-party package?

That's not what bothers me.  What bothers me is that we burden a
low-level API with considerations whose source is the application.





^ permalink raw reply	[flat|nested] 12+ messages in thread

* bug#61639: 30.0.50; [PATCH] Do not error out on non image file in image-dired
  2023-02-20 14:45         ` Eli Zaretskii
@ 2023-02-20 16:41           ` Manuel Giraud via Bug reports for GNU Emacs, the Swiss army knife of text editors
  2023-02-21 12:01             ` Eli Zaretskii
  0 siblings, 1 reply; 12+ messages in thread
From: Manuel Giraud via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2023-02-20 16:41 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 61639

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

Eli Zaretskii <eliz@gnu.org> writes:

[...]

>> So your idea is to keep `image-dired--get-create-thumbnail-file' as is
>> and filter its input in `image-dired-display-thumbs'?  But then we won't
>> get any message or error that something was not an image (this could go
>> unnoticed as well ;-)
>
> I don't necessarily see a reason to alert the user in this case, but
> if you think we had better done that, we could show a message when we
> find a non-image file in the list of the selected ones.

Ok.  Here is a new version of the patch that address those issues.  I
choose to not alert the user.

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Do-not-error-out-on-non-image-file-bug-61639.patch --]
[-- Type: text/x-patch, Size: 1505 bytes --]

From ddbeb1171c02f3865359edd9b2b435e329d35f7c Mon Sep 17 00:00:00 2001
From: Manuel Giraud <manuel@ledu-giraud.fr>
Date: Sun, 19 Feb 2023 21:03:57 +0100
Subject: [PATCH] Do not error out on non image file (bug#61639)

* lisp/image/image-dired.el
(image-dired-display-thumbs): Do not insert non image file and do not
display image-dired buffer if it is empty.
---
 lisp/image/image-dired.el | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/lisp/image/image-dired.el b/lisp/image/image-dired.el
index cfcd1851188..5798af77f35 100644
--- a/lisp/image/image-dired.el
+++ b/lisp/image/image-dired.el
@@ -586,13 +586,15 @@ image-dired-display-thumbs
               (erase-buffer))
           (goto-char (point-max)))
         (dolist (file files)
-          (let ((thumb (image-dired--get-create-thumbnail-file file)))
+          (when (string-match-p (image-dired--file-name-regexp) file)
             (image-dired-insert-thumbnail
-             thumb file dired-buf
+             (image-dired--get-create-thumbnail-file file) file dired-buf
              (cl-incf image-dired--number-of-thumbnails)))))
-      (if do-not-pop
-          (display-buffer buf)
-        (pop-to-buffer buf))
+      (if (plusp image-dired--number-of-thumbnails)
+          (if do-not-pop
+              (display-buffer buf)
+            (pop-to-buffer buf))
+        (message "No images selected"))
       (image-dired--line-up-with-method)
       (image-dired--update-header-line))))
 
-- 
2.39.1


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

-- 
Manuel Giraud

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* bug#61639: 30.0.50; [PATCH] Do not error out on non image file in image-dired
  2023-02-20 16:41           ` Manuel Giraud via Bug reports for GNU Emacs, the Swiss army knife of text editors
@ 2023-02-21 12:01             ` Eli Zaretskii
  2023-02-21 12:16               ` Manuel Giraud via Bug reports for GNU Emacs, the Swiss army knife of text editors
  0 siblings, 1 reply; 12+ messages in thread
From: Eli Zaretskii @ 2023-02-21 12:01 UTC (permalink / raw)
  To: Manuel Giraud; +Cc: 61639

> From: Manuel Giraud <manuel@ledu-giraud.fr>
> Cc: 61639@debbugs.gnu.org
> Date: Mon, 20 Feb 2023 17:41:35 +0100
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> > I don't necessarily see a reason to alert the user in this case, but
> > if you think we had better done that, we could show a message when we
> > find a non-image file in the list of the selected ones.
> 
> Ok.  Here is a new version of the patch that address those issues.  I
> choose to not alert the user.

Thanks.  This LGTM, but what is 'plusp'?

> diff --git a/lisp/image/image-dired.el b/lisp/image/image-dired.el
> index cfcd1851188..5798af77f35 100644
> --- a/lisp/image/image-dired.el
> +++ b/lisp/image/image-dired.el
> @@ -586,13 +586,15 @@ image-dired-display-thumbs
>                (erase-buffer))
>            (goto-char (point-max)))
>          (dolist (file files)
> -          (let ((thumb (image-dired--get-create-thumbnail-file file)))
> +          (when (string-match-p (image-dired--file-name-regexp) file)
>              (image-dired-insert-thumbnail
> -             thumb file dired-buf
> +             (image-dired--get-create-thumbnail-file file) file dired-buf
>               (cl-incf image-dired--number-of-thumbnails)))))
> -      (if do-not-pop
> -          (display-buffer buf)
> -        (pop-to-buffer buf))
> +      (if (plusp image-dired--number-of-thumbnails)  <<<<<<<<<<<<<<<<<<<<<<
> +          (if do-not-pop
> +              (display-buffer buf)
> +            (pop-to-buffer buf))
> +        (message "No images selected"))
>        (image-dired--line-up-with-method)
>        (image-dired--update-header-line))))





^ permalink raw reply	[flat|nested] 12+ messages in thread

* bug#61639: 30.0.50; [PATCH] Do not error out on non image file in image-dired
  2023-02-21 12:01             ` Eli Zaretskii
@ 2023-02-21 12:16               ` Manuel Giraud via Bug reports for GNU Emacs, the Swiss army knife of text editors
  2023-02-21 13:03                 ` Eli Zaretskii
  0 siblings, 1 reply; 12+ messages in thread
From: Manuel Giraud via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2023-02-21 12:16 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 61639

Eli Zaretskii <eliz@gnu.org> writes:

[...]

> Thanks.  This LGTM, but what is 'plusp'?

'plusp' is a classic Common Lisp predicate (that also seems to be
defined in Emacs).  It is equivalent to (> x 0)… maybe you prefer that.
Should I fix my patch?
-- 
Manuel Giraud





^ permalink raw reply	[flat|nested] 12+ messages in thread

* bug#61639: 30.0.50; [PATCH] Do not error out on non image file in image-dired
  2023-02-21 12:16               ` Manuel Giraud via Bug reports for GNU Emacs, the Swiss army knife of text editors
@ 2023-02-21 13:03                 ` Eli Zaretskii
  2023-02-21 14:23                   ` Manuel Giraud via Bug reports for GNU Emacs, the Swiss army knife of text editors
  0 siblings, 1 reply; 12+ messages in thread
From: Eli Zaretskii @ 2023-02-21 13:03 UTC (permalink / raw)
  To: Manuel Giraud; +Cc: 61639

> From: Manuel Giraud <manuel@ledu-giraud.fr>
> Cc: 61639@debbugs.gnu.org
> Date: Tue, 21 Feb 2023 13:16:04 +0100
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> [...]
> 
> > Thanks.  This LGTM, but what is 'plusp'?
> 
> 'plusp' is a classic Common Lisp predicate (that also seems to be
> defined in Emacs).  It is equivalent to (> x 0)… maybe you prefer that.
> Should I fix my patch?

Where is it defined?  It isn't defined in "emacs -Q", and not after
loading image-dired.  (Tested in Emacs 29.)





^ permalink raw reply	[flat|nested] 12+ messages in thread

* bug#61639: 30.0.50; [PATCH] Do not error out on non image file in image-dired
  2023-02-21 13:03                 ` Eli Zaretskii
@ 2023-02-21 14:23                   ` Manuel Giraud via Bug reports for GNU Emacs, the Swiss army knife of text editors
  2023-02-22 13:22                     ` Eli Zaretskii
  0 siblings, 1 reply; 12+ messages in thread
From: Manuel Giraud via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2023-02-21 14:23 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 61639

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

Eli Zaretskii <eliz@gnu.org> writes:

[...]

>> 'plusp' is a classic Common Lisp predicate (that also seems to be
>> defined in Emacs).  It is equivalent to (> x 0)… maybe you prefer that.
>> Should I fix my patch?
>
> Where is it defined?  It isn't defined in "emacs -Q", and not after
> loading image-dired.  (Tested in Emacs 29.)

C-h f gives me this:
--8<---------------cut here---------------start------------->8---
plusp is an alias for ‘cl-plusp’ in ‘cl.el’.

This function is obsolete since 27.1; use ‘cl-plusp’ instead.
--8<---------------cut here---------------end--------------->8---

… anyway here is an updated version of the patch to avoid this.

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Do-not-error-out-on-non-image-file-bug-61639.patch --]
[-- Type: text/x-patch, Size: 1503 bytes --]

From e9b6f649a97800fcc045b52a63556d25ca4b472c Mon Sep 17 00:00:00 2001
From: Manuel Giraud <manuel@ledu-giraud.fr>
Date: Sun, 19 Feb 2023 21:03:57 +0100
Subject: [PATCH] Do not error out on non image file (bug#61639)

* lisp/image/image-dired.el
(image-dired-display-thumbs): Do not insert non image file and do not
display image-dired buffer if it is empty.
---
 lisp/image/image-dired.el | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/lisp/image/image-dired.el b/lisp/image/image-dired.el
index cfcd1851188..20697b3c9e0 100644
--- a/lisp/image/image-dired.el
+++ b/lisp/image/image-dired.el
@@ -586,13 +586,15 @@ image-dired-display-thumbs
               (erase-buffer))
           (goto-char (point-max)))
         (dolist (file files)
-          (let ((thumb (image-dired--get-create-thumbnail-file file)))
+          (when (string-match-p (image-dired--file-name-regexp) file)
             (image-dired-insert-thumbnail
-             thumb file dired-buf
+             (image-dired--get-create-thumbnail-file file) file dired-buf
              (cl-incf image-dired--number-of-thumbnails)))))
-      (if do-not-pop
-          (display-buffer buf)
-        (pop-to-buffer buf))
+      (if (> image-dired--number-of-thumbnails 0)
+          (if do-not-pop
+              (display-buffer buf)
+            (pop-to-buffer buf))
+        (message "No images selected"))
       (image-dired--line-up-with-method)
       (image-dired--update-header-line))))
 
-- 
2.39.1


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

-- 
Manuel Giraud

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* bug#61639: 30.0.50; [PATCH] Do not error out on non image file in image-dired
  2023-02-21 14:23                   ` Manuel Giraud via Bug reports for GNU Emacs, the Swiss army knife of text editors
@ 2023-02-22 13:22                     ` Eli Zaretskii
  0 siblings, 0 replies; 12+ messages in thread
From: Eli Zaretskii @ 2023-02-22 13:22 UTC (permalink / raw)
  To: Manuel Giraud; +Cc: 61639-done

> From: Manuel Giraud <manuel@ledu-giraud.fr>
> Cc: 61639@debbugs.gnu.org
> Date: Tue, 21 Feb 2023 15:23:01 +0100
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> [...]
> 
> >> 'plusp' is a classic Common Lisp predicate (that also seems to be
> >> defined in Emacs).  It is equivalent to (> x 0)… maybe you prefer that.
> >> Should I fix my patch?
> >
> > Where is it defined?  It isn't defined in "emacs -Q", and not after
> > loading image-dired.  (Tested in Emacs 29.)
> 
> C-h f gives me this:
> --8<---------------cut here---------------start------------->8---
> plusp is an alias for ‘cl-plusp’ in ‘cl.el’.
> 
> This function is obsolete since 27.1; use ‘cl-plusp’ instead.
> --8<---------------cut here---------------end--------------->8---
> 
> … anyway here is an updated version of the patch to avoid this.

Thanks, I installed this on the emacs-29 branch, and closing the bug.





^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2023-02-22 13:22 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-02-19 20:09 bug#61639: 30.0.50; [PATCH] Do not error out on non image file in image-dired Manuel Giraud via Bug reports for GNU Emacs, the Swiss army knife of text editors
2023-02-20 12:49 ` Eli Zaretskii
2023-02-20 13:35   ` Manuel Giraud via Bug reports for GNU Emacs, the Swiss army knife of text editors
2023-02-20 13:46     ` Eli Zaretskii
2023-02-20 14:13       ` Manuel Giraud via Bug reports for GNU Emacs, the Swiss army knife of text editors
2023-02-20 14:45         ` Eli Zaretskii
2023-02-20 16:41           ` Manuel Giraud via Bug reports for GNU Emacs, the Swiss army knife of text editors
2023-02-21 12:01             ` Eli Zaretskii
2023-02-21 12:16               ` Manuel Giraud via Bug reports for GNU Emacs, the Swiss army knife of text editors
2023-02-21 13:03                 ` Eli Zaretskii
2023-02-21 14:23                   ` Manuel Giraud via Bug reports for GNU Emacs, the Swiss army knife of text editors
2023-02-22 13:22                     ` Eli Zaretskii

Code repositories for project(s) associated with this external index

	https://git.savannah.gnu.org/cgit/emacs.git
	https://git.savannah.gnu.org/cgit/emacs/org-mode.git

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.