Package: emacs;
Reported by: Juri Linkov <juri <at> jurta.org>
Date: Tue, 10 Sep 2013 20:51:02 UTC
Severity: minor
Tags: patch
Done: Juri Linkov <juri <at> jurta.org>
Bug is archived. No further changes may be made.
View this message in rfc822 format
From: Juri Linkov <juri <at> jurta.org> To: Drew Adams <drew.adams <at> oracle.com> Cc: Karl Fogel <kfogel <at> red-bean.com>, 15329 <at> debbugs.gnu.org Subject: bug#15329: saveplace restores dired positions to random places Date: Mon, 16 Dec 2013 22:58:41 +0200
>> - (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))
>
> Is `dired-directory' really the right test here? I am used to seeing
> (derived-mode-p 'dired-mode) for that purpose (assuming I understand the
> purpose here).
Yes, `dired-directory' is the right test, because `dired-directory'
is used as a key in `save-place-alist'.
> There is, BTW, nothing in the doc string of `dired-directory' that says
> what a nil value means.
Thanks for pointing to the doc string of `dired-directory' where I noticed:
May be a list, in which case the car is the
directory name and the cdr is the list of files to mention.
This case should be handled properly that I added to the following patch.
> Should code now instead be using `dired-directory' to test whether the
> mode is (derived from) Dired?
Yes, code should be using `dired-directory' now.
> If so, then at the very least the doc string of that variable should
> describe such a Boolean meaning: nil means not in Dired mode or a mode
> derived from it (or whatever the completely correct interpretation is).
Are you sure that nil means not in Dired mode or a mode derived from it?
Anyway, this is a new version:
=== modified file 'lisp/saveplace.el'
--- lisp/saveplace.el 2013-09-12 05:32:57 +0000
+++ lisp/saveplace.el 2013-12-16 20:56:14 +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.
@@ -170,20 +172,29 @@ (defun save-place-to-alist ()
;; will be saved again when Emacs is killed.
(or save-place-loaded (load-save-place-alist-from-file))
(let ((item (or buffer-file-name
- (and dired-directory (expand-file-name dired-directory)))))
+ (and dired-directory
+ (if (consp dired-directory)
+ (expand-file-name (car dired-directory))
+ (expand-file-name dired-directory))))))
(when (and item
(or (not save-place-ignore-files-regexp)
(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))))
+ (dired-directory
+ (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 +301,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,18 +311,27 @@ (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))
- (let ((cell (assoc (expand-file-name dired-directory) save-place-alist)))
+ (let ((cell (assoc (if (consp dired-directory)
+ (expand-file-name (car dired-directory))
+ (expand-file-name dired-directory))
+ save-place-alist)))
(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)))))
GNU bug tracking system
Copyright (C) 1999 Darren O. Benham,
1997,2003 nCipher Corporation Ltd,
1994-97 Ian Jackson.