unofficial mirror of bug-gnu-emacs@gnu.org 
 help / color / mirror / code / Atom feed
* bug#57334: 28.1; Fix wdired with (dired '(dir f1 f2 ...))
@ 2022-08-22  6:23 Thierry Volpiatto
  2022-08-22 10:27 ` Lars Ingebrigtsen
                   ` (2 more replies)
  0 siblings, 3 replies; 16+ messages in thread
From: Thierry Volpiatto @ 2022-08-22  6:23 UTC (permalink / raw)
  To: 57334


Hello Emacs,

there is an action in helm that allows creating a dired buffer with
marked files for further editing with wdired.
For this I have to call dired with its dirname argument as a list:
  (dired ' (dir f1 f2 f3))
Unfortunately this is broken since years and until now I had to use an advice
to fix it.
The advice is working up to emacs-28.1 but now it becomes difficult to
write an advice compatible with all emacs versions, here is a patch to
apply on 29.0.50.

To reproduce the bug from emacs -Q:

1) Ensure ~/tmp/test.txt and ~/tmp/test2.txt exist
2) (dired '("~/tmp" "/home/thierry/tmp/test.txt" "/home/thierry/tmp/test2.txt"))
3) M-x wdired-change-to-dired-mode (C-x C-q)
4) Rename test.txt to test1.txt
5) C-x C-s

