Re: [ELPA] New package: disk-usage

[Michael Albinus <michael.albinus@gmx.de>]

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

Pierre Neidhardt <mail@ambrevar.xyz> writes:

Hi Pierre,

> I've released 1.2.0.

Thanks. I needed to apply the following patch to make it work on remote
systems. In my local environment, I could test it on "/sudo::"; it makes
a difference because "/root" is readable only for user "root".


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

*** /home/albinus/.emacs.d/elpa/disk-usage-1.2.0/disk-usage.el.~1~	2019-02-26 11:50:08.976859553 +0100
--- /home/albinus/.emacs.d/elpa/disk-usage-1.2.0/disk-usage.el	2019-02-26 12:21:11.661869542 +0100
***************
*** 290,298 ****
    "This is the equivalent of running the shell command
  $ find . -type f -exec du -sb {} +"
    (setq directory (or directory default-directory))
!   (let ((pair-strings (split-string (with-temp-buffer
                             (process-file disk-usage--find-command nil '(t nil) nil
!                                          directory
                                           "-type" "f"
                                           "-exec"
                                           disk-usage--du-command
--- 290,299 ----
    "This is the equivalent of running the shell command
  $ find . -type f -exec du -sb {} +"
    (setq directory (or directory default-directory))
!   (let* ((default-directory directory)
! 	 (pair-strings (split-string (with-temp-buffer
                             (process-file disk-usage--find-command nil '(t nil) nil
!                                          (file-local-name directory)
                                           "-type" "f"
                                           "-exec"
                                           disk-usage--du-command
***************
*** 301,307 ****
                                      "\n" 'omit-nulls)))
      (cl-loop for pair-string in pair-strings
               for pair = (split-string pair-string "\t")
!              for name = (cadr pair)
               for attributes = (file-attributes name)
               when (cl-loop for filter in disk-usage-filters
                             always (funcall filter name attributes))
--- 302,308 ----
                                      "\n" 'omit-nulls)))
      (cl-loop for pair-string in pair-strings
               for pair = (split-string pair-string "\t")
!              for name = (concat (file-remote-p directory) (cadr pair))
               for attributes = (file-attributes name)
               when (cl-loop for filter in disk-usage-filters
                             always (funcall filter name attributes))
***************
*** 351,357 ****
         (with-output-to-string
           (process-file disk-usage--du-command
                         nil '(t nil) nil
!                        disk-usage--du-args path))
         (buffer-string))))))
  
  (defun disk-usage--sort-by-size (a b)
--- 352,358 ----
         (with-output-to-string
           (process-file disk-usage--du-command
                         nil '(t nil) nil
!                        disk-usage--du-args (file-local-name path)))
         (buffer-string))))))
  
  (defun disk-usage--sort-by-size (a b)

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


There are further things to be improved:

* In `disk-usage--list-recursively', you make a loop over

             for attributes = (file-attributes name)

  This is slow, at least on remote systems, because you apply
  `file-attributes' on every file on a possibly large list. Maybe you
  could call `directory-files-and-attributes' per directory, instead.
  
* As said before, the commands you apply might vary on different
  hosts. For example, on my QNAP NAS server, getting du info on "/tmp"
  with "find /tmp -type f -exec du -sb \{\} \+" does not work. It must
  be "find /tmp -type f | xargs du -sb" instead. So the commands must be
  configurable per host. See for example, how `grep-host-defaults-alist'
  is computed via `grep-compute-defaults' in grep.el.
  
Best regards, Michael.