On Mon, Mar 17, 2025 at 6:35 AM Ship Mints wrote: > On Sun, Mar 16, 2025 at 5:10 PM Ship Mints wrote: > >> On Sat, Mar 15, 2025 at 10:18 AM Ship Mints wrote: >> >>> On Sat, Mar 15, 2025 at 1:37 AM Thierry Volpiatto >>> wrote: >>> >>>> Ship Mints writes: >>>> >>>> > I have workarounds that work only for the most simplistic cases. Many >>>> > of our bookmarks themselves contain embedded bookmarks and bookmark >>>> > references (which are individually addressable so can be used >>>> > separately) with window-states we need to restore in tab-bar tabs that >>>> > they represent. >>>> >>>> I don't really understand what your packages are doing or are intended >>>> doing, but FWICS in bufferlo: You are using in some places >>>> (bookmark-jump name #'ignore); why don't you do all this work (restore >>>> window-states in tab) in DISPLAY-FUNCTION instead of using `ignore`? >>>> Your handler would be much simpler by moving the window-state-put and >>>> alike calls in DISPLAY-FUNCTION: >>>> >>>> (bookmark-jump name #'your_function_restoring_window_or_frame_state) >>>> >>>> Using (bookmark-jump name #'ignore) with all the code that jump to >>>> frame/tab etc... in the handler is just a workaround to fix the previous >>>> buggy behavior of bookmark--jump-via. IMO. >>>> >>>> It would be good to start with a good example or recipe to see if we can >>>> find a good solution. >>>> >>> >>> We need the bookmarks to work from the bookmark menu where no >>> display-function overrides are supported. >>> >>> I suggest we add bookmark-record keys that indicate to bookmark-jump to >>> inhibit/or allow messing with window-configurations. The bufferlo >>> bookmarks (and Adam's if he wants) would contain these hint keys. >>> >>> '(bookmark-jump-inhibit-window-actions . t) ; or whatever we come up with >>> >>> I can contrive an example, if necessary, but I believe y'all get the >>> point. Nested bookmarks whose handlers expect their window-configurations >>> not to be messed with up the chain, will always be broken without >>> additional controls. >>> >> >> The attached patch implements such a scheme that works for us, and is >> transparent to other bookmark uses. >> > > Perhaps we should restore bookmark--jump-via to its previous behavior and > better document the "rules of the road" for bookmark handlers. For simple > file- and point-based bookmarks, handlers need to ensure that when they > return, the selected window and current buffer are what's intended. For > bookmark handlers that perform other actions, those rules need not apply to > leverage the bookmark infrastructure. > > eww's handler could simply do this itself since it seems eww's url opening > behavior warrants this: > > (defun eww-bookmark-jump (bookmark) > "Default bookmark handler for EWW buffers." > (save-window-excursion > (eww (bookmark-prop-get bookmark 'location)))) > > I'd also suggest that we recommend adding an additional property to a > bookmark-handler function that could inhibit bookmark--jump-via's call to > display-func entirely. In our case, when called programmatically, we use > #'ignore, but when the bookmark menu is used, we'd prefer to skip the > default pop-to-buffer-same-window. > Here's an example of another handler in the wild that handles its own save-window-excursion: https://github.com/emacsmirror/info-plus/blob/c6e26367abd2e99791f6e85fced2383de9ec605a/info%2B.el#L7025C1-L7039C98 (defun Info-bookmark-jump (bookmark) "Handler function for record returned by `Info-bookmark-make-record'. BOOKMARK is a bookmark name or a bookmark record. If `Info-bookmark-use-only-node-not-file-flag' is nil, and the recorded Info file is readable, then use it. If not, then go to the recorded Info node in the manual for the current Emacs version." (let* ((absfile (bookmark-prop-get bookmark 'filename)) (file (if (or Info-bookmark-use-only-node-not-file-flag (not (file-readable-p absfile))) (file-name-nondirectory absfile) absfile)) (info-node (bookmark-prop-get bookmark 'info-node)) (buf (save-window-excursion ; Vanilla FIXME: doesn't work with frames! (Info-find-node file info-node) (current-buffer)))) (bookmark-default-handler `("" (buffer . ,buf) . ,(bookmark-get-bookmark-record bookmark)))))