Re: master 1914d94 2/2: Change how Dired displays available space

Re: master 1914d94 2/2: Change how Dired displays available space

[Michael Albinus]

larsi@gnus.org (Lars Ingebrigtsen) writes:

Hi Lars,

>     Change how Dired displays available space
>
>     * doc/emacs/dired.texi (Misc Dired Features): Document it (bug#23812).
>     * lisp/dired.el (dired-free-space): New user option.
>     (dired-insert-directory): Use it from here.
>     (dired--insert-disk-space): New function that uses the user option.
>
>     * lisp/files.el (insert-directory): Don't transform "total" here.
>     * lisp/ls-lisp.el (ls-lisp--insert-directory): Or here.  Instead
>     just leave the "total <num>" bit alone, and let Dired transform it.

Hmm. insert-directory can be used w/o being called from dired. And there
are further implementations of insert-directory, because it uses file
name handlers. So these functions must be adapted as well.

--8<---------------cut here---------------start------------->8---
$ find . -name "*.el" -exec grep --color=auto -i -nH -E 'defun.+-insert-directory' \{\} +
./eshell/em-ls.el:249:(defun eshell-ls--insert-directory
./dired.el:1506:(defun dired-insert-directory (dir switches &optional file-list wildcard hdr)
./net/tramp-smb.el:1047:(defun tramp-smb-handle-insert-directory
./net/tramp-fuse.el:111:(defun tramp-fuse-handle-insert-directory
./net/tramp-sh.el:2534:(defun tramp-sh-handle-insert-directory
./net/ange-ftp.el:4579:(defun ange-ftp-real-insert-directory (&rest args)
./net/ange-ftp.el:4605:(defun ange-ftp-insert-directory (file switches &optional wildcard full)
./net/tramp-archive.el:632:(defun tramp-archive-handle-insert-directory
./net/tramp.el:3787:(defun tramp-handle-insert-directory
./net/tramp-crypt.el:785:(defun tramp-crypt-handle-insert-directory
./find-lisp.el:241:(defun find-lisp-insert-directory (dir
./ls-lisp.el:259:(defun ls-lisp--insert-directory (orig-fun file switches &optional wildcard full-directory-p)
./ls-lisp.el:343:(defun ls-lisp-insert-directory
--8<---------------cut here---------------end--------------->8---

Granted, most of them are from Tramp, but there could be also other
implementations in the wild.

Is there a shiny reason to move this functionality from insert-directory
to dired-insert-directory? A discussion about, or at least a trigger for
this change (aka announcement in etc/NEWS), would have been appreciated.

Best regards, Michael.

Re: master 1914d94 2/2: Change how Dired displays available space

[Lars Ingebrigtsen]

Michael Albinus <michael.albinus@gmx.de> writes:

> Hmm. insert-directory can be used w/o being called from dired. And there
> are further implementations of insert-directory, because it uses file
> name handlers. So these functions must be adapted as well.

My assumption was that (as for the first bit) that not translating the
"total x" thing wouldn't be catastrophic, so that could be adjusted
afterwards.

As for the other implementations, I assumed that they would just
continue to work as is, and if they do that "total" transformation, it
would be trivial to just remove that bit.

If this causes major problems, I'm sorry.

> Granted, most of them are from Tramp, but there could be also other
> implementations in the wild.
>
> Is there a shiny reason to move this functionality from insert-directory
> to dired-insert-directory? A discussion about, or at least a trigger for
> this change (aka announcement in etc/NEWS), would have been appreciated.

I implemented this first as a part of insert-directory.  But the problem
is that it's dired itself that prepends the /current/path/name at the
top, so the thing had to go to dired.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

Re: master 1914d94 2/2: Change how Dired displays available space

[Michael Albinus]

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

Lars Ingebrigtsen <larsi@gnus.org> writes:

Hi Lars,

>> Hmm. insert-directory can be used w/o being called from dired. And there
>> are further implementations of insert-directory, because it uses file
>> name handlers. So these functions must be adapted as well.
>
> My assumption was that (as for the first bit) that not translating the
> "total x" thing wouldn't be catastrophic, so that could be adjusted
> afterwards.

"Afterwards" is in dired-insert-directory? As said, this is not always
in play.

> As for the other implementations, I assumed that they would just
> continue to work as is, and if they do that "total" transformation, it
> would be trivial to just remove that bit.
>
> If this causes major problems, I'm sorry.

Don't know whether it causes major problems. I guess the cases in core
Emacs can be fixed easily. For possible other implementations in the
wild, I don't know.

>> Granted, most of them are from Tramp, but there could be also other
>> implementations in the wild.
>>
>> Is there a shiny reason to move this functionality from insert-directory
>> to dired-insert-directory? A discussion about, or at least a trigger for
>> this change (aka announcement in etc/NEWS), would have been appreciated.
>
> I implemented this first as a part of insert-directory.  But the problem
> is that it's dired itself that prepends the /current/path/name at the
> top, so the thing had to go to dired.

I do not oppose heavily to this change. The advantage is, that the
boring "adjust the free disk space" must not be implemented again and
again in all insert-directory incarnations. But a coordinated change
would have been better ...

Well, what about this change in etc/NEWS:


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: Type: text/x-patch, Size: 458 bytes --]

diff --git a/etc/NEWS b/etc/NEWS
index d783fc019c..304f707f2e 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -741,6 +741,10 @@ with recent versions of Firefox.
 ** The function 'image-dired-get-exif-data' is now obsolete.
 Use 'exif-parse-file' and 'exif-field' instead.

+---
+** 'insert-directory' alternatives should not change the free disk space line.
+This change is now applied in 'dired-insert-directory'.
+
 \f
 * Lisp Changes in Emacs 29.1


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


And I will see what must be done in Tramp and ange-ftp.el

Best regards, Michael.

Re: master 1914d94 2/2: Change how Dired displays available space

[Lars Ingebrigtsen]

Michael Albinus <michael.albinus@gmx.de> writes:

>> My assumption was that (as for the first bit) that not translating the
>> "total x" thing wouldn't be catastrophic, so that could be adjusted
>> afterwards.
>
> "Afterwards" is in dired-insert-directory? As said, this is not always
> in play.

No, as in "after this commit was pushed".  😀

> I do not oppose heavily to this change. The advantage is, that the
> boring "adjust the free disk space" must not be implemented again and
> again in all insert-directory incarnations. But a coordinated change
> would have been better ...

My hope that this change wouldn't actually break anything for anybody,
so whatever glitches appear could be fixed as discovered, so no
coordination would be necessary.

> Well, what about this change in etc/NEWS:

Sure, looks good to me.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

RE: [External] : Re: master 1914d94 2/2: Change how Dired displays available space

[Drew Adams]

> My hope that this change wouldn't actually break
> anything for anybody, so whatever glitches appear
> could be fixed as discovered, so no coordination
> would be necessary.

FWIW, this might well break my code that rewrites
that line with the "total" info (to add the file
count for the given (sub)directory).  Guess I'll
find out after this is released into the wild in
an Emacs release.

Re: master 1914d94 2/2: Change how Dired displays available space

[Michael Albinus]

Lars Ingebrigtsen <larsi@gnus.org> writes:

Hi Lars,

>> Well, what about this change in etc/NEWS:
>
> Sure, looks good to me.

Pushed to master. Tramp changes will follow tomorrow, first I need to
run the regression tests. After all, there will be less changes than
expected.

Best regards, Michael.