bug#62413: 29.0.60; [PATCH] save-place-mode cannot restore saved position

[Liu Hui <liuhui1610@gmail.com>]

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

Eli Zaretskii <eliz@gnu.org> 于2023年3月30日周四 13:33写道：
>
> I think there's a cleaner way: a defcustom can have a :set function,
> which is called each time the variable is customized; this setter
> function should be defined for a defcustom when changing its value has
> non-trivial effects.  So we can define such a setter function to
> rewrite the list, and document in the doc string of the defcustom that
> users should not just set the value with setq, but instead use either
> setopt or "M-x customize-variable".  WDYT?

OK, I think it is good. Please see the attached patch.

[-- Attachment #2: 0001-Restore-positions-reliably-for-abbreviated-file-name.patch --]
[-- Type: text/x-patch, Size: 5744 bytes --]

From 89fba3250a41ed956b36414912b7fac72450c5f3 Mon Sep 17 00:00:00 2001
From: Liu Hui <liuhui1610@gmail.com>
Date: Mon, 3 Apr 2023 08:47:11 +0800
Subject: [PATCH] Restore positions reliably for abbreviated file names in
 saveplace.el

* lisp/saveplace.el (save-place-abbreviate-file-names): Add setter
function for rewriting `save-place-alist'.  Update docstring.
(save-place-mode): Make sure `save-place-alist' is loaded.
(save-place-to-alist): Save Abbreviated dired-filename.
(save-place-find-file-hook):
(save-place-dired-hook): Use abbreviated file name when
`save-place-abbreviate-file-names' is non-nil.
---
 lisp/saveplace.el | 79 ++++++++++++++++++++++++++++++++++-------------
 1 file changed, 57 insertions(+), 22 deletions(-)

diff --git a/lisp/saveplace.el b/lisp/saveplace.el
index 7512fc87c5d..543c72ee511 100644
--- a/lisp/saveplace.el
+++ b/lisp/saveplace.el
@@ -35,6 +35,8 @@

 ;;; Code:

+(require 'cl-lib)
+
 ;; this is what I was using during testing:
 ;; (define-key ctl-x-map "p" 'toggle-save-place-globally)

@@ -90,8 +92,32 @@ save-place-forget-unreadable-files
 (defcustom save-place-abbreviate-file-names nil
   "If non-nil, abbreviate file names before saving them.
 This can simplify sharing the `save-place-file' file across
-different hosts."
+different hosts.
+
+Changing this option requires rewriting `save-place-alist' with
+corresponding file name format, therefore setting this option
+just using `setq' may cause out-of-sync problems.  You should
+first turn on `save-place-mode' to load `save-place-alist', and
+then use either `setopt' or M-x customize-variable to set this
+option."
   :type 'boolean
+  :set (lambda (sym val)
+         (set-default sym val)
+         (let ((fun (if val 'abbreviate-file-name 'expand-file-name)))
+           (setq save-place-alist
+                 (cl-delete-duplicates
+                  (cl-loop for (k . v) in save-place-alist
+                           collect
+                           (cons (funcall fun k)
+                                 (if (listp v)
+                                     (cl-loop for (k1 . v1) in v
+                                              collect
+                                              (cons k1 (funcall fun v1)))
+                                   v)))
+                  :key #'car
+                  :from-end t
+                  :test #'equal)))
+         val)
   :version "28.1")

 (defcustom save-place-save-skipped t
@@ -153,6 +179,7 @@ save-place-mode
 where it was when you previously visited the same file."
   :global t
   :group 'save-place
+  (or save-place-loaded (save-place-load-alist-from-file))
   (save-place--setup-hooks save-place-mode))

 (make-variable-buffer-local 'save-place-mode)
@@ -214,7 +241,11 @@ save-place-to-alist
 			    ((and (derived-mode-p 'dired-mode) directory)
 			     (let ((filename (dired-get-filename nil t)))
 			       (if filename
-				   `((dired-filename . ,filename))
+                                   (list
+                                    (cons 'dired-filename
+                                          (if save-place-abbreviate-file-names
+                                              (abbreviate-file-name filename)
+                                            filename)))
 				 (point))))
 			    (t (point)))))
         (if cell
@@ -353,7 +384,11 @@ save-place-find-file-hook
   "Function added to `find-file-hook' by `save-place-mode'.
 It runs the hook `save-place-after-find-file-hook'."
   (or save-place-loaded (save-place-load-alist-from-file))
-  (let ((cell (assoc buffer-file-name save-place-alist)))
+  (let ((cell (and (stringp buffer-file-name)
+                   (assoc (if save-place-abbreviate-file-names
+                              (abbreviate-file-name buffer-file-name)
+                            buffer-file-name)
+                          save-place-alist))))
     (if cell
 	(progn
 	  (or revert-buffer-in-progress-p
@@ -368,25 +403,25 @@ save-place-find-file-hook
 (defun save-place-dired-hook ()
   "Position the point in a Dired buffer."
   (or save-place-loaded (save-place-load-alist-from-file))
-  (let* ((directory (and (derived-mode-p 'dired-mode)
-                         (boundp 'dired-subdir-alist)
-			 dired-subdir-alist
-			 (dired-current-directory)))
-	 (cell (assoc (and directory
-			   (expand-file-name (if (consp directory)
-						 (car directory)
-					       directory)))
-		      save-place-alist)))
-    (if cell
-        (progn
-          (or revert-buffer-in-progress-p
-              (cond
-	       ((integerp (cdr cell))
-		(goto-char (cdr cell)))
-	       ((and (listp (cdr cell)) (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-mode t)))))
+  (when-let ((directory (and (derived-mode-p 'dired-mode)
+                             (boundp 'dired-subdir-alist)
+			     dired-subdir-alist
+			     (dired-current-directory)))
+             (item (expand-file-name (if (consp directory)
+					 (car directory)
+				       directory)))
+	     (cell (assoc (if save-place-abbreviate-file-names
+                              (abbreviate-file-name item) item)
+		          save-place-alist)))
+    (or revert-buffer-in-progress-p
+        (cond
+	 ((integerp (cdr cell))
+	  (goto-char (cdr cell)))
+	 ((listp (cdr cell))
+          (when-let ((elt (assq 'dired-filename (cdr cell))))
+            (dired-goto-file (expand-file-name (cdr elt)))))))
+    ;; and make sure it will be saved again for later
+    (setq save-place-mode t)))

 (defun save-place-kill-emacs-hook ()
   ;; First update the alist.  This loads the old save-place-file if nec.
--
2.25.1