Subject: Re: bug#15329: saveplace restores dired positions to random places

From: Juri Linkov <[email-masked]>
Date: Sun, 15 Dec 2013 22:20:04 +0200

> While the current rather random behavior in dired is obviously a bug,
> and fixing it would be a Good Thing, I'm not sure it rises to the level
> where I drop other things to work on it :-).  However, if someone were
> to write a patch (along the lines described in this bug report), I'd
> certainly commit to reviewing it.

I realized now that the current format is completely backward-compatible.
When `find-file-hook' restores the saved position in a file, it doesn't see
the dired entries (that end with a directory separator) in the new format.

As an additional benefit, the users can add a regexp like "/$" to
`save-place-ignore-files-regexp' to not save dired positions.

More packages can do the same later, e.g. Info could save the entries like
"(emacs) Top" with own additional information that only Info will restore, etc.

So now dired positions should be fixed by this patch:

=== modified file 'lisp/saveplace.el'
--- lisp/saveplace.el	2013-09-12 05:32:57 +0000
+++ lisp/saveplace.el	2013-12-15 20:12:27 +0000
@@ -152,8 +152,8 @@ (defun toggle-save-place (&optional parg
 
 \(setq-default save-place t\)"
   (interactive "P")
-  (if (not buffer-file-name)
-      (message "Buffer `%s' not visiting a file" (buffer-name))
+  (if (not (or buffer-file-name dired-directory))
+      (message "Buffer `%s' not visiting a file or directory" (buffer-name))
     (if (and save-place (or (not parg) (<= parg 0)))
 	(progn
 	  (message "No place will be saved in this file")
@@ -161,6 +161,8 @@ (defun toggle-save-place (&optional parg
       (message "Place will be saved")
       (setq save-place t))))
 
+(declare-function dired-get-filename "dired" (&optional localp no-error-if-not-filep))
+
 (defun save-place-to-alist ()
   ;; put filename and point in a cons box and then cons that onto the
   ;; front of the save-place-alist, if save-place is non-nil.
@@ -176,14 +178,20 @@ (defun save-place-to-alist ()
                    (not (string-match save-place-ignore-files-regexp
                                       item))))
       (let ((cell (assoc item save-place-alist))
-            (position (if (not (eq major-mode 'hexl-mode))
-                          (point)
-                        (with-no-warnings
-                          (1+ (hexl-current-address))))))
+            (position (cond ((eq major-mode 'hexl-mode)
+			     (with-no-warnings
+			       (1+ (hexl-current-address))))
+			    ((derived-mode-p 'dired-mode)
+			     (let ((filename (dired-get-filename nil t)))
+			       (if filename
+				   `((dired-filename . ,filename))
+				 (point))))
+			    (t (point)))))
         (if cell
             (setq save-place-alist (delq cell save-place-alist)))
         (if (and save-place
-                 (not (= position 1)))  ;; Optimize out the degenerate case.
+                 (not (and (integerp position)
+			   (= position 1)))) ;; Optimize out the degenerate case.
             (setq save-place-alist
                   (cons (cons item position)
                         save-place-alist)))))))
@@ -290,7 +298,8 @@ (defun save-places-to-alist ()
       (with-current-buffer (car buf-list)
 	;; save-place checks buffer-file-name too, but we can avoid
 	;; overhead of function call by checking here too.
-	(and buffer-file-name (save-place-to-alist))
+	(and (or buffer-file-name dired-directory)
+	     (save-place-to-alist))
 	(setq buf-list (cdr buf-list))))))
 
 (defun save-place-find-file-hook ()
@@ -299,10 +308,13 @@ (defun save-place-find-file-hook ()
     (if cell
 	(progn
 	  (or revert-buffer-in-progress-p
-	      (goto-char (cdr cell)))
+	      (and (integerp (cdr cell))
+		   (goto-char (cdr cell))))
           ;; and make sure it will be saved again for later
           (setq save-place t)))))
 
+(declare-function dired-goto-file "dired" (file))
+
 (defun save-place-dired-hook ()
   "Position the point in a dired buffer."
   (or save-place-loaded (load-save-place-alist-from-file))
@@ -310,7 +322,10 @@ (defun save-place-dired-hook ()
     (if cell
         (progn
           (or revert-buffer-in-progress-p
-              (goto-char (cdr cell)))
+              (if (integerp (cdr cell))
+		  (goto-char (cdr cell))
+		(and (assq 'dired-filename (cdr cell))
+		     (dired-goto-file (cdr (assq 'dired-filename (cdr cell)))))))
           ;; and make sure it will be saved again for later
           (setq save-place t)))))