RE: bookmark.el bug report

["Drew Adams" <drew.adams@oracle.com>]

> Note to Drew Adams: I took your suggestions about new constants.
> However, I did not wrap the new `save-excursion' in `ignore-errors'
> or `condition-case', because we should never be at eob right 
> after calling `bookmark-bmenu-ensure-position' (and if we are,
> I want to know).

Hi Karl,

Do as you like, but part of my message was that there is no need to call
`bookmark-bmenu-check-position' here (which you renamed `...ensure...').

Since the "check" part of that function doesn't really do anything, all it
really does is move point to the nearest bookmark line. If point is already on a
bookmark line, then it is a no-op.

And the way to tell if point is on a bookmark line is (now) to call
`bookmark-bmenu-bookmark'. So I use this simplified code (FWIW):

(defun bookmark-bmenu-check-position ()
  "Move to the beginning of the nearest bookmark line."
  (beginning-of-line)
  (unless (bookmark-bmenu-bookmark)
    (if (and (bolp) (eobp))
        (beginning-of-line 0)
      (goto-char (point-min))
      (forward-line bookmarkp-bmenu-header-lines)))
  t) ; Vanilla bookmark code depends on non-nil value.

(defun bookmark-bmenu-bookmark ()
  "Return the name of the bookmark on this line."
  (condition-case nil
      (save-excursion
       (forward-line 0)
       (forward-char (1+ bookmarkp-bmenu-marks-width))
       (get-text-property (point) 'bookmarkp-bookmark-name))
    (error nil)))

Instead of calling `*-bmenu-bookmark' at the beginning of `ensure' to check for
a no-op, the original code has it backward: it calls `ensure' at the start of
each call to `*-bmenu-bookmark', to ensure that point is on a bookmark line. But
for many calls to `*-bmenu-bookmark', we already know that point is on a
bookmark line.

If it is not called also within `*-bmenu-bookmark', then `ensure' is called much
less often than `*-bmenu-bookmark'. And the original code has extra calls to
`ensure', besides the one in `*-bmenu-mark'. In fact, it often calls it both at
the beginning and at the end of a function (e.g. `*-bmenu-delete').

`ensure' only needs to be called before some (not all) of the calls to
`*-bmenu-bookmark' - it has no use otherwise.

That's why I said you can get rid of some of the calls to `ensure'. But if
`ensure' is not called from within `*-bmenu-bookmark' then you need to protect
the eob case (just to inhibit a msg) - hence the `condition-case'.

HTH.

--

I don't know what changes you actually made, since I cannot see them here:
http://cvs.savannah.gnu.org/viewvc/emacs/emacs/lisp/
The last revision of bookmark.el shown there dates from Nov 24.

Has the URL for the Emacs development source code changed, now that the
repository is on BZR? If so, what is it - how does one get to the source code
via HTTP? And if that URL is no longer valid, then its Web page should redirect
to the proper URL, no?

Personally, I do not intend to access the repository using BZR (just as I did
not use CVS to access it before). I'm looking for a way to download a revision
by just clicking an ordinary web-page link in my browser, as before.