bug#68486: 29.1; Confusing window layout changes in image-dired

bug#68486: 29.1; Confusing window layout changes in image-dired

[joakim]

I have a question about image dired. I find it very confusing the way
window layout changes in image-dired happen.

Basically I want a window layout like depicted here:
https://www.emacswiki.org/emacs/ImageDired

In words: a 3 window layout, dired to the left, image dired top right, larger
view of current image bottom right.

+----+----+
|    |    |
|    +----+
|    |    |
+----+----+

Whenever I try to set this layout up by hand the main image view jumps between
windows, ruining my layout.

Theres a function image-dired-display-image that displays the image
you select for display in the thumb view window. the call to
kill-buffer and then pop-to-buffer would seem to indicate that this is
the intended behaviour, or is it? Shouldnt the code try to preserve
the users window configuration if possible?

The function, notice kill-buffer then pop-to-buffer.

(defun image-dired-display-image (file &optional _ignored)
  "Display image FILE in the image buffer window.
If FILE is an image, the window will use `image-dired-image-mode'
which is based on `image-mode'."
  (declare (advertised-calling-convention (file) "29.1"))
  (setq file (expand-file-name file))
  (when (not (file-exists-p file))
    (error "No such file: %s" file))
  (let ((buf (get-buffer image-dired-display-image-buffer))
        (cur-win (selected-window)))
    (when buf
      (kill-buffer buf))
    (when-let ((buf (find-file-noselect file nil t)))
      (pop-to-buffer buf)
      (rename-buffer image-dired-display-image-buffer)
      (if (string-match (image-file-name-regexp) file)
          (image-dired-image-mode)
        ;; Support visiting PDF files.
        (normal-mode))
      (select-window cur-win))))



In GNU Emacs 29.1 (build 1, x86_64-redhat-linux-gnu, GTK+ Version
 3.24.38, cairo version 1.17.8) of 2023-09-24 built on
 e1d0f5fdfea948b9bb25e212c9c623e4
Windowing system distributor 'The X.Org Foundation', version 11.0.70000000
System Description: Fedora Linux 39 (Workstation Edition)

Configured using:
 'configure --build=x86_64-redhat-linux-gnu
 --host=x86_64-redhat-linux-gnu --program-prefix=
 --disable-dependency-tracking --prefix=/usr --exec-prefix=/usr
 --bindir=/usr/bin --sbindir=/usr/sbin --sysconfdir=/etc
 --datadir=/usr/share --includedir=/usr/include --libdir=/usr/lib64
 --libexecdir=/usr/libexec --localstatedir=/var --runstatedir=/run
 --sharedstatedir=/var/lib --mandir=/usr/share/man
 --infodir=/usr/share/info --with-dbus --with-gif --with-jpeg --with-png
 --with-rsvg --with-tiff --with-xpm --with-x-toolkit=gtk3 --with-gpm=no
 --with-xwidgets --with-modules --with-harfbuzz --with-cairo --with-json
 --with-native-compilation=aot --with-tree-sitter --with-sqlite3
 --with-webp --with-xinput2 build_alias=x86_64-redhat-linux-gnu
 host_alias=x86_64-redhat-linux-gnu CC=gcc 'CFLAGS=-DMAIL_USE_LOCKF -O2
 -flto=auto -ffat-lto-objects -fexceptions -g -grecord-gcc-switches
 -pipe -Wall -Werror=format-security
 -Wp,-U_FORTIFY_SOURCE,-D_FORTIFY_SOURCE=3 -Wp,-D_GLIBCXX_ASSERTIONS
 -specs=/usr/lib/rpm/redhat/redhat-hardened-cc1 -fstack-protector-strong
 -specs=/usr/lib/rpm/redhat/redhat-annobin-cc1 -m64 -mtune=generic
 -fasynchronous-unwind-tables -fstack-clash-protection -fcf-protection
 -fno-omit-frame-pointer -mno-omit-leaf-frame-pointer '
 LDFLAGS=-Wl,-z,relro
 PKG_CONFIG_PATH=:/usr/lib64/pkgconfig:/usr/share/pkgconfig CXX=g++
 'CXXFLAGS=-O2 -flto=auto -ffat-lto-objects -fexceptions -g
 -grecord-gcc-switches -pipe -Wall -Werror=format-security
 -Wp,-U_FORTIFY_SOURCE,-D_FORTIFY_SOURCE=3 -Wp,-D_GLIBCXX_ASSERTIONS
 -specs=/usr/lib/rpm/redhat/redhat-hardened-cc1 -fstack-protector-strong
 -specs=/usr/lib/rpm/redhat/redhat-annobin-cc1 -m64 -mtune=generic
 -fasynchronous-unwind-tables -fstack-clash-protection -fcf-protection
 -fno-omit-frame-pointer -mno-omit-leaf-frame-pointer ''

