bug#65055: 30.0.50; save-place-abbreviation-file-names :set fails for saveplace-pdf-view places

[Eli Zaretskii]

> From: Visuwesh <visuweshm@gmail.com>
> Cc: 65055@debbugs.gnu.org
> Date: Fri, 04 Aug 2023 23:43:56 +0530
> 
> [வெள்ளி ஆகஸ்ட் 04, 2023] Eli Zaretskii wrote:
> 
> >>     ("/home/viz/tmp/piron1976.pdf"
> >>       (pdf-view-bookmark "piron1976.pdf"
> >>                  (filename . "~/tmp/piron1976.pdf")
> >>                  (position . 1)
> >>                  (last-modified 25805 5369 546130 641000)
> >>                  (page . 2)
> >>                  (slice)
> >>                  (size . fit-width)
> >>                  (origin)
> >>                  (handler . pdf-view-bookmark-jump-handler)))
> >> 
> >> This fails with the following backtrace because 
> >> (cdr (pdf-view-bookmark ...) is a list,
> >
> > I don't understand: the valid format of save-place-alist is documented
> > in its doc string, so this looks like a bug in saveplace-pdf-view
> > package?  Or what am I missing?
> 
> Unfortunately, that's only half the story.  save-place-alist can also
> have elements such as
> 
>     ("SOME-DIRECTORY" (dired-filename . "SOME-FILENAME"))
> 
> to save the place for dired buffers.

Where is this documented, and when the change to support this
extension was added?

And in any way, the form you show above, which is used by
saveplace-pdf-view, is not even of that extended format, or am I again
missing something?

> But instead of checking if the second element of the place's cdr is a
> string, the :set function simply assumes that it will be always be a
> string.  This is not wrong as such since that's the only allowed format
> as far as core Emacs is concerned but I think having the check in place
> would help expand the save-place-alist format as the user desires.

I don't think we should tweak our code to support save-place-alist
formats that violate the documented formats.  If the documented format
is not followed, how do you know which elements of which parts of the
data structures are file names in the first place?

The right way of extending such data structures is to allow functions
as the values, and let those functions deal with non-standard formats.
The way saveplace-pdf-view does it, IIUC, is simply wrong: it just
happened to work by sheer luck with previous versions because Emacs
never actually acted on the assumption that a file name is in a
certain place in the data stricture.