Package: emacs;
Reported by: Thierry Volpiatto <thievol <at> posteo.net>
Date: Sat, 4 Jan 2025 16:15:02 UTC
Severity: normal
Found in version 29.4
Done: Eli Zaretskii <eliz <at> gnu.org>
Bug is archived. No further changes may be made.
View this message in rfc822 format
From: Thierry Volpiatto <thievol <at> posteo.net> To: Ship Mints <shipmints <at> gmail.com> Cc: thievol <at> posteo.net, eliz <at> gnu.org, 75354 <at> debbugs.gnu.org Subject: bug#75354: (29.4; eww buffer is not displayed correctly when used from bookmark-jump ) Date: Tue, 18 Mar 2025 07:03:17 +0000
[Message part 1 (text/plain, inline)]
Sorry for late reply, was busy.
Ship Mints <shipmints <at> gmail.com> writes:
> On Sun, Mar 16, 2025 at 5:10 PM Ship Mints <shipmints <at> gmail.com> wrote:
>
> On Sat, Mar 15, 2025 at 10:18 AM Ship Mints <shipmints <at> gmail.com> wrote:
>
> On Sat, Mar 15, 2025 at 1:37 AM Thierry Volpiatto <thievol <at> posteo.net> wrote:
>
> Ship Mints <shipmints <at> gmail.com> 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.
What we could do is propose a more flexible solution so that you could
use whatever you want for bookmark--jump-via; With what you have proposed so
far, you still have the problem of DISPLAY-FUNCTION which will always
run (I see there is comments about this problem in your mentionned
packages), with the patch below you could define a display-function
entry in your bookmark-record e.g. (display-function . ignore) and then
add a special method for bookmark--jump-via:
(cl-defmethod bookmark--jump-via (bookmark-name-or-record (_ (eql 'ignore)))
(do_watever_you_want_here)) ; e.g. run only the handler fn.
NOTE: I used 'ignore as example but you could use whatever you want.
Here the patch:
diff --git a/lisp/bookmark.el b/lisp/bookmark.el
index 99bb26e83cc..e594387f364 100644
--- a/lisp/bookmark.el
+++ b/lisp/bookmark.el
@@ -1259,7 +1259,7 @@ it to the name of the bookmark currently being set, advancing
"Hook run after `bookmark-jump' jumps to a bookmark.
Useful for example to unhide text in `outline-mode'.")
-(defun bookmark--jump-via (bookmark-name-or-record display-function)
+(cl-defgeneric bookmark--jump-via (bookmark-name-or-record display-function)
"Handle BOOKMARK-NAME-OR-RECORD, then call DISPLAY-FUNCTION.
DISPLAY-FUNCTION is called with the new buffer as argument.
@@ -1319,8 +1319,12 @@ DISPLAY-FUNC would be `switch-to-buffer-other-window'."
;; Don't use `switch-to-buffer' because it would let the
;; window-point override the bookmark's point when
;; `switch-to-buffer-preserve-window-point' is non-nil.
- (bookmark--jump-via bookmark (or display-func 'pop-to-buffer-same-window)))
+ (bookmark-jump-1 bookmark display-func))
+(defun bookmark-jump-1 (bookmark display-func)
+ (let ((dfn (or (bookmark-prop-get bookmark 'display-function)
+ display-func 'pop-to-buffer-same-window)))
+ (bookmark--jump-via bookmark dfn)))
;;;###autoload
(defun bookmark-jump-other-window (bookmark)
@@ -2303,7 +2307,7 @@ the related behaviors of `bookmark-save' and `bookmark-bmenu-save'."
(pop-up-windows t))
(delete-other-windows)
(switch-to-buffer (other-buffer) nil t)
- (bookmark--jump-via bmrk 'pop-to-buffer)
+ (bookmark-jump-1 bmrk 'pop-to-buffer)
(bury-buffer menu)))
@@ -2317,7 +2321,7 @@ the related behaviors of `bookmark-save' and `bookmark-bmenu-save'."
"Select this line's bookmark in other window, leaving bookmark menu visible."
(interactive nil bookmark-bmenu-mode)
(let ((bookmark (bookmark-bmenu-bookmark)))
- (bookmark--jump-via bookmark 'switch-to-buffer-other-window)))
+ (bookmark-jump-1 bookmark 'switch-to-buffer-other-window)))
(defun bookmark-bmenu-other-frame ()
@@ -2333,7 +2337,7 @@ The current window remains selected."
(interactive nil bookmark-bmenu-mode)
(let ((bookmark (bookmark-bmenu-bookmark))
(fun (lambda (b) (display-buffer b t))))
- (bookmark--jump-via bookmark fun)))
+ (bookmark-jump-1 bookmark fun)))
(defun bookmark-bmenu-other-window-with-mouse (event)
"Jump to bookmark at mouse EVENT position in other window.
Also I guess trying to call bookmark-jump-other-window and friends is
failing with your special bookmarks, with this it would run just as
bookmark-jump without (possible) errors.
WDYT?
--
Thierry
[signature.asc (application/pgp-signature, inline)]
GNU bug tracking system
Copyright (C) 1999 Darren O. Benham,
1997,2003 nCipher Corporation Ltd,
1994-97 Ian Jackson.