recentf new feature proposal

recentf new feature proposal

[David PONCE]

Hi,

I submit you the following patch for recentf.el that introduces a new
feature to restore the last visited position of recently opened files.

Here is the change log:

2003-06-06  David Ponce  <david@dponce.com>

	* recentf.el

	Add new option to restore point when opening a recent file.
	
	(recentf-positions): New variable.
	(recentf-restore-position-flag): New option.
	(recentf-get-position, recentf-save-position)
	(recentf-sync-positions, recentf-open-file): New functions.
	(recentf-make-menu-item): Use `recentf-open-file'.
	(recentf-track-closed-file): Save position in current buffer.
	(recentf-open-files-action): Use `recentf-open-file'.
	(recentf-save-list): Save positions of recently opened file.

It would be nice if that enhancement could be included in Emacs.

Sincerely,
David

Index: lisp/recentf.el
===================================================================
RCS file: /cvsroot/emacs/emacs/lisp/recentf.el,v
retrieving revision 1.24
diff -c -r1.24 recentf.el
*** lisp/recentf.el	2 May 2003 12:13:14 -0000	1.24
--- lisp/recentf.el	6 Jun 2003 10:47:39 -0000
***************
*** 49,54 ****
--- 49,57 ----
  (defvar recentf-list nil
    "List of recently opened files.")
  
+ (defvar recentf-positions nil
+   "List of positions in recently opened files.")
+ 
  (defvar recentf-data-cache nil
    "Cache of data used to build the recentf menu.
  The menu is rebuilt when this data has changed.")
***************
*** 236,241 ****
--- 239,249 ----
             ;; Unavailable until recentf has been loaded.
             (recentf-auto-cleanup))))
  
+ (defcustom recentf-restore-position-flag t
+   "*non-nil means to restore last visited position of opened files."
+   :group 'recentf
+   :type 'boolean)
+ 
  (defcustom recentf-load-hook nil
     "*Normal hook run at end of loading the `recentf' package."
    :group 'recentf
***************
*** 351,356 ****
--- 359,376 ----
        (setq rl (cdr rl)))
      (null rl)))
  
+ (defsubst recentf-get-position (file)
+   "Return the last visited position of FILE."
+   (assoc file recentf-positions))
+ 
+ (defun recentf-save-position ()
+   "Save the point in current buffer as file last visited position."
+   (let* ((filename (recentf-expand-file-name buffer-file-name))
+          (position (recentf-get-position filename)))
+     (if position
+         (setcdr position (point))
+       (push (cons filename (point)) recentf-positions))))
+ 
  (defsubst recentf-add-file (filename)
    "Add or move FILENAME at the beginning of the recent list.
  Does nothing it if it matches any of the `recentf-exclude' regexps."
***************
*** 366,371 ****
--- 386,399 ----
                (recentf-expand-file-name filename) recentf-list)))
        (and m (setq recentf-list (delq (car m) recentf-list))))))
  
+ (defun recentf-sync-positions ()
+   "Synchronize recent positions with the recent list."
+   (let (position positions)
+     (dolist (f recentf-list)
+       (when (setq position (recentf-get-position f))
+         (push position positions)))
+     (setq recentf-positions (nreverse positions))))
+ 
  (defun recentf-find-file (filename)
    "Edit file FILENAME using `find-file'.
  If the file does not exist or is non readable, and
***************
*** 386,391 ****
--- 414,431 ----
          (recentf-string-lessp (file-name-nondirectory f1)
                                (file-name-nondirectory f2))
        (recentf-string-lessp d1 d2))))
+ 
+ (defun recentf-open-file (file)
+   "Open FILE and move point to the last visited position."
+   (let ((visited (find-buffer-visiting file))
+         position)
+     (if visited
+         (switch-to-buffer visited)
+       (funcall recentf-menu-action file)
+       (when recentf-restore-position-flag
+         (setq position (recentf-get-position file))
+         (when (number-or-marker-p (cdr position))
+           (goto-char (cdr position)))))))
  \f
  ;;; Menu building
  ;;
***************
*** 516,522 ****
          (value (recentf-menu-element-value elt)))
      (if (recentf-sub-menu-element-p elt)
          (cons item (mapcar 'recentf-make-menu-item value))
!       (vector item (list recentf-menu-action value)
                :help (concat "Open " value)
                :active t))))
  
