Re: bookmark.el bug report

[Karl Fogel <kfogel@red-bean.com>]

"Drew Adams" <drew.adams@oracle.com> writes:
>> (defun bookmark-bmenu-bookmark ()
>>   "Return the name of the bookmark on this line."
>>   (when (bookmark-bmenu-check-position)
>>     (save-excursion
>>       (forward-line 0) (forward-char 3)
>>       (get-text-property (point) 'bmkext-bookmark-name))))
>
> 1. There is no need for the `bookmark-bmenu-check-position' test. All such
> pseudotests can be removed - that function always returns non-nil (unless the
> `bookmark-alist' is nil).

I have a pending change for that (in my CVS working copy -- need to port
it over to Bzr).  I want to make absolutely sure there wasn't some edge
case about when bookmark-alist is nil, but I strongly suspect you're
right, Drew.

> 2. Wrap the `save-excursion' in `ignore-errors' or a `condition-case',
> for the `forward-char' at eob. E.g., this is the definition in
> bookmark+.el:
>
> (defun bookmark-bmenu-bookmark ()
>   "Return the name of the bookmark on this line."
>   (condition-case nil
>       (save-excursion
>        (forward-line 0) (forward-char 3)
>        (get-text-property (point) 'bookmarkp-bookmark-name))
>     (error nil)))

Will take into account.

Thanks,
-K