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

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

[Juri Linkov]

> Remembering dired positions is a good feature, but
> unfortunately in its current implementation is useless.
>
> The problem is that often the position of point in Dired
> is restored to random places because in Dired buffers
> it depends on many irrelevant ever-changing factors
> such as the total size of all files in the directory
> and available space (thus the line with this information
> changes its length), sizes of each file above the restored
> file in the Dired buffer also accounts to the incorrect position
> of point, also added/deleted files in Dired change the restored
> position to random places, different sorting order, etc.
>
> An obvious solution to this problem is to save the
> current file name in Dired instead of its point.
>
> It seems saving information other than point doesn't contradict
> the purpose of saveplace.el that automatically saves places where
> visiting them later automatically moves point to the saved position.
> Also elements of `save-place-alist' are (FILENAME . POSITION),
> not more limited (FILENAME . POINT).  So saving a string to
> POSITION would be consistent with the design of saveplace.el.

I realized this is not backward-compatible change, i.e.
older Emacs versions won't be able to read saved places
in the new format.  Unfortunately, Dired filename positions
can't be added to the current format that uses an alist
with cons pair cells.  I mean (FILENAME . POINT)
can't be changed to (FILENAME POINT DIRED-FILENAME-POSITION)
because older Emacs versions will fail to read them.

The only solution that I see is to save two alists to the places file:

((FILENAME1 . POINT1)
 (FILENAME2 . POINT2)
 ...)
((FILENAME1 DIRED-FILENAME-POSITION1)
 (FILENAME2 DIRED-FILENAME-POSITION2)
 ...)

Then the current code:

  (with-demoted-errors
    (car (read-from-string
      (buffer-substring (point-min) (point-max)))))

will read the first alist without problems,
and additional code in newer versions like

  (with-demoted-errors
    (cadr (read-from-string
      (buffer-substring (point-min) (point-max)))))

will read the second alist with more information
about the context of saved places (like bookmarks).

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

[Karl Fogel]

Juri Linkov wrote:
>> An obvious solution to this problem is to save the
>> current file name in Dired instead of its point.
>>
>> It seems saving information other than point doesn't contradict
>> the purpose of saveplace.el that automatically saves places where
>> visiting them later automatically moves point to the saved position.
>> Also elements of `save-place-alist' are (FILENAME . POSITION),
>> not more limited (FILENAME . POINT).  So saving a string to
>> POSITION would be consistent with the design of saveplace.el.
>
>I realized this is not backward-compatible change, i.e.
>older Emacs versions won't be able to read saved places
>in the new format.  Unfortunately, Dired filename positions
>can't be added to the current format that uses an alist
>with cons pair cells.  I mean (FILENAME . POINT)
>can't be changed to (FILENAME POINT DIRED-FILENAME-POSITION)
>because older Emacs versions will fail to read them.
>
>The only solution that I see is to save two alists to the places file:
>
>((FILENAME1 . POINT1)
> (FILENAME2 . POINT2)
> ...)
>((FILENAME1 DIRED-FILENAME-POSITION1)
> (FILENAME2 DIRED-FILENAME-POSITION2)
> ...)
>
>Then the current code:
>
>  (with-demoted-errors
>    (car (read-from-string
>      (buffer-substring (point-min) (point-max)))))
>
>will read the first alist without problems,
>and additional code in newer versions like
>
>  (with-demoted-errors
>    (cadr (read-from-string
>      (buffer-substring (point-min) (point-max)))))
>
>will read the second alist with more information
>about the context of saved places (like bookmarks).

Well, rather, we could use that as an upgrade strategy for the saveplace
format as a whole.  In other words, starting now and for a few versions
of Emacs into the future, write the old format to the first part of the
file, and then a more flexible new format to the second part.  The
modern format would have a more extensible structure, similarly to how
bookmark.el does it.  Say, a sublist whose first element is the type of
the record, and the rest of which is the data for that record.  Like:

  ((FILE_OR_DIR_NAME_1 ('position (position information goes here)))
   (FILE_OR_DIR_NAME_2 ('dired-position (different kind of information)))
   ...)

etc.

Thoughts?

-Karl

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

[Stefan Monnier]

>   ((FILE_OR_DIR_NAME_1 ('position (position information goes here)))
>    (FILE_OR_DIR_NAME_2 ('dired-position (different kind of information)))
>    ...)

Re-using as much as possible of bookmarks's format would make sense.


        Stefan

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

[Juri Linkov]

> The modern format would have a more extensible structure, similarly to how
> bookmark.el does it.  Say, a sublist whose first element is the type of
> the record, and the rest of which is the data for that record.

Do you think it would be possible to use the existing
infrastructure of bookmark.el, so for instance, to save
a place in an Info manual, saveplace.el could call
`Info-bookmark-make-record' and to restore it with
`Info-bookmark-jump'.  This would be better than adding
a third hook for saveplace (the second existing hook
is desktop-specific like `Info-desktop-buffer-misc-data'
and `Info-restore-desktop-buffer').

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

[Karl Fogel]

Juri Linkov <juri@jurta.org> writes:
>> The modern format would have a more extensible structure, similarly to how
>> bookmark.el does it.  Say, a sublist whose first element is the type of
>> the record, and the rest of which is the data for that record.
>
>Do you think it would be possible to use the existing
>infrastructure of bookmark.el, so for instance, to save
>a place in an Info manual, saveplace.el could call
>`Info-bookmark-make-record' and to restore it with
>`Info-bookmark-jump'.  This would be better than adding
>a third hook for saveplace (the second existing hook
>is desktop-specific like `Info-desktop-buffer-misc-data'
>and `Info-restore-desktop-buffer').

Yes; since both saveplace and bookmark are in the standard Emacs dist,
it's fine for them to share code, and would improve maintainability.

I'm not sure when I'll get a chance to work on this, though.  The
original bug here is about saving/restoring position in dired buffers.
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.

Best,
-Karl