bug#20845: 25.0.50; bookmark.el, handling of fictitious `buffer' property

[Stefan Monnier]

> I'm looking into Emacs Bug#20845 which asks about the purpose of the
> "bookmark" property in bookmark.el.

I don't see any `bookmark` property, sorry.

>> Function `bookmark-default-handler' picks up and handles the property
>> (field) named `bookmark', if present.

Where?

>> I see nowhere else where this property is used, and nowhere where it is
>> set.  And it is not documented.

Sounds like the description of leftover code.  Usually it's either
because of an oversight or for backward compatibility purposes.

>> IOW, there is no notion or existence (AFAICT) of any bookmarks that
>> record a `buffer' property.

Which property are we talking about, `buffer` or `bookmark`?

>> * If the file is readable and is not visited then it visits the file
>>   using `find-file-noselect'.  Why?

I'll assume this question was the result of a temporary confusion on
Drew's part, since I think it's pretty obvious why we'd want to visit
the file here.

>> * Otherwise, if the buffer named by property `buffer' exists then the
>>   default handler does nothing (no-op).

No: it selects that buffer as the current buffer.

> This handling was added in this commit:
>
>     commit dbf8402bc76a775284905f09399b4d88ee0c03e5
>     Author: Stefan Monnier <monnier@iro.umontreal.ca>
>     Date:   Wed Feb 10 15:02:54 2010 -0500
>
>         (bookmark-handle-bookmark): Catch the right error.
>         (bookmark-default-handler): Accept new bookmark field `buffer'.

And it had been earlier removed by:

    commit 13901bcbc4926630bdb2127301af0cdf7bcc50f7
    Author: Karl Fogel <kfogel@red-bean.com>
    Date:   Mon Oct 5 01:35:34 2009 +0000

so my commit re-instated support for it.

> But trying to find out why this was committed made me even more
> confused.  I could find Stefan Monnier arguing against this change on
> two different occasions:
>
>     2010-01-25 - https://debbugs.gnu.org/cgi/bugreport.cgi?bug=5476#14
>     2010-01-26 - https://debbugs.gnu.org/cgi/bugreport.cgi?bug=5476#20

Good digging, thanks.

> I understand this was a long time ago, but if you still remember it,
> do you think you could help shed some light into the purpose of this
> code?

I can't remember why I accepted to install it, but these discussions
seem to indicate that the purpose for the patch was to preserve
compatibility with old bookmarks that don't use their own
bookmark handler.

> The suggestion given by Drew above is to remove it.
> Do you have an opinion about that?

Not really, no.  But such "old-style" bookmarks might still be used by
third-party packages and might still appear in users's
~/.emacs.d/bookmarks, so if we want to remove support for it, maybe we
should try and do it gradually.

[...taking a second look...]

Oh, I see how/where it's used and it's definitely still used:
Info-bookmark-jump uses it (i.e. not in the stored bookmark object, but
in a transient object passed to bookmark-default-handler).
So maybe it should be documented in the docstring of
`bookmark-default-handler` for the benefit of other handlers.


        Stefan