Configured features:
ACL CAIRO DBUS FREETYPE GIF GLIB GMP GNUTLS GSETTINGS HARFBUZZ JPEG JSON
LIBOTF LIBSELINUX LIBSYSTEMD LIBXML2 M17N_FLT MODULES NATIVE_COMP NOTIFY
INOTIFY PDUMPER PNG RSVG SECCOMP SOUND SQLITE3 THREADS TIFF
TOOLKIT_SCROLL_BARS TREE_SITTER WEBP X11 XDBE XIM XINPUT2 XPM XWIDGETS
GTK3 ZLIB

Important settings:
  value of $LANG: sv_SE.UTF-8
  value of $XMODIFIERS: @im=none
  locale-coding-system: utf-8-unix

Major mode: Org

Minor modes in effect:
  async-bytecomp-package-mode: t
  global-git-commit-mode: t
  magit-auto-revert-mode: t
  org-super-agenda-mode: t
  org-ai-mode: t
  which-key-mode: t
  recentf-mode: t
  desktop-save-mode: t
  keyfreq-autosave-mode: t
  keyfreq-mode: t
  winner-mode: t
  projectile-mode: t
  smartparens-global-strict-mode: t
  global-highlight-parentheses-mode: t
  highlight-parentheses-mode: t
  company-quickhelp-mode: t
  company-quickhelp-local-mode: t
  global-company-mode: t
  shx-global-mode: t
  erc-stamp-mode: t
  diredfl-global-mode: t
  zen-reward-mode: t
  server-mode: t
  gcmh-mode: t
  global-treesit-auto-mode: t
  savehist-mode: t
  vertico-mode: t
  marginalia-mode: t
  shell-dirtrack-mode: t
  override-global-mode: t
  yas-global-mode: t
  yas-minor-mode: t
  tooltip-mode: t
  global-eldoc-mode: t
  show-paren-mode: t
  electric-indent-mode: t
  mouse-wheel-mode: t
  global-prettify-symbols-mode: t
  file-name-shadow-mode: t
  global-font-lock-mode: t
  font-lock-mode: t
  line-number-mode: t
  auto-fill-function: #[128 \304\300\301.#\207 [yas--auto-fill org-auto-fill-function :around nil apply] 5 advice]
  transient-mark-mode: t
  auto-composition-mode: t
  auto-encryption-mode: t
  auto-compression-mode: t
  abbrev-mode: t

Load-path shadows:
/home/joakim/.emacs.d/elpa/cmake-mode-20231214.1547/cmake-mode hides /usr/share/emacs/site-lisp/cmake/cmake-mode
/home/joakim/.emacs.d/elpa/transient-20240101.2225/transient hides /usr/share/emacs/29.1/lisp/transient
/home/joakim/.emacs.d/elpa/seq-2.24.0.20230904.183335/seq hides /usr/share/emacs/29.1/lisp/emacs-lisp/seq

