Subject: Re: bug#41225: [PATCH] Show bookmark list in new tab

From: Lars Ingebrigtsen <[email-masked]>
Date: Sat, 08 Aug 2020 13:47:38 +0200

Matthias Meulien <[email-masked]> writes:

> A small patch that adds a customize choice to `tab-bar-new-tab-choice`
> in order to show bookmark list in new tab.

[...]

> +When the 'bookmark.el' library is loaded, a customize choice is added
> +to 'tab-bar-new-tab-choice' for new tab to show the bookmark list.

Makes sense to me.

> +;;;###autoload
> +(defun bookmark-bmenu-get-buffer ()
> +  "Return the Bookmark List, building it if it doesn't exists.
> +Don't affect the buffer ring order."
> +  (cond
> +     ((get-buffer bookmark-bmenu-buffer))
> +     (t (save-excursion
> +	  (save-window-excursion
> +	    (bookmark-bmenu-list)
> +	    (get-buffer bookmark-bmenu-buffer))))))

I changed this bit into an `or' before committing.

> +(nconc (get 'tab-bar-new-tab-choice 'custom-type)
> +       '((const :tag "Bookmark List" bookmark-bmenu-get-buffer)))

And I'm not sure this is safe.  For one, if you load the file twice,
you'll get this added twice.  And it's destructively modifying a list
that's dumped with Emacs, which I think is undefined behaviour?

So I've added a new utility function custom-add-choice to custom.el that
checks for double entry, and doesn't modify anything destructively, and
pushed this to Emacs 28.1.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no