dired-revert: save line numbers instead of positions

Positions might change if the length of one dired header line
changes; this happen, for instance, if we add new files.
Instead, line numbers are invariant under shrinks/enlargements
of the file header.
https://lists.gnu.org/archive/html/emacs-devel/2017-07/msg01092.html
* lisp/dired.el (dired-save-positions): Save the line numbers at point.
(dired-restore-positions): Use forward-line to restore the original
position (Bug#27968).
* test/lisp/dired-tests.el (dired-test-bug27968): Add test.

2 files changed, 61 insertions, 9 deletions

diff --git a/lisp/dired.el b/lisp/dired.el
index 24759c6c9bd..d04bd6fe037 100644
--- a/lisp/dired.el
+++ b/lisp/dired.el
@@ -1444,18 +1444,22 @@ ARG and NOCONFIRM, passed from `revert-buffer', are ignored."
 The positions have the form (BUFFER-POSITION WINDOW-POSITIONS).
 
 BUFFER-POSITION is the point position in the current Dired buffer.
-It has the form (BUFFER DIRED-FILENAME BUFFER-POINT).
+It has the form (BUFFER DIRED-FILENAME BUFFER-LINE-NUMBER).
 
 WINDOW-POSITIONS are current positions in all windows displaying
 this dired buffer.  The window positions have the form (WINDOW
-DIRED-FILENAME WINDOW-POINT)."
+DIRED-FILENAME WINDOW-LINE-NUMBER).
+
+We store line numbers instead of point positions because the header
+lines might change as well: when this happen the line number doesn't
+change; the point does."
   (list
-   (list (current-buffer) (dired-get-filename nil t) (point))
+   (list (current-buffer) (dired-get-filename nil t) (line-number-at-pos))
    (mapcar (lambda (w)
-             (list w
-                   (with-selected-window w
-                     (dired-get-filename nil t))
-                   (window-point w)))
+             (with-selected-window w
+               (list w
+                     (dired-get-filename nil t)
+                     (line-number-at-pos (window-point w)))))
            (get-buffer-window-list nil 0 t))))
 
 (defun dired-restore-positions (positions)
@@ -1464,7 +1468,8 @@ DIRED-FILENAME WINDOW-POINT)."
          (buffer (nth 0 buf-file-pos)))
     (unless (and (nth 1 buf-file-pos)
                  (dired-goto-file (nth 1 buf-file-pos)))
-      (goto-char (nth 2 buf-file-pos))
+      (goto-char (point-min))
+      (forward-line (1- (nth 2 buf-file-pos)))
       (dired-move-to-filename))
     (dolist (win-file-pos (nth 1 positions))
       ;; Ensure that window still displays the original buffer.
@@ -1472,7 +1477,8 @@ DIRED-FILENAME WINDOW-POINT)."
         (with-selected-window (nth 0 win-file-pos)
           (unless (and (nth 1 win-file-pos)
                        (dired-goto-file (nth 1 win-file-pos)))
-            (goto-char (nth 2 win-file-pos))
+            (goto-char (point-min))
+            (forward-line (1- (nth 2 win-file-pos)))
             (dired-move-to-filename)))))))
 
 (defun dired-remember-marks (beg end)
diff --git a/test/lisp/dired-tests.el b/test/lisp/dired-tests.el
index b14bbc63609..105a79f0014 100644
--- a/test/lisp/dired-tests.el
+++ b/test/lisp/dired-tests.el
@@ -308,5 +308,51 @@
           (should (eq 2 (current-column))))
       (dired-hide-details-mode orig))))
 
+(ert-deftest dired-test-bug27968 ()
+  "Test for http://debbugs.gnu.org/27968 ."
+  (let* ((top-dir (make-temp-file "top-dir" t))
+         (subdir (expand-file-name "subdir" top-dir))
+         (header-len-fn (lambda ()
+                          (save-excursion
+                            (goto-char 1)
+                            (forward-line 1)
+                            (- (point-at-eol) (point)))))
+         orig-len len diff pos line-nb)
+    (make-directory subdir 'parents)
+    (unwind-protect
+        (with-current-buffer (dired-noselect subdir)
+          (setq orig-len (funcall header-len-fn)
+                pos (point)
+                line-nb (line-number-at-pos))
+          ;; Bug arises when the header line changes its length; this may
+          ;; happen if the used space has changed: for instance, with the
+          ;; creation of additional files.
+          (make-directory "subdir" t)
+          (dired-revert)
+          ;; Change the header line.
+          (save-excursion
+            (goto-char 1)
+            (forward-line 1)
+            (let ((inhibit-read-only t))
+              (delete-region (point) (point-at-eol))
+              (insert "  test-bug27968")))
+          (setq len (funcall header-len-fn)
+                diff (- len orig-len))
+          (should-not (zerop diff)) ; Header length has changed.
+          ;; If diff > 0, then the point moves back.
+          ;; If diff < 0, then the point moves forward.
+          ;; If diff = 0, then the point doesn't move.
+          ;; Sometimes this point movement causes
+          ;; line-nb != (line-number-at-pos pos), so that we get
+          ;; an unexpected file at point if we store buffer points.
+          ;; Note that the line number before/after revert
+          ;; doesn't change.
+          (should (= line-nb
+                     (line-number-at-pos)
+                     (line-number-at-pos (+ pos diff))))
+          ;; After revert, the point must be in 'subdir' line.
+          (should (equal "subdir" (dired-get-filename 'local t))))
+      (delete-directory top-dir t))))
+
 (provide 'dired-tests)
 ;; dired-tests.el ends here