Features:
(shadow emacsbug exiftool-autoloads woman man proced image-dired-dired
image-file image-converter calc-undo generic cider cider-debug
cider-browse-ns cider-mode cider-xref-backend cider-find cider-inspector
cider-completion cider-profile cider-eval cider-jar cider-repl-history
cider-repl cider-resolve cider-test cider-overlays cider-stacktrace
cider-doc cider-browse-spec cider-clojuredocs cider-eldoc
cider-docstring cider-client cider-common cider-completion-context
cider-connection cider-util cider-popup sesman-browser nrepl-client
queue nrepl-dict spinner parseedn parseclj-parser parseclj-lex
parseclj-alist smartparens-clojure sesman clojure-mode solar cal-dst
package-x org-ql-find css-mode cmake-ts-mode epa-file reveal mailalias
bbdb-com completion wisi-run-indent-test cus-start apropos pulse
org-lint org-archive ob-ditaa org-colview org-crypt org-ctags org-mouse
org-plot org-protocol info-look tramp-archive tramp-gvfs zeroconf
helm-org-rifle helm-org-rifle-autoloads helm-for-files helm-imenu
semantic/util-modes semantic/util semantic semantic/tag cedet
helm-bookmark helm-external helm-mode helm-misc helm-elisp helm-files
image-dired image-dired-tags image-dired-external image-dired-util
helm-buffers helm-tags helm-locate helm-eval helm-occur helm-grep
helm-regexp helm-net helm-adaptive helm-info helm-utils helm-types
helm-help helm helm-global-bindings helm-easymenu helm-core
async-bytecomp helm-autoloads helm-source helm-multi-match helm-lib
helm-core-autoloads smtpmail async async-autoloads wfnames-autoloads
flow-fill dictionary dictionary-connection org-attach mouseme
ess-toolbar ess-jags-d essd-els ess-sas-d ess-sp6-d ess-sas-l ess-sas-a
ess-bugs-d ess-bugs-l geiser-mode geiser-xref geiser-compile geiser-repl
geiser-capf geiser-debug geiser-image geiser-doc geiser-menu
geiser-autodoc geiser-edit geiser-completion geiser-eval
geiser-connection tq geiser-syntax geiser-log geiser-popup view
geiser-impl geiser-custom geiser-base smartparens-html sgml-mode
js2-mode smartparens-javascript js c-ts-common smartparens-c cc-mode
cc-fonts cc-guess cc-menus cc-cmds cc-styles cc-align cc-engine cc-vars
cc-defs ediff ediff-merg ediff-mult ediff-wind ediff-diff ediff-help
ediff-init ediff-util benchmark org-ql-completing-read vertico-flat
tar-mode arc-mode archive-mode display-line-numbers gnus-search
eieio-opt speedbar ezimage dframe gnus-cite smiley shr-color mail-extr
textsec uni-scripts idna-mapping ucs-normalize uni-confusable
textsec-check gnus-async gnus-bcklg qp gnus-ml utf-7 rfc2104 tramp-cmds
tramp-cache time-stamp tramp-sh smartparens-latex tex-mode cl-print
pcmpl-linux time pcmpl-gnu conf-mode goto-addr markdown-mode
toml-ts-mode make-mode combobulate yaml-ts-mode detached-dired calc-alg
calc-ext calc-menu calc calc-loaddefs calc-macs term/xterm xterm tabify
help-fns org-datetree org-capture gnus-topic nndraft nnmh gnus-agent
gnus-srvr gnus-score score-mode nnvirtual gnus-msg nnfolder gnus-cache
nntp ivy-yasnippet ivy delsel ivy-faces ivy-overlay colir cal-move vc-hg
vc-bzr vc-src vc-sccs vc-cvs vc-rcs log-view bug-reference wdired
magit-extras magit-bookmark magit-submodule 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
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 magit-git magit-base magit-section pcmpl-git
dired-aux gnus-dired detached-compile pcmpl-rpm pcmpl-unix
detached-shell detached notifications autorevert latexenc ox-hugo
ox-hugo-deprecated ffap ox-blackfriday ox-md tomelr ox-odt rng-loc
rng-uri rng-parse rng-match rng-dt rng-util rng-pttrn nxml-parse nxml-ns
nxml-enc xmltok nxml-util ox-latex ox-icalendar ox-html table ox-ascii
ox-publish ox misearch multi-isearch org-clock org-ql-search org-ql-view
ov crm diary-lib diary-loaddefs cal-iso org-super-agenda ht org-ai
org-ai-oobabooga websocket bindat org-ai-sd org-ai-talk
org-ai-on-project org-ai-useful org-ai-openai-image org-ai-openai
org-ai-block quail oc-basic ol-eww eww url-queue mm-url ol-rmail ol-mhe
ol-irc ol-info ol-gnus nnselect gnus-art mm-uu mml2015 mm-view mml-smime
smime dig ol-docview doc-view filenotify jka-compr image-mode exif
ol-bibtex bibtex ol-bbdb ol-w3m ol-doi org-link-doi origami
origami-parsers hl-line mule-util company-oddmuse company-keywords
company-etags etags fileloop company-gtags company-dabbrev-code
company-dabbrev company-files company-clang company-capf company-cmake
company-semantic company-template company-bbdb init which-key spray
face-remap wgrep memory-usage recentf tree-widget desktop frameset
keyfreq zygospore winner ace-window avy restclient nginx-mode yaml-mode
dockerfile-mode sh-script smie executable geiser hy-mode hy-jedhy
hy-shell hy-font-lock hy-base autodisass-java-bytecode ad-javap-mode
meghanada which-func imenu projectile grep ibuf-ext ibuffer
ibuffer-loaddefs hydra lv smartparens-config smartparens-rst
smartparens-markdown smartparens-org smartparens-text smartparens-python
smartparens-ess smartparens loadhist highlight-parentheses
company-quickhelp pos-tip company shx bash-completion ob-cypher n4js
cypher-mode cmake-mode rst use-package-ensure-system-package use-package
use-package-delight use-package-diminish system-packages printing
ps-print ps-print-loaddefs lpr gnus-alias nnmairix nnml 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 gnus-spec gnus-int
gnus-range gnus-win gnus nnoo nnheader range sort bbdb bbdb-site
timezone erc-match erc-ezbounce erc-hl-nicks erc-button erc-fill
erc-stamp erc-goodies erc erc-backend erc-networks erc-common erc-compat
erc-loaddefs pastebin secrets dbus diredfl dired-x popup sauron cl
boxquote rect generic-x disp-table server gcmh ag vc-svn find-dired f
f-shortdoc shortdoc csound-mode csound-manual-lookup csound-skeleton
csound-indentation csound-repl highlight facemenu
csound-repl-interaction shut-up multi csound-score csound-font-lock
csound-eldoc csound-util csound-opcodes ert ewoc combobulate-json
combobulate-yaml combobulate-css combobulate-js-ts combobulate-python
combobulate-query combobulate-ui combobulate-display combobulate-contrib
combobulate-envelope transient scheme combobulate-html combobulate-ztree
edebug debug backtrace combobulate-manipulation tempo combobulate-rules
combobulate-navigation combobulate-misc combobulate-interface
combobulate-settings loaddefs-gen radix-tree cus-edit cus-load wid-edit
vc-git diff-mode cursor-sensor mm-archive message sendmail yank-media
dired dired-loaddefs rfc822 mml mml-sec epa derived epg rfc6068
epg-config gnus-util mailabbrev gmm-utils mailheader mm-decode mm-bodies
mm-encode mail-utils gnutls network-stream url-cache url-http url-auth
mail-parse rfc2231 rfc2047 rfc2045 mm-util ietf-drums mail-prsvr url-gw
nsm puny package-vc vc vc-dispatcher treesit-auto orderless savehist
vertico marginalia consult bookmark pp detached-init
modus-operandi-theme modus-themes highlight-indent-guides advice
ada-mode wisi-process-parse align ada-skel wisi-skel skeleton
ada-indent-user-options ada-core uniquify-files gnat-compiler wisi-prj
wisi wisi-fringe wisi-parse-common semantic/lex semantic/fw mode-local
find-file ob-shell ob-python python compat treesit ob-julia ob-ess-julia
ess-julia julia-mode julia-mode-latexsubs ess-r-mode ess-r-flymake
ess-r-xref xref ess-trns ess-r-package ess-r-completion ess-roxy
ess-r-syntax ess-rd hideshow ess-s-lang ess-help ess-mode ess-inf
ess-tracebug comp comp-cstr tramp tramp-loaddefs trampver
tramp-integration files-x tramp-compat shell parse-time iso8601 ess
lisp-mnt ess-utils ess-custom flymake-proc flymake project compile
text-property-search warnings thingatpt ob-gnuplot ob-plantuml ob-dot
general org-ql ts org-duration peg org-habit org-agenda org-element
org-persist xdg org-id avl-tree generator org-refile kurecolor dash s
color edmacro kmacro use-package-bind-key bind-key easy-mmode
yasnippet-snippets yasnippet outshine outshine-org-cmds outorg 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 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 format-spec noutline outline icons cl-extra
help-mode use-package-ensure use-package-core flyspell ispell finder-inf
clang-rename clang-include-fixer let-alist clang-format xml
ace-window-autoloads ada-mode-autoloads ag-autoloads
autodisass-java-bytecode-autoloads avy-autoloads
bash-completion-autoloads bbdb-autoloads boxquote-autoloads
calfw-autoloads calfw-org-autoloads cider-autoloads
clojure-mode-autoloads cmake-mode-autoloads combobulate-autoloads
company-quickhelp-autoloads csound-mode-autoloads detached-autoloads
diredfl-autoloads dockerfile-mode-autoloads embark-consult-autoloads
consult-autoloads embark-autoloads erc-hl-nicks-autoloads
expand-region-autoloads gcmh-autoloads geiser-autoloads
general-autoloads gnat-compiler-autoloads gnus-alias-autoloads
go-mode-autoloads groovy-mode-autoloads haskell-mode-autoloads
helpful-autoloads elisp-refs-autoloads highlight-autoloads
highlight-indent-guides-autoloads highlight-parentheses-autoloads
hy-mode-autoloads hydra-autoloads ivy-yasnippet-autoloads ivy-autoloads
js2-mode-autoloads keyfreq-autoloads kotlin-mode-autoloads
kurecolor-autoloads lua-mode-autoloads lv-autoloads magit-autoloads
pcase git-commit-autoloads magit-section-autoloads marginalia-autoloads
markdown-mode-autoloads meghanada-autoloads flycheck-autoloads
company-autoloads memory-usage-autoloads multi-autoloads
multitran-autoloads n4js-autoloads nginx-mode-autoloads
ob-cypher-autoloads dash-functional-autoloads cypher-mode-autoloads
ob-ess-julia-autoloads julia-mode-autoloads ess-autoloads
orderless-autoloads org-ai-autoloads org-ql-autoloads f-autoloads
org-super-agenda-autoloads ht-autoloads origami-autoloads
outshine-autoloads outorg-autoloads ov-autoloads ox-hugo-autoloads
ox-reveal-autoloads parseedn-autoloads parseclj-autoloads
pastebin-autoloads peg-autoloads pkg-info-autoloads epl-autoloads
popup-autoloads pos-tip-autoloads projectile-autoloads
protobuf-mode-autoloads queue-autoloads restclient-autoloads
rust-mode-autoloads sauron-autoloads sesman-autoloads shut-up-autoloads
shx-autoloads smartparens-autoloads spinner-autoloads spray-autoloads
system-packages-autoloads tomelr-autoloads transient-autoloads
treesit-auto-autoloads ts-autoloads s-autoloads dash-autoloads
tuareg-autoloads rx caml-autoloads uniquify-files-autoloads
vertico-autoloads webpaste-autoloads request-autoloads
websocket-autoloads wgrep-autoloads which-key-autoloads
wiki-summary-autoloads wisi-autoloads with-editor-autoloads info
compat-autoloads seq-autoloads yaml-mode-autoloads
yasnippet-snippets-autoloads yasnippet-autoloads zygospore-autoloads
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 xwidget-internal dbusbind
inotify dynamic-setting system-font-setting font-render-setting cairo
move-toolbar gtk x-toolkit xinput2 x multi-tty make-network-process
native-compile emacs)

