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

Previous Next

Package: emacs;

Reported by: Visuwesh <visuweshm <at> gmail.com>

Date: Fri, 4 Aug 2023 16:16:02 UTC

Severity: normal

Found in version 30.0.50

Done: Eli Zaretskii <eliz <at> gnu.org>

Bug is archived. No further changes may be made.

Full log


View this message in rfc822 format

From: Eli Zaretskii <eliz <at> gnu.org>
To: Visuwesh <visuweshm <at> gmail.com>
Cc: 65055 <at> debbugs.gnu.org
Subject: bug#65055: 30.0.50; save-place-abbreviation-file-names :set fails for saveplace-pdf-view places
Date: Fri, 04 Aug 2023 22:03:39 +0300
> From: Visuwesh <visuweshm <at> gmail.com>
> Cc: 65055 <at> 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.




This bug report was last modified 1 year and 348 days ago.

Previous Next


GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.