On 10/11/18 12:06 AM, Drew Adams wrote:
>> people sometimes use several frames, so i would like to make opening
>> bookmark one shot. Attached patch adds two funcs,
>> "bookmark-jump-other-frame" and the equivalent from the bookmarks menu,
>> "bookmark-bmenu-other-frame"
> 
> My suggestions in this regard, FWIW:
> 
> 1. Don't use `view-buffer-other-frame'.
>      Select the buffer, and not just read-only. Jumping to a bookmark
>      typically puts you at its location (hence select), and the buffer is
>      typically not put in a read-only mode. IOW, do the equivalent of
>      this, or similar:
> 
>      (let ((pop-up-frames  t)) (bookmark-jump-other-window bookmark)
> 
> 2. Don't use `F' as the key binding in the bookmark-list buffer.
>      `F' is more often used for files than for frames. Maybe use `5'.
>   
>      (I use `J 5' in Bookmark+. `J' is a prefix for the jump commands
>      in the bookmark-list buffer. The `5' is from `C-x 5' bindings for
>      other-frame. I bind the command to `C-x 5 B' and `C-x j 5' globally.
>      `C-x j' is a global prefix key for bookmark jump commands.)
> 

So here is the new version of the patch
1. Use pop-up-frames variable to avoid a read-only mode on new frame.
    I also had to use (other-frame 1) to ensure new frame is raised.

2. Use "5" binding both in bookmark-map, and bookmark-bmenu-mode-map

3. Split in two commits, one per new function
    I tried to respect the conventions on the log.

The patches are generated using git format-patch.

Regards
Pierre-Yves