Memory information:
((conses 16 15532749 6431434)
 (symbols 48 209577 8825)
 (strings 32 1052743 884354)
 (string-bytes 1 36386995)
 (vectors 16 405317)
 (vector-slots 8 8518378 8366455)
 (floats 8 4011 16155)
 (intervals 56 1151426 134332)
 (buffers 984 899))
--
Joakim Verona
joakim@verona.se

bug#68486: 29.1; Confusing window layout changes in image-dired

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

joakim@verona.se writes:

> I have a question about image dired. I find it very confusing the way
> window layout changes in image-dired happen.
>
> Basically I want a window layout like depicted here:
> https://www.emacswiki.org/emacs/ImageDired
>
> In words: a 3 window layout, dired to the left, image dired top right, larger
> view of current image bottom right.
>
> +----+----+
> |    |    |
> |    +----+
> |    |    |
> +----+----+
>
> Whenever I try to set this layout up by hand the main image view jumps between
> windows, ruining my layout.

Hi Joakim,

I have tried your layout and you're right that this is annoying: for me
the dired buffer and image view buffer are switching windows each time I
go to the next image.

Maybe we should try to fix this but, as usual, with Emacs display layout
it could not be to everybody's taste.

> Theres a function image-dired-display-image that displays the image
> you select for display in the thumb view window. the call to
> kill-buffer and then pop-to-buffer would seem to indicate that this is
> the intended behaviour, or is it? Shouldnt the code try to preserve
> the users window configuration if possible?
>
> The function, notice kill-buffer then pop-to-buffer.

