On Mon, Mar 17, 2025 at 6:41 AM Ship Mints wrote: > 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))))) > And another that handles its own windows which the change to bookmark--jump-via will break: https://github.com/sunrise-commander/sunrise-commander/blob/16e6df7e86c7a383fb4400fae94af32baf9cb24e/sunrise-checkpoint.el#L83 (defun sunrise-checkpoint-handler (&optional bookmark) "Handler for a checkpoint BOOKMARK." (sunrise-ensure-running) (sunrise-select-window 'left) (let ((dirs (cdr (assq 'sunrise-directories (cdr bookmark)))) (missing '())) (mapc (lambda (x) (if (file-directory-p x) (sunrise-save-aspect (dired x) (sunrise-bookmark-jump)) (setq missing (cons sunrise-selected-window missing))) (sunrise-change-window)) dirs) (if missing (sunrise-checkpoint-relocate bookmark (reverse missing))))) https://github.com/sunrise-commander/sunrise-commander/blob/16e6df7e86c7a383fb4400fae94af32baf9cb24e/sunrise.el#L2371 (defun sunrise-change-window() "Change to the other Sunrise pane." (interactive) (sunrise-select-window (sunrise-other-side)) (setq sunrise-selected-window-width nil))