diff --git a/lisp/wdired.el b/lisp/wdired.el
index 106d57174d5..7322eeff872 100644
--- a/lisp/wdired.el
+++ b/lisp/wdired.el
@@ -537,15 +537,23 @@ non-nil means return old filename."
     (wdired-change-to-dired-mode)
     (if changes
 	(progn
-	  ;; If we are displaying a single file (rather than the
-	  ;; contents of a directory), change dired-directory if that
-	  ;; file was renamed.  (This ought to be generalized to
-	  ;; handle the multiple files case, but that's less trivial).
-	  (when (and (stringp dired-directory)
-		     (not (file-directory-p dired-directory))
-		     (null some-file-names-unchanged)
-		     (= (length files-renamed) 1))
-	    (setq dired-directory (cdr (car files-renamed))))
+	  (cond ((and (stringp dired-directory)
+                      (not (file-directory-p dired-directory))
+                      (null some-file-names-unchanged)
+                      (= (length files-renamed) 1))
+                 (setq dired-directory (cdr (car files-renamed))))
+                ;; Fix dired buffers created with
+                ;; (dired '(foo f1 f2 f3)).
+                ((and (consp dired-directory)
+                      (cdr dired-directory)
+                      files-renamed)
+                 (setcdr dired-directory
+                         ;; Replace in `dired-directory' files that have
+                         ;; been modified with their new name keeping
+                         ;; the ones that are unmodified at the same place.
+                         (cl-loop for f in (cdr dired-directory)
+                                  collect (or (assoc-default f files-renamed)
+                                              f)))))
 	  ;; Re-sort the buffer.
 	  (revert-buffer)
 	  (let ((inhibit-read-only t))



In GNU Emacs 28.1 (build 2, x86_64-pc-linux-gnu, Motif Version 2.3.8, cairo version 1.16.0)
 of 2022-04-20 built on IPad-S340
Windowing system distributor 'The X.Org Foundation', version 11.0.12013000
System Description: Linux Mint 20.3

Configured using:
 'configure CFLAGS=-O8 --with-mailutils --with-cairo --without-dbus
 --without-gconf --without-gsettings --with-x-toolkit=motif'

Configured features:
ACL CAIRO FREETYPE GIF GLIB GMP GNUTLS GPM HARFBUZZ JPEG JSON LCMS2
LIBOTF LIBSELINUX LIBSYSTEMD LIBXML2 M17N_FLT MODULES NOTIFY INOTIFY
PDUMPER PNG RSVG SECCOMP SOUND THREADS TIFF TOOLKIT_SCROLL_BARS X11 XDBE
XIM XPM MOTIF ZLIB

Important settings:
  value of $LANG: fr_FR.UTF-8
  locale-coding-system: utf-8-unix

Major mode: İĽ

Minor modes in effect:
  global-undo-tree-mode: t
  undo-tree-mode: t
  psession-mode: t
  psession-savehist-mode: t
  global-git-gutter-mode: t
  display-time-mode: t
  winner-mode: t
  helm-epa-mode: t
  helm-descbinds-mode: t
  helm-adaptive-mode: t
  helm-mode: t
  helm-minibuffer-history-mode: t
  helm-ff-icon-mode: t
  shell-dirtrack-mode: t
  helm-popup-tip-mode: t
  async-bytecomp-package-mode: t
  dired-async-mode: t
  minibuffer-depth-indicate-mode: t
  gcmh-mode: t
  tooltip-mode: t
  global-eldoc-mode: t
  eldoc-mode: t
  show-paren-mode: t
  mouse-wheel-mode: t
  file-name-shadow-mode: t
  global-font-lock-mode: t
  font-lock-mode: t
  auto-composition-mode: t
  auto-encryption-mode: t
  auto-compression-mode: t
  column-number-mode: t
  line-number-mode: t
  transient-mark-mode: t

Load-path shadows:
None found.

Features:
(shadow epa-mail face-remap emacsbug helm-command whitespace tabify
tramp-cache tv-mu4e-config mu4e-contrib mu4e-patch mu4e mu4e-org
mu4e-main mu4e-view gnus-art mm-uu mml2015 mm-view mml-smime smime dig
mu4e-headers mu4e-compose mu4e-draft mu4e-actions smtpmail sendmail
mu4e-search mu4e-lists mu4e-bookmarks mu4e-mark mu4e-message flow-fill
hl-line mu4e-contacts mu4e-update mu4e-folders mu4e-server mu4e-context
mu4e-vars mu4e-helpers mu4e-config ido helm-ring helm-dabbrev
smerge-mode helm-x-files helm-for-files image-file image-converter
char-fold tramp-archive tramp-gvfs rst vc-filewise vc-rcs conf-mode
ledger-config ledger-mode ledger-check ledger-texi ledger-test
ledger-sort ledger-report ledger-reconcile ledger-occur ledger-fonts
ledger-fontify ledger-state ledger-complete ledger-schedule ledger-init
ledger-xact ledger-post ledger-exec ledger-navigate eshell esh-cmd
esh-ext esh-opt esh-proc esh-io esh-arg esh-module esh-groups esh-util
ledger-context ledger-commodities ledger-regex checkdoc lisp-mnt
markdown-mode make-mode flymake-shellcheck flymake-proc flymake project
warnings sh-script smie executable bug-reference naquadah-theme solar
cal-dst holidays hol-loaddefs tv-utils osm yaml-mode undo-tree diff
queue rainbow-mode color psession frameset log-view pcvs-util
bash-completion cl-indent pcase ffap thingatpt autocrypt-message
autocrypt-gnus addressbook-bookmark gnus-sum shr kinsoku svg dom
gnus-group gnus-undo gnus-start gnus-dbus dbus gnus-cloud nnimap nnmail
mail-source utf7 netrc nnoo gnus-spec gnus-int gnus-range gnus-win
message rmc puny rfc822 mml mml-sec mm-decode mm-bodies mm-encode
mail-parse rfc2231 mailabbrev gmm-utils mailheader gnus nnheader
gnus-util rmail rmail-loaddefs rfc2047 rfc2045 mail-utils mm-util
mail-prsvr autocrypt-mu4e autocrypt ietf-drums config-w3m git-gutter
mule-util appt diary-lib diary-loaddefs gud wdired dired-extension
org-config ob-gnuplot org-crypt net-utils time winner autotest-mode
autoconf-mode woman man ediff ediff-merg ediff-mult ediff-wind
ediff-diff ediff-help ediff-init ediff-util init-helm helm-ls-git vc-git
diff-mode vc vc-dispatcher helm-fd epa derived epg rfc6068 epg-config
helm-epa helm-imenu imenu helm-elisp-package helm-find helm-org org ob
ob-tangle ob-ref ob-lob ob-table ob-exp org-macro org-footnote org-src
ob-comint org-pcomplete org-list org-faces org-entities noutline outline
org-version ob-emacs-lisp ob-core ob-eval org-table oc-basic bibtex ol
rx org-keys oc org-compat advice org-macs org-loaddefs cal-menu calendar
cal-loaddefs helm-external isearch-light helm-descbinds helm-wikipedia
all-the-icons all-the-icons-faces data-material data-weathericons
data-octicons data-fileicons data-faicons data-alltheicons cus-edit
wid-edit helm-ipython helm-elisp helm-eval edebug backtrace find-func
python tramp-sh helm-bookmark helm-net xml helm-info bookmark pp
helm-adaptive helm-mode helm-misc helm-files image-dired image-mode exif
filenotify tramp tramp-loaddefs trampver tramp-integration files-x
tramp-compat shell pcomplete parse-time iso8601 time-date ls-lisp
helm-buffers helm-occur helm-tags helm-locate helm-grep wgrep-helm wgrep
grep compile text-property-search comint ring helm-regexp format-spec
ansi-color helm-utils helm-help helm-types helm-extensions-autoloads
helm-config helm-autoloads helm helm-global-bindings helm-easymenu
helm-core async-bytecomp helm-source helm-multi-match helm-lib
dired-async dired-aux dired dired-loaddefs async popup diminish mb-depth
server edmacro kmacro avoid cus-load gcmh cl-extra help-mode use-package
use-package-ensure use-package-delight use-package-diminish
use-package-bind-key bind-key easy-mmode use-package-core finder-inf
package browse-url url url-proxy url-privacy url-expand url-methods
url-history url-cookie url-domsuf url-util mailcap url-handlers
url-parse auth-source cl-seq eieio eieio-core cl-macs eieio-loaddefs
password-cache json subr-x map url-vars seq byte-opt gv bytecomp
byte-compile cconv cl-loaddefs cl-lib info w3m-load iso-transl tooltip
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 cl-generic 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 simple abbrev obarray
cl-preloaded nadvice button loaddefs faces cus-face macroexp files
window text-properties overlay sha1 md5 base64 format env code-pages
mule custom widget hashtable-print-readable backquote threads inotify
lcms2 dynamic-setting font-render-setting cairo motif x-toolkit x
multi-tty make-network-process emacs)

Memory information:
((conses 16 895555 348095)
 (symbols 48 43408 4)
 (strings 32 257791 42475)
 (string-bytes 1 7581558)
 (vectors 16 88254)
 (vector-slots 8 1888821 470585)
 (floats 8 3310 1630)
 (intervals 56 21597 12985)
 (buffers 992 134))
<#secure method=pgpmime mode=sign>

-- 
Thierry





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

* bug#57334: 28.1; Fix wdired with (dired '(dir f1 f2 ...))
  2022-08-22  6:23 bug#57334: 28.1; Fix wdired with (dired '(dir f1 f2 ...)) Thierry Volpiatto
@ 2022-08-22 10:27 ` Lars Ingebrigtsen
  2022-08-22 14:42   ` Thierry Volpiatto
  2022-08-22 11:26 ` Eli Zaretskii
  2022-08-23  1:01 ` Michael Heerdegen
  2 siblings, 1 reply; 16+ messages in thread
From: Lars Ingebrigtsen @ 2022-08-22 10:27 UTC (permalink / raw)
  To: Thierry Volpiatto; +Cc: 57334

Thierry Volpiatto <thievol@posteo.net> writes:

> +                 (setcdr dired-directory
> +                         ;; Replace in `dired-directory' files that have
> +                         ;; been modified with their new name keeping
> +                         ;; the ones that are unmodified at the same place.
> +                         (cl-loop for f in (cdr dired-directory)
> +                                  collect (or (assoc-default f files-renamed)
> +                                              f)))))

This isn't obviously safe -- I think you're changing the list that
`dired' was originally called with here, which we shouldn't do.  (It may
even be a constant.)

So I think this should be changed to not do that.





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

* bug#57334: 28.1; Fix wdired with (dired '(dir f1 f2 ...))
  2022-08-22  6:23 bug#57334: 28.1; Fix wdired with (dired '(dir f1 f2 ...)) Thierry Volpiatto
  2022-08-22 10:27 ` Lars Ingebrigtsen
@ 2022-08-22 11:26 ` Eli Zaretskii
  2022-08-22 14:51   ` Thierry Volpiatto
  2022-08-23  1:01 ` Michael Heerdegen
  2 siblings, 1 reply; 16+ messages in thread
From: Eli Zaretskii @ 2022-08-22 11:26 UTC (permalink / raw)
  To: Thierry Volpiatto; +Cc: 57334

> From: Thierry Volpiatto <thievol@posteo.net>
> Date: Mon, 22 Aug 2022 06:23:52 +0000
> 
> there is an action in helm that allows creating a dired buffer with
> marked files for further editing with wdired.
> For this I have to call dired with its dirname argument as a list:
>   (dired ' (dir f1 f2 f3))
> Unfortunately this is broken since years and until now I had to use an advice
> to fix it.
> The advice is working up to emacs-28.1 but now it becomes difficult to
> write an advice compatible with all emacs versions, here is a patch to
> apply on 29.0.50.
> 
> To reproduce the bug from emacs -Q:
> 
> 1) Ensure ~/tmp/test.txt and ~/tmp/test2.txt exist
> 2) (dired '("~/tmp" "/home/thierry/tmp/test.txt" "/home/thierry/tmp/test2.txt"))
> 3) M-x wdired-change-to-dired-mode (C-x C-q)
> 4) Rename test.txt to test1.txt
> 5) C-x C-s

Is this the same problem as reported here:

  https://lists.gnu.org/archive/html/help-gnu-emacs/2022-08/msg00093.html

And if so, does the patch I posted in response, here:

  https://lists.gnu.org/archive/html/help-gnu-emacs/2022-08/msg00102.html

solve it, perhaps?





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

* bug#57334: 28.1; Fix wdired with (dired '(dir f1 f2 ...))
  2022-08-22 10:27 ` Lars Ingebrigtsen
@ 2022-08-22 14:42   ` Thierry Volpiatto
  2022-08-22 14:54     ` Lars Ingebrigtsen
  0 siblings, 1 reply; 16+ messages in thread
From: Thierry Volpiatto @ 2022-08-22 14:42 UTC (permalink / raw)
  To: Lars Ingebrigtsen; +Cc: 57334

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


Lars Ingebrigtsen <larsi@gnus.org> writes:

> Thierry Volpiatto <thievol@posteo.net> writes:
>
>> +                 (setcdr dired-directory
>> +                         ;; Replace in `dired-directory' files that have
>> +                         ;; been modified with their new name keeping
>> +                         ;; the ones that are unmodified at the same place.
>> +                         (cl-loop for f in (cdr dired-directory)
>> +                                  collect (or (assoc-default f files-renamed)
>> +                                              f)))))
>
> This isn't obviously safe -- I think you're changing the list that
> `dired' was originally called with here,

Of course it have to be changed, it has been modified by wdired at this point,
so if you want to redisplay a dired buffer reflecting your changes you
have to modify it no ?

> which we shouldn't do.

You already do it when DIRNAME is a string isn't it?

> (It may even be a constant.)

Can you elaborate?

> So I think this should be changed to not do that.


-- 
Thierry

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

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

* bug#57334: 28.1; Fix wdired with (dired '(dir f1 f2 ...))
  2022-08-22 11:26 ` Eli Zaretskii
@ 2022-08-22 14:51   ` Thierry Volpiatto
  0 siblings, 0 replies; 16+ messages in thread
From: Thierry Volpiatto @ 2022-08-22 14:51 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 57334

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


Eli Zaretskii <eliz@gnu.org> writes:

>> From: Thierry Volpiatto <thievol@posteo.net>
>> Date: Mon, 22 Aug 2022 06:23:52 +0000
>> 
>> there is an action in helm that allows creating a dired buffer with
>> marked files for further editing with wdired.
>> For this I have to call dired with its dirname argument as a list:
>>   (dired ' (dir f1 f2 f3))
>> Unfortunately this is broken since years and until now I had to use an advice
>> to fix it.
>> The advice is working up to emacs-28.1 but now it becomes difficult to
>> write an advice compatible with all emacs versions, here is a patch to
>> apply on 29.0.50.
>> 
>> To reproduce the bug from emacs -Q:
>> 
>> 1) Ensure ~/tmp/test.txt and ~/tmp/test2.txt exist
>> 2) (dired '("~/tmp" "/home/thierry/tmp/test.txt" "/home/thierry/tmp/test2.txt"))
>> 3) M-x wdired-change-to-dired-mode (C-x C-q)
>> 4) Rename test.txt to test1.txt
>> 5) C-x C-s
>
> Is this the same problem as reported here:
>
>   https://lists.gnu.org/archive/html/help-gnu-emacs/2022-08/msg00093.html

Not really, the dired call is more or less the same but the action is
different, here it is wdired which is involved.

> And if so, does the patch I posted in response, here:
>
>   https://lists.gnu.org/archive/html/help-gnu-emacs/2022-08/msg00102.html
>
> solve it, perhaps?

Didn't try but I guess no.

-- 
Thierry

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

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

* bug#57334: 28.1; Fix wdired with (dired '(dir f1 f2 ...))
  2022-08-22 14:42   ` Thierry Volpiatto
@ 2022-08-22 14:54     ` Lars Ingebrigtsen
  2022-08-22 14:59       ` Lars Ingebrigtsen
  2022-08-22 15:45       ` Thierry Volpiatto
  0 siblings, 2 replies; 16+ messages in thread
From: Lars Ingebrigtsen @ 2022-08-22 14:54 UTC (permalink / raw)
  To: Thierry Volpiatto; +Cc: 57334

Thierry Volpiatto <thievol@posteo.net> writes:

>>> +                 (setcdr dired-directory

[...]

> Of course it have to be changed, it has been modified by wdired at this point,
> so if you want to redisplay a dired buffer reflecting your changes you
> have to modify it no ?

I don't understand what you mean.

>> which we shouldn't do.
>
> You already do it when DIRNAME is a string isn't it?

Strings can't be modified.

>> (It may even be a constant.)
>
> Can you elaborate?

If the list is in purespace, for instance, it can't be modified.

In case there's any misunderstanding here, I'm talking about the
destructive alteration of the list pointed to by dired-directory by that
`setcdr' -- not the altering of the dired-directory variable.  So the
safe change here would be something like

  (setq dired-directory (cons (car dired-directory) (mapcar ...)))





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

* bug#57334: 28.1; Fix wdired with (dired '(dir f1 f2 ...))
  2022-08-22 14:54     ` Lars Ingebrigtsen
@ 2022-08-22 14:59       ` Lars Ingebrigtsen
  2022-08-22 15:45       ` Thierry Volpiatto
  1 sibling, 0 replies; 16+ messages in thread
From: Lars Ingebrigtsen @ 2022-08-22 14:59 UTC (permalink / raw)
  To: Thierry Volpiatto; +Cc: 57334

Lars Ingebrigtsen <larsi@gnus.org> writes:

> Strings can't be modified.

(Well, that's not accurate -- you can modify (some) strings with
`aset'.)






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

* bug#57334: 28.1; Fix wdired with (dired '(dir f1 f2 ...))
  2022-08-22 14:54     ` Lars Ingebrigtsen
  2022-08-22 14:59       ` Lars Ingebrigtsen
@ 2022-08-22 15:45       ` Thierry Volpiatto
  2022-08-22 15:56         ` Lars Ingebrigtsen
  1 sibling, 1 reply; 16+ messages in thread
From: Thierry Volpiatto @ 2022-08-22 15:45 UTC (permalink / raw)
  To: Lars Ingebrigtsen; +Cc: 57334

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


Lars Ingebrigtsen <larsi@gnus.org> writes:

> Thierry Volpiatto <thievol@posteo.net> writes:
>
>>>> +                 (setcdr dired-directory
>
> [...]
>
>> Of course it have to be changed, it has been modified by wdired at this point,
>> so if you want to redisplay a dired buffer reflecting your changes you
>> have to modify it no ?
>
> I don't understand what you mean.

Well, you dired buffer is not the same before and after editing and
saved.

>>> which we shouldn't do.
>>
>> You already do it when DIRNAME is a string isn't it?
>
> Strings can't be modified.

You are speaking of destructive operations, ok, I am just saying you
just modify the string non destructively yes, but you modify it.

>>> (It may even be a constant.)
>>
>> Can you elaborate?
>
> If the list is in purespace, for instance, it can't be modified.

Yes but I hardly see how it would be the case for this.

> In case there's any misunderstanding here, I'm talking about the
> destructive alteration of the list pointed to by dired-directory by that
> `setcdr' -- not the altering of the dired-directory variable.  So the
> safe change here would be something like
>
>   (setq dired-directory (cons (car dired-directory) (mapcar ...)))

Ok, so here a new patch which works same as the previous:

diff --git a/lisp/wdired.el b/lisp/wdired.el
index 106d57174d5..74d05a093b4 100644
--- a/lisp/wdired.el
+++ b/lisp/wdired.el
@@ -537,15 +537,27 @@ non-nil means return old filename."
     (wdired-change-to-dired-mode)
     (if changes
 	(progn
-	  ;; If we are displaying a single file (rather than the
-	  ;; contents of a directory), change dired-directory if that
-	  ;; file was renamed.  (This ought to be generalized to
-	  ;; handle the multiple files case, but that's less trivial).
-	  (when (and (stringp dired-directory)
-		     (not (file-directory-p dired-directory))
-		     (null some-file-names-unchanged)
-		     (= (length files-renamed) 1))
-	    (setq dired-directory (cdr (car files-renamed))))
+          (setq dired-directory
+	        (cond (;; If we are displaying a single file (rather than the
+	               ;; contents of a directory), change dired-directory if that
+	               ;; file was renamed.
+                       (and (stringp dired-directory)
+                            (not (file-directory-p dired-directory))
+                            (null some-file-names-unchanged)
+                            (= (length files-renamed) 1))
+                       (cdr (car files-renamed)))
+                      ;; Fix dired buffers created with
+                      ;; (dired '(foo f1 f2 f3)).
+                      ((and (consp dired-directory)
+                            (cdr dired-directory)
+                            files-renamed)
+                       (cons (car dired-directory)
+                             ;; Replace in `dired-directory' files that have
+                             ;; been modified with their new name keeping
+                             ;; the ones that are unmodified at the same place.
+                             (cl-loop for f in (cdr dired-directory)
+                                      collect (or (assoc-default f files-renamed)
+                                                  f))))))
 	  ;; Re-sort the buffer.
 	  (revert-buffer)
 	  (let ((inhibit-read-only t))

-- 
Thierry

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

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

* bug#57334: 28.1; Fix wdired with (dired '(dir f1 f2 ...))
  2022-08-22 15:45       ` Thierry Volpiatto
@ 2022-08-22 15:56         ` Lars Ingebrigtsen
  2022-08-22 18:46           ` Thierry Volpiatto
  0 siblings, 1 reply; 16+ messages in thread
From: Lars Ingebrigtsen @ 2022-08-22 15:56 UTC (permalink / raw)
  To: Thierry Volpiatto; +Cc: 57334

Thierry Volpiatto <thievol@posteo.net> writes:

> Ok, so here a new patch which works same as the previous:

Thanks.  But this leads to the following test failures:

3 unexpected results:
   FAILED  wdired-test-bug32173-01
   FAILED  wdired-test-bug32173-02
   FAILED  wdired-test-bug34915






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

* bug#57334: 28.1; Fix wdired with (dired '(dir f1 f2 ...))
  2022-08-22 15:56         ` Lars Ingebrigtsen
@ 2022-08-22 18:46           ` Thierry Volpiatto
  2022-08-22 18:49             ` Lars Ingebrigtsen
  0 siblings, 1 reply; 16+ messages in thread
From: Thierry Volpiatto @ 2022-08-22 18:46 UTC (permalink / raw)
  To: Lars Ingebrigtsen; +Cc: 57334

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


Lars Ingebrigtsen <larsi@gnus.org> writes:

> Thierry Volpiatto <thievol@posteo.net> writes:
>
>> Ok, so here a new patch which works same as the previous:
>
> Thanks.  But this leads to the following test failures:
>
> 3 unexpected results:
>    FAILED  wdired-test-bug32173-01
>    FAILED  wdired-test-bug32173-02
>    FAILED  wdired-test-bug34915

Sorry but I am not a big fan of ERT, I have no idea what these tests do.

-- 
Thierry

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

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

* bug#57334: 28.1; Fix wdired with (dired '(dir f1 f2 ...))
  2022-08-22 18:46           ` Thierry Volpiatto
@ 2022-08-22 18:49             ` Lars Ingebrigtsen
  2022-08-22 20:05               ` Thierry Volpiatto
  2022-08-23  4:50               ` Thierry Volpiatto
  0 siblings, 2 replies; 16+ messages in thread
From: Lars Ingebrigtsen @ 2022-08-22 18:49 UTC (permalink / raw)
  To: Thierry Volpiatto; +Cc: 57334

Thierry Volpiatto <thievol@posteo.net> writes:

>> 3 unexpected results:
>>    FAILED  wdired-test-bug32173-01
>>    FAILED  wdired-test-bug32173-02
>>    FAILED  wdired-test-bug34915
>
> Sorry but I am not a big fan of ERT, I have no idea what these tests do.

If you say "make wdired-tests" in the "test" directory, the output will
tell you what the problem is.  (I didn't examine the output myself.)





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

* bug#57334: 28.1; Fix wdired with (dired '(dir f1 f2 ...))
  2022-08-22 18:49             ` Lars Ingebrigtsen
@ 2022-08-22 20:05               ` Thierry Volpiatto
  2022-08-23  4:50               ` Thierry Volpiatto
  1 sibling, 0 replies; 16+ messages in thread
From: Thierry Volpiatto @ 2022-08-22 20:05 UTC (permalink / raw)
  To: Lars Ingebrigtsen; +Cc: 57334

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


Lars Ingebrigtsen <larsi@gnus.org> writes:

> Thierry Volpiatto <thievol@posteo.net> writes:
>
>>> 3 unexpected results:
>>>    FAILED  wdired-test-bug32173-01
>>>    FAILED  wdired-test-bug32173-02
>>>    FAILED  wdired-test-bug34915
>>
>> Sorry but I am not a big fan of ERT, I have no idea what these tests do.
>
> If you say "make wdired-tests" in the "test" directory, the output will
> tell you what the problem is.  (I didn't examine the output myself.)

The output is unuseful => (wrong-type-argument stringp ni).
No backtrace, so we don't know what the error could be.
Thus I guess the tests are written with the possible existing bugs in
mind and behave with these bugs behavior (not sure if I explain myself
correctly sorry).

-- 
Thierry

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

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

* bug#57334: 28.1; Fix wdired with (dired '(dir f1 f2 ...))
  2022-08-22  6:23 bug#57334: 28.1; Fix wdired with (dired '(dir f1 f2 ...)) Thierry Volpiatto
  2022-08-22 10:27 ` Lars Ingebrigtsen
  2022-08-22 11:26 ` Eli Zaretskii
@ 2022-08-23  1:01 ` Michael Heerdegen
  2 siblings, 0 replies; 16+ messages in thread
From: Michael Heerdegen @ 2022-08-23  1:01 UTC (permalink / raw)
  To: Thierry Volpiatto; +Cc: 57334

Thierry Volpiatto <thievol@posteo.net> writes:

> Hello Emacs,
>
> there is an action in helm that allows creating a dired buffer with
> marked files for further editing with wdired.
> For this I have to call dired with its dirname argument as a list:
>   (dired ' (dir f1 f2 f3))
> Unfortunately this is broken since years and until now I had to use an advice
> to fix it.
> The advice is working up to emacs-28.1 but now it becomes difficult to
> write an advice compatible with all emacs versions, here is a patch to
> apply on 29.0.50.

I do also have such an advice.  My approach is to make
`wdired-finish-edit' _not_ call `revert-buffer'.  This is faster and
doesn't mess the original file order.  AFAIR one also has to avoid the
`dired-advertize' call for such buffers, else they are confused with
normal buffers displayed the DIR.  And file changes are not propagated
to other dired buffers, that's also something I do manually.

And a different thing is that changes in other dired buffers are not
propagated to these special file-list buffers.  This is a tricky matter.
I think fixing all problems will require some work.  They are not
independent.

Michael.





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

* bug#57334: 28.1; Fix wdired with (dired '(dir f1 f2 ...))
  2022-08-22 18:49             ` Lars Ingebrigtsen
  2022-08-22 20:05               ` Thierry Volpiatto
@ 2022-08-23  4:50               ` Thierry Volpiatto
  2022-08-23 10:04                 ` Lars Ingebrigtsen
  1 sibling, 1 reply; 16+ messages in thread
From: Thierry Volpiatto @ 2022-08-23  4:50 UTC (permalink / raw)
  To: Lars Ingebrigtsen; +Cc: 57334

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


Lars Ingebrigtsen <larsi@gnus.org> writes:

> Thierry Volpiatto <thievol@posteo.net> writes:
>
>>> 3 unexpected results:
>>>    FAILED  wdired-test-bug32173-01
>>>    FAILED  wdired-test-bug32173-02
>>>    FAILED  wdired-test-bug34915
>>
>> Sorry but I am not a big fan of ERT, I have no idea what these tests do.
>
> If you say "make wdired-tests" in the "test" directory, the output will
> tell you what the problem is.  (I didn't examine the output myself.)

The tests succeed with following patch (avoid setting dired-directory to
nil).

diff --git a/lisp/wdired.el b/lisp/wdired.el
index 106d57174d5..40008f186eb 100644
--- a/lisp/wdired.el
+++ b/lisp/wdired.el
@@ -537,15 +537,27 @@ non-nil means return old filename."
     (wdired-change-to-dired-mode)
     (if changes
 	(progn
-	  ;; If we are displaying a single file (rather than the
-	  ;; contents of a directory), change dired-directory if that
-	  ;; file was renamed.  (This ought to be generalized to
-	  ;; handle the multiple files case, but that's less trivial).
-	  (when (and (stringp dired-directory)
-		     (not (file-directory-p dired-directory))
-		     (null some-file-names-unchanged)
-		     (= (length files-renamed) 1))
-	    (setq dired-directory (cdr (car files-renamed))))
+	  (cond (;; If we are displaying a single file (rather than the
+	         ;; contents of a directory), change dired-directory if that
+	         ;; file was renamed.
+                 (and (stringp dired-directory)
+                      (not (file-directory-p dired-directory))
+                      (null some-file-names-unchanged)
+                      (= (length files-renamed) 1))
+                 (setq dired-directory (cdr (car files-renamed))))
+                ;; Fix dired buffers created with
+                ;; (dired '(foo f1 f2 f3)).
+                ((and (consp dired-directory)
+                      (cdr dired-directory)
+                      files-renamed)
+                 (setq dired-directory
+                       (cons (car dired-directory)
+                             ;; Replace in `dired-directory' files that have
+                             ;; been modified with their new name keeping
+                             ;; the ones that are unmodified at the same place.
+                             (cl-loop for f in (cdr dired-directory)
+                                      collect (or (assoc-default f files-renamed)
+                                                  f))))))
 	  ;; Re-sort the buffer.
 	  (revert-buffer)
 	  (let ((inhibit-read-only t))

-- 
Thierry

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

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

* bug#57334: 28.1; Fix wdired with (dired '(dir f1 f2 ...))
  2022-08-23  4:50               ` Thierry Volpiatto
@ 2022-08-23 10:04                 ` Lars Ingebrigtsen
  2022-08-23 10:53                   ` Thierry Volpiatto
  0 siblings, 1 reply; 16+ messages in thread
From: Lars Ingebrigtsen @ 2022-08-23 10:04 UTC (permalink / raw)
  To: Thierry Volpiatto; +Cc: 57334

Thierry Volpiatto <thievol@posteo.net> writes:

> The tests succeed with following patch (avoid setting dired-directory to
> nil).

Michael noted that there's more fundamental problems in this area, so
perhaps more work is needed later, but your change makes sense anyway, I
think, so I've now pushed it to Emacs 29.





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

* bug#57334: 28.1; Fix wdired with (dired '(dir f1 f2 ...))
  2022-08-23 10:04                 ` Lars Ingebrigtsen
@ 2022-08-23 10:53                   ` Thierry Volpiatto
  0 siblings, 0 replies; 16+ messages in thread
From: Thierry Volpiatto @ 2022-08-23 10:53 UTC (permalink / raw)
  To: Lars Ingebrigtsen; +Cc: 57334

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


Lars Ingebrigtsen <larsi@gnus.org> writes:

> Thierry Volpiatto <thievol@posteo.net> writes:
>
>> The tests succeed with following patch (avoid setting dired-directory to
>> nil).
>
> Michael noted that there's more fundamental problems in this area,

Indeed there is other problems, like Michael mentionned but also Eli.

I had also problems with `wdired-get-filename` which at the end use
(concat dir file) instead of (expand-file-name file dir).
If filename is absolute we endup with
e.g. "/home/you/tmp//home/you/tmp/foo.txt" instead of
"/home/you/tmp/foo.txt".
Curiously this happen in emacs-28 but not in 29 (why I submitted no
patch for this).

> so perhaps more work is needed later, but your change makes sense
> anyway, I think, so I've now pushed it to Emacs 29.

Great thanks.

-- 
Thierry

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

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

end of thread, other threads:[~2022-08-23 10:53 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-08-22  6:23 bug#57334: 28.1; Fix wdired with (dired '(dir f1 f2 ...)) Thierry Volpiatto
2022-08-22 10:27 ` Lars Ingebrigtsen
2022-08-22 14:42   ` Thierry Volpiatto
2022-08-22 14:54     ` Lars Ingebrigtsen
2022-08-22 14:59       ` Lars Ingebrigtsen
2022-08-22 15:45       ` Thierry Volpiatto
2022-08-22 15:56         ` Lars Ingebrigtsen
2022-08-22 18:46           ` Thierry Volpiatto
2022-08-22 18:49             ` Lars Ingebrigtsen
2022-08-22 20:05               ` Thierry Volpiatto
2022-08-23  4:50               ` Thierry Volpiatto
2022-08-23 10:04                 ` Lars Ingebrigtsen
2022-08-23 10:53                   ` Thierry Volpiatto
2022-08-22 11:26 ` Eli Zaretskii
2022-08-22 14:51   ` Thierry Volpiatto
2022-08-23  1:01 ` Michael Heerdegen

Code repositories for project(s) associated with this public inbox

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

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).