Thanks for this hint.  You're right that the kill/pop-to/rename dance
may be the cause of this issue.
-- 
Manuel Giraud

bug#68486: 29.1; Confusing window layout changes in image-dired

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

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

Hi,

Could you try this patch?
-- 
Manuel Giraud

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Refill-image-dired-display-image-buffer.patch --]
[-- Type: text/x-patch, Size: 1852 bytes --]

From b979865cf8cf2fa8ec73ac8bc6cfee32f6de68fc Mon Sep 17 00:00:00 2001
From: Manuel Giraud <manuel@ledu-giraud.fr>
Date: Tue, 16 Jan 2024 10:47:45 +0100
Subject: [PATCH] Refill 'image-dired-display-image-buffer'

Bug#68486

* lisp/image/image-dired.el (image-dired-display-image): Refill
'image-dired-display-image-buffer' instead of killing it, open a new
file and then renaming it.
---
 lisp/image/image-dired.el | 22 +++++++++++-----------
 1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/lisp/image/image-dired.el b/lisp/image/image-dired.el
index 9eb68e240fe..b08aa129b98 100644
--- a/lisp/image/image-dired.el
+++ b/lisp/image/image-dired.el
@@ -1260,18 +1260,18 @@ image-dired-display-image
   (setq file (expand-file-name file))
   (when (not (file-exists-p file))
     (error "No such file: %s" file))
