bug#72919: 29.1; chart-space-usage in chart.el does not work correctly on windows

[Eli Zaretskii <eliz@gnu.org>]

[Please use Reply All to reply, so that the bug tracker is CC'ed.]

> From: Harm Van der Vegt <harmvegt@gmail.com>
> Date: Sun, 1 Sep 2024 20:11:39 +0200
> 
> >Thanks.  However, I think your changes are not entirely correct: they
> >fail to account for space usage of files inside subdirectories of the
> >directory which the user types at the prompt, whereas the
> >implementation with "du" does account for that.
> 
> That is indeed a pretty major difference, my bad!
> 
> >In addition, I think if someone has 'du' on Windows, it should be used.
> 
> Is there a preference for using system binaries above elisp for cases like these?

I think 'du' has some tricks up its sleeve (like accounting correctly
for block size), which a Lisp implementation can only approximate.  I
don't think minor differences matter here, since this command is just
an illustration of what chart.el can be used for, but I do want to use
'du' if it's available.

> >So I came up with the following changes instead.  Could you please try
> >them, both with and without du.exe on PATH?  If these changes give
> >good results, I will install them.
> 
> Thanks.
> 
> I've tried your changes with and without du. This uncovered something in the
> original implementation, namely that the original implementation did not count
> hidden files and directories. du * skips dotfiles for me.
>
> With du present chart-space-usage shows the lisp directory as the largest in the 
> emacs repository root. Without du it shows .git as the largest.

This just means minor adjustments in the code I posted: we need to use
a different regexp in the call to directory-files-recursively, and
also ignore files that start with a dot in the command itself.  I will
make those changes, thanks for pointing them out

> I'm not sure which output is the wanted output.

I think we want to be as close to 'du' as possible.

> I've attempted to find a shell independent way to have du show dotfiles, but it 
> appears to be rather tricky.

I don't think we want that.

> I've made a new patch that makes the elisp implementation recursive, uses the
> rounding function provided by you and shows dotfiles.

I'd prefer to avoid recursion (in addition to what
directory-files-recursively already does), because that could overflow
the stack.

I will post a version that ignores dotfiles.

Here's the version you sent, repeated for the bug tracker:

>From 6f53d65f9ae5e1c61a2ca2650b149c895ce9794c Mon Sep 17 00:00:00 2001
>From: Harm van der Vegt <harmvegt@gmail.com>
Date: Sun, 1 Sep 2024 20:03:34 +0200
Subject: [PATCH] Make chart-space-usage OS and shell independent

---
 lisp/emacs-lisp/chart.el | 41 +++++++++++++++++++++++++++++------------
 1 file changed, 29 insertions(+), 12 deletions(-)

diff --git a/lisp/emacs-lisp/chart.el b/lisp/emacs-lisp/chart.el
index c195ccb7165..b0302b55278 100644
--- a/lisp/emacs-lisp/chart.el
+++ b/lisp/emacs-lisp/chart.el
@@ -641,27 +641,44 @@ SORT-PRED if desired."
 		       (lambda (a b) (> (cdr a) (cdr b))))
     ))
 
+;; This assumes 4KB blocks
+(defun chart--file-size (size)
+  (* (/ (+ size 4095) 4096) 4096))
+
+(defun chart--directory-size (dir)
+  "Compute total size of files in directory DIR and its subdirectories.
+DIR is assumed to be a directory, verified by the caller."
+  (let ((total-size 0))
+    (dolist (file (directory-files dir t directory-files-no-dot-files-regexp))
+      (cond
+       ((file-regular-p file)
+        (setq total-size (+ total-size (chart--file-size (nth 7 (file-attributes file))))))
+       ((file-directory-p file)
+        (setq total-size (+ total-size (chart--directory-size file))))))
+    total-size))
+
 (defun chart-space-usage (d)
   "Display a top usage chart for directory D."
   (interactive "DDirectory: ")
   (message "Collecting statistics...")
   (let ((nmlst nil)
-        (cntlst nil))
-    (dolist (file (directory-files d t))
-      (when (file-regular-p file)
-        (let ((size (nth 7 (file-attributes file))))
+	(cntlst nil))
+
+    (dolist (file (directory-files d t directory-files-no-dot-files-regexp))
+        (let ((size (if (file-regular-p file)
+                        (nth 7 (file-attributes file))
+                      (chart--directory-size file))))
           (setq nmlst (cons (file-name-nondirectory file) nmlst))
-          (setq cntlst (cons size cntlst)))))
+          (setq cntlst (cons (chart--file-size size) cntlst))))
 
     (if (not nmlst)
-        (error "No files found!"))
-
-    ;; Display the chart if files are found
+	(error "No files found!"))
     (chart-bar-quickie 'vertical (format "Largest files in %s" d)
-                       nmlst "File Name"
-                       cntlst "File Size"
-                       10
-                       (lambda (a b) (> (cdr a) (cdr b))))))
+		       nmlst "File Name"
+		       cntlst "File Size"
+		       10
+		       (lambda (a b) (> (cdr a) (cdr b))))
+    ))
 
 (defun chart-emacs-storage ()
   "Chart the current storage requirements of Emacs."
-- 
2.11.0.windows.3