--- 556,562 ----
          (value (recentf-menu-element-value elt)))
      (if (recentf-sub-menu-element-p elt)
          (cons item (mapcar 'recentf-make-menu-item value))
!       (vector item (list 'recentf-open-file value)
                :help (concat "Open " value)
                :active t))))
  
***************
*** 905,913 ****
    "Update the recent list when a buffer is killed.
  That is, remove a non readable file from the recent list, if
  `recentf-keep-non-readable-files-flag' is nil."
!   (and buffer-file-name
!        (not recentf-keep-non-readable-files-flag)
!        (recentf-remove-if-non-readable buffer-file-name)))
  
  (defun recentf-update-menu ()
    "Update the recentf menu from the current recent list."
--- 945,954 ----
    "Update the recent list when a buffer is killed.
  That is, remove a non readable file from the recent list, if
  `recentf-keep-non-readable-files-flag' is nil."
!   (when buffer-file-name
!     (unless (and (not recentf-keep-non-readable-files-flag)
!                  (recentf-remove-if-non-readable buffer-file-name))
!       (recentf-save-position))))
  
  (defun recentf-update-menu ()
    "Update the recentf menu from the current recent list."
***************
*** 1019,1025 ****
  Used internally by `recentf-open-files'.
  IGNORE other arguments."
    (kill-buffer (current-buffer))
!   (funcall recentf-menu-action (widget-value widget)))
  
  (defvar recentf-open-files-item-shift ""
    "Amount of space to shift right sub-menu items.
--- 1060,1066 ----
  Used internally by `recentf-open-files'.
  IGNORE other arguments."
    (kill-buffer (current-buffer))
!   (recentf-open-file (widget-value widget)))
  
  (defvar recentf-open-files-item-shift ""
    "Amount of space to shift right sub-menu items.
***************
*** 1106,1115 ****
--- 1147,1158 ----
    "Save the recent list.
  Write data into the file specified by `recentf-save-file'."
    (interactive)
+   (recentf-sync-positions)
    (with-temp-file (expand-file-name recentf-save-file)
      (erase-buffer)
      (insert (format recentf-save-file-header (current-time-string)))
      (recentf-dump-variable 'recentf-list recentf-max-saved-items)
+     (recentf-dump-variable 'recentf-positions recentf-max-saved-items)
      (recentf-dump-variable 'recentf-filter-changer-state)
      nil))
  
Index: etc/NEWS
===================================================================
RCS file: /cvsroot/emacs/emacs/etc/NEWS,v
retrieving revision 1.823
diff -c -r1.823 NEWS
*** etc/NEWS	5 Jun 2003 23:56:32 -0000	1.823
--- etc/NEWS	6 Jun 2003 10:47:40 -0000
***************
*** 129,134 ****
--- 129,138 ----
  
  ** recentf changes.
  
+ Depending on the new option `recentf-restore-position-flag', after a
+ recent file is opened, the point is moved to the last position it was
+ on.  By default, last positions are restored.
+ 
  The recent file list is now automatically cleanup when recentf mode is
  enabled.  The new option `recentf-auto-cleanup' controls when to do
  automatic cleanup.

Re: recentf new feature proposal

[Juanma Barranquero]

On Fri,  6 Jun 2003 13:11:23 +0200 (CEST)
David PONCE <david.ponce@wanadoo.fr> wrote:

> I submit you the following patch for recentf.el that introduces a new
> feature to restore the last visited position of recently opened files.

AFAIK, saveplace and recentf already do interact correctly... Why should
it be necessary to add saveplace-like functionality to recentf?

                                                                Juanma

Re: recentf new feature proposal

[David PONCE]

Hi Juanma,

> AFAIK, saveplace and recentf already do interact correctly... Why
> should it be necessary to add saveplace-like functionality to
> recentf?

This is a good point.  I didn't know that saveplace already provided
that feature :-(

However, from a user point of view, it could be more convenient to use
only one library to open last visited files at last visited position.

Also, that will reduce the number of hooks used, and the number of
session related files ;-)

recentf could check if saveplace is enabled, and not restore the point in that case.

Maybe other people have different thoughts?

Anyway, feel free to not install the patch if you don't like it.

David

Re: recentf new feature proposal

[Juanma Barranquero]

On Fri,  6 Jun 2003 15:54:55 +0200 (CEST), David PONCE <david.ponce@wanadoo.fr> wrote:

> This is a good point.  I didn't know that saveplace already provided
> that feature :-(

Ah. Take a look at it. I use it all the time.

> However, from a user point of view, it could be more convenient to use
> only one library to open last visited files at last visited position.

Hmm.

recentf offers a way to (re)visit files; saveplace offers a way to save
yourself from remembering where you were in the files (and the pain to
go back to the right spot). Both behaviors are orthogonal (although
vaguely related).

IMO, if you use recentf and not saveplace, likely you don't want to
restore the point anywhere. If you use recentf and want to restore the
point for revisited files, likely saveplace's functionality will
interest you even if you visit your files by other methods than recentf...

> Also, that will reduce the number of hooks used, and the number of
> session related files ;-)

saveplace.el[c] is quite small.

> recentf could check if saveplace is enabled, and not restore the point in that case.

What worries me is duplicating this kind of functionality. There are
already other modules who ease visiting files... Should every one of
them implement point restoring?

> 
> Maybe other people have different thoughts?

Perhaps it'd be posible to extend (a little) saveplace.el so you can
better customize when it should activate; this way, people who want to
use saveplace-like functionality outside recentf-loaded files but not
want if for recentf or viceversa could customize it to his heart's
content.

> Anyway, feel free to not install the patch if you don't like it.

I feel it's unnecessary. Other than that, I've got nothing to say.

                                                           /L/e/k/t/u

Re: recentf new feature proposal

[Stefan Monnier]

> What worries me is duplicating this kind of functionality. There are

Agreed.  Duplication is bad.
I suggest David try and use saveplace.el for a while and if something's
amiss with it, he should try and improve it so that everybody benefits.

As for using several files, rather than a single one, it might be a good
point, but that sounds like an argument to get recentf and saveplace
to better cooperate rather than to reimplement saveplace in recentf.


	Stefan

Re: recentf new feature proposal

[David Ponce]

>>What worries me is duplicating this kind of functionality. There are
> 
> 
> Agreed.  Duplication is bad.
> I suggest David try and use saveplace.el for a while and if something's
> amiss with it, he should try and improve it so that everybody benefits.

Good suggestion!  I will do that and see...

> As for using several files, rather than a single one, it might be a good
> point, but that sounds like an argument to get recentf and saveplace
> to better cooperate rather than to reimplement saveplace in recentf.

That make sense.  May be I will be able to suggest some enhancement after
using the two libraries for a while ;-)

Thanks for your feedback!

David