-  (let ((buf (get-buffer image-dired-display-image-buffer))
+  (let ((buf (get-buffer-create image-dired-display-image-buffer))
         (cur-win (selected-window)))
-    (when buf
-      (kill-buffer buf))
-    (when-let ((buf (find-file-noselect file nil t)))
-      (pop-to-buffer buf)
-      (rename-buffer image-dired-display-image-buffer)
-      (if (string-match (image-file-name-regexp) file)
-          (image-dired-image-mode)
-        ;; Support visiting PDF files.
-        (normal-mode))
-      (select-window cur-win))))
+    (with-current-buffer buf
+      (let ((inhibit-read-only t))
+        (erase-buffer)
+        (insert-file-contents file)
+        (if (string-match (image-file-name-regexp) file)
+            (image-dired-image-mode)
+          ;; Support visiting PDF files.
+          (normal-mode))))
+    (when buf (pop-to-buffer buf))
+    (select-window cur-win)))
 
 (defun image-dired-display-this (&optional arg)
   "Display current thumbnail's original image in display buffer.
-- 
2.43.0

bug#68486: 29.1; Confusing window layout changes in image-dired

[joakim]

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

> joakim@verona.se writes:
>
>> I have a question about image dired. I find it very confusing the way
>> window layout changes in image-dired happen.
>>
>> Basically I want a window layout like depicted here:
>> https://www.emacswiki.org/emacs/ImageDired
>>
>> In words: a 3 window layout, dired to the left, image dired top right, larger
>> view of current image bottom right.
>>
>> +----+----+
>> |    |    |
>> |    +----+
>> |    |    |
>> +----+----+
>>
>> Whenever I try to set this layout up by hand the main image view jumps between
>> windows, ruining my layout.
>
> Hi Joakim,
>
> I have tried your layout and you're right that this is annoying: for me
> the dired buffer and image view buffer are switching windows each time I
> go to the next image.
>
> Maybe we should try to fix this but, as usual, with Emacs display layout
> it could not be to everybody's taste.

Agreed, but I think it wouldnt be controversial to keep an existing
user created layout, and you seem to agree with this sentiment.

>
>> Theres a function image-dired-display-image that displays the image
>> you select for display in the thumb view window. the call to
>> kill-buffer and then pop-to-buffer would seem to indicate that this is
>> the intended behaviour, or is it? Shouldnt the code try to preserve
>> the users window configuration if possible?
>>
>> The function, notice kill-buffer then pop-to-buffer.
>
> Thanks for this hint.  You're right that the kill/pop-to/rename dance
> may be the cause of this issue.

I tried just removing the kill-buffer, but that didnt work, I guess the
call is there for a reason. I thought that if the buffer isnt killed,
and the content just replaced everything should work, but aparently not.

If you come up with a patch I will try it!

-- 
Joakim Verona
joakim@verona.se

bug#68486: 29.1; Confusing window layout changes in image-dired

[joakim]

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

> Hi,
>
> Could you try this patch?

After some failures trying the patch in my running emacs session, I
tried with emacs -q, and now it worked, big improvement!



> -- 
> Manuel Giraud
>
> From b979865cf8cf2fa8ec73ac8bc6cfee32f6de68fc Mon Sep 17 00:00:00 2001
> From: Manuel Giraud <manuel@ledu-giraud.fr>
> Date: Tue, 16 Jan 2024 10:47:45 +0100
> Subject: [PATCH] Refill 'image-dired-display-image-buffer'
>
> Bug#68486
>
> * lisp/image/image-dired.el (image-dired-display-image): Refill
> 'image-dired-display-image-buffer' instead of killing it, open a new
> file and then renaming it.
> ---
>  lisp/image/image-dired.el | 22 +++++++++++-----------
>  1 file changed, 11 insertions(+), 11 deletions(-)
>
> diff --git a/lisp/image/image-dired.el b/lisp/image/image-dired.el
> index 9eb68e240fe..b08aa129b98 100644
> --- a/lisp/image/image-dired.el
> +++ b/lisp/image/image-dired.el
> @@ -1260,18 +1260,18 @@ image-dired-display-image
>    (setq file (expand-file-name file))
>    (when (not (file-exists-p file))
>      (error "No such file: %s" file))
> -  (let ((buf (get-buffer image-dired-display-image-buffer))
> +  (let ((buf (get-buffer-create image-dired-display-image-buffer))
>          (cur-win (selected-window)))
> -    (when buf
> -      (kill-buffer buf))
> -    (when-let ((buf (find-file-noselect file nil t)))
> -      (pop-to-buffer buf)
> -      (rename-buffer image-dired-display-image-buffer)
> -      (if (string-match (image-file-name-regexp) file)
> -          (image-dired-image-mode)
> -        ;; Support visiting PDF files.
> -        (normal-mode))
> -      (select-window cur-win))))
> +    (with-current-buffer buf
> +      (let ((inhibit-read-only t))
> +        (erase-buffer)
> +        (insert-file-contents file)
> +        (if (string-match (image-file-name-regexp) file)
> +            (image-dired-image-mode)
> +          ;; Support visiting PDF files.
> +          (normal-mode))))
> +    (when buf (pop-to-buffer buf))
> +    (select-window cur-win)))
>  
>  (defun image-dired-display-this (&optional arg)
>    "Display current thumbnail's original image in display buffer.
-- 
Joakim Verona
joakim@verona.se

bug#68486: 29.1; Confusing window layout changes in image-dired

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

joakim@verona.se writes:

> Manuel Giraud via "Bug reports for GNU Emacs, the Swiss army knife of
> text editors" <bug-gnu-emacs@gnu.org> writes:
>
>> Hi,
>>
>> Could you try this patch?
>
> After some failures trying the patch in my running emacs session, I
> tried with emacs -q, and now it worked, big improvement!

Ah yes sorry this patch is on current master.  Maybe I'd have rebase it
on 29 but let's wait for input from Eli and Stefan K.
-- 
Manuel Giraud

bug#68486: 29.1; Confusing window layout changes in image-dired

[Eli Zaretskii]

> Cc: 68486@debbugs.gnu.org
> Date: Tue, 16 Jan 2024 15:36:59 +0100
> From:  Manuel Giraud via "Bug reports for GNU Emacs,
>  the Swiss army knife of text editors" <bug-gnu-emacs@gnu.org>
> 
> joakim@verona.se writes:
> 
> > Manuel Giraud via "Bug reports for GNU Emacs, the Swiss army knife of
> > text editors" <bug-gnu-emacs@gnu.org> writes:
> >
> >> Hi,
> >>
> >> Could you try this patch?
> >
> > After some failures trying the patch in my running emacs session, I
> > tried with emacs -q, and now it worked, big improvement!
> 
> Ah yes sorry this patch is on current master.  Maybe I'd have rebase it
> on 29 but let's wait for input from Eli and Stefan K.

I think the problem is not serious enough to warrant such a change on
the release branch.

bug#68486: 29.1; Confusing window layout changes in image-dired

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

Eli Zaretskii <eliz@gnu.org> writes:

[...]

> I think the problem is not serious enough to warrant such a change on
> the release branch.

Ok so we can try to solve this on master then.
-- 
Manuel Giraud