Subject: Re: bug#13575: 24.1; dired-mark-sexp misparses directory contents

From: Wolfgang Jenkner <[email-masked]>
Date: Tue, 29 Jan 2013 06:35:44 +0100

On Mon, Jan 28 2013, Wolfgang Jenkner wrote:

> As a first attempt
> to fix this I propose the following barely tested patch.

Or perhaps rather try the patch below, which I tested a bit with GNU and
FreeBSD ls, and also with ls-lisp.el.  It's still WIP but I hope it
works for you.

Please note that the output of "ls -lh" is parsed correctly, but the
unit suffixes are simply ignored for the time being; I'll try to fix
this, though.

Also, specifying "-s" without also passing "-i" to "ls" will do the
wrong thing: the output being ambiguous in this case, a number before
the permission string is (arbitrarily) interpreted as inode number; that
can't be fixed without actually looking at dired-listing-switches.

Wolfgang

diff --git a/lisp/dired-x.el b/lisp/dired-x.el
index a2c1303..5a33479 100644
--- a/lisp/dired-x.el
+++ b/lisp/dired-x.el
@@ -1449,54 +1449,57 @@ to mark all zero length files."
         ;; to nil or the appropriate value, so they need not be initialized.
         ;; Moves point within the current line.
         (dired-move-to-filename)
-        (let (pos
-              (mode-len 10) ; length of mode string
-              ;; like in dired.el, but with subexpressions \1=inode, \2=s:
-              (dired-re-inode-size "\\s *\\([0-9]*\\)\\s *\\([0-9]*\\) ?"))
-          (beginning-of-line)
-          (forward-char 2)
-          (if (looking-at dired-re-inode-size)
-              (progn
-                (goto-char (match-end 0))
-                (setq inode (string-to-number
-                             (buffer-substring (match-beginning 1)
-                                               (match-end 1)))
-                      s (string-to-number
-                         (buffer-substring (match-beginning 2)
-                                           (match-end 2)))))
-            (setq inode nil
-                  s nil))
+        (let ((mode-len 10) ; length of mode string
+	      ;; like in dired.el, but with subexpressions \1=inode, \2=s:
+	      ;; GNU ls -hs suffixes the block count with a unit and
+	      ;; prints it as a float, FreeBSD does neither.
+	      (dired-re-inode-size "\\=\\s *\\([0-9]+\\s +\\)?\\(?:\\([0-9]+\\(?:\\.[0-9]*\\)?[BkKMGTPEZY]?\\)? ?\\)"))
+	  (beginning-of-line)
+	  (forward-char 2)
+	  (search-forward-regexp dired-re-inode-size nil t)
+	  ;; XXX Might be a size without unit.
+	  (setq inode (when (match-string 1)
+			(string-to-number (match-string 1))))
+	  (setq s (when (match-string 2)
+		    ;; XXX Deal with units (for the -h switch).
+		    (string-to-number (match-string 2))))
           (setq mode (buffer-substring (point) (+ mode-len (point))))
           (forward-char mode-len)
           (setq nlink (read (current-buffer)))
           ;; Karsten Wenger <[email-masked]> fixed uid.
-          (setq uid (buffer-substring (1+ (point))
-                                      (progn (forward-word 1) (point))))
-          (re-search-forward directory-listing-before-filename-regexp)
-          (goto-char (match-beginning 1))
-          (forward-char -1)
-          (setq size (string-to-number
-                      (buffer-substring (save-excursion
-                                          (backward-word 1)
-                                          (setq pos (point)))
-                                        (point))))
-          (goto-char pos)
-          (backward-word 1)
-          ;; if no gid is displayed, gid will be set to uid
-          ;; but user will then not reference it anyway in PREDICATE.
-          (setq gid (buffer-substring (save-excursion
-                                        (forward-word 1) (point))
-                                      (point))
-                time (buffer-substring (match-beginning 1)
-                                       (1- (dired-move-to-filename)))
-                name (buffer-substring (point)
-                                       (or
-                                        (dired-move-to-end-of-filename t)
-                                        (point)))
-                sym (if (looking-at " -> ")
-                        (buffer-substring (progn (forward-char 4) (point))
-                                          (line-end-position))
-                      ""))
+	  ;; GNU ls -n right-justifies numerical UIDs and GIDs,
+	  ;; while FreeBSD left-justifies them.  Both of them
+	  ;; right-justify all other numbers.
+          (setq uid (buffer-substring (progn (forward-word 1) (point))
+                                      (progn (forward-word -1) (point))))
+	  (dired-move-to-filename)
+	  (save-excursion
+	    ;; Don't do (forward-char -1) since the regexp search will
+	    ;; then match in the preceding line.
+	    (setq time (buffer-substring (1- (point))
+					 (progn
+					   (re-search-backward directory-listing-before-filename-regexp)
+					   (forward-word 1)
+					   (1+ (point))))
+		  ;; XXX Deal with units (for the -h switch).
+		  size (string-to-number
+			(buffer-substring (point) (progn (forward-word -1)
+							 (when (= (char-before) ?.)
+							   (forward-char -1)
+							   (forward-word -1))
+							 (point))))
+		  ;; if no gid is displayed, gid will be set to uid
+		  ;; but user will then not reference it anyway in PREDICATE.
+		  gid (buffer-substring (progn (forward-word -1) (point))
+					(progn (forward-word 1) (point)))))
+	  (setq name (buffer-substring (point)
+				       (or
+					(dired-move-to-end-of-filename t)
+					(point)))
+		sym (if (looking-at " -> ")
+			(buffer-substring (progn (forward-char 4) (point))
+					  (line-end-position))
+		      ""))
           t)
         (eval predicate)))
      (format "'%s file" predicate))))