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.
Message #107 received at submit <at> debbugs.gnu.org (full text, mbox):
From: Ship Mints <shipmints <at> gmail.com> To: Thierry Volpiatto <thievol <at> posteo.net> Cc: Eli Zaretskii <eliz <at> gnu.org>, bug-gnu-emacs <at> gnu.org Subject: Re: bug#75354: (29.4; eww buffer is not displayed correctly when used from bookmark-jump ) Date: Tue, 18 Mar 2025 12:28:17 -0400
[Message part 1 (text/plain, inline)]
On Tue, Mar 18, 2025 at 11:55 AM Thierry Volpiatto <thievol <at> posteo.net> wrote: > Ship Mints <shipmints <at> gmail.com> writes: > > > 1. ( ) text/plain (*) text/html > > > > On Tue, Mar 18, 2025 at 11:15 AM Thierry Volpiatto <thievol <at> posteo.net> > wrote: > > > > Ship Mints <shipmints <at> gmail.com> writes: > > > > > 1. ( ) text/plain (*) text/html > > > > > > On Tue, Mar 18, 2025 at 2:55 AM Thierry Volpiatto < > thievol <at> posteo.net> wrote: > > > > > > 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? > > > > > > Thanks for the continuing discussion. > > > > > > The concept will work but it feels a bit over-engineered. > > > > It is not, it is quite simple. > > > > > The approach of ignoring save-window-excursion and display-func via > > > bookmark-record entries or using properties on the handler seem > less > > > intrusive or a mix, if we feel that's appropriate. > > > > I proposed this solution to help you cleaning up your code which is > full > > of workarounds for the current behavior (prior 31). Of course if you > > don't want to make an effort to update your code, what you propose is > > simpler (i.e. you have nothing to change on your side), but > generally we > > (external emacs extensions developers) try to adapt ourselves to > Emacs > > source and not the contrary. > > > > Thanks for the input. > > > > The idea that I "don't want to make an effort" is insulting. > > Sorry if you take it like this, it was not the intention. > > > Perhaps a little less coffee. > > I don't drink coffee. > > > > Why not just fix the eww bookmark handler to do its own > > > save-window-excursion, again, rather than make a default bookmark > jump > > > behavior policy change? > > > > Because the problem is not just about eww, but more generally on how > > bookmark handlers work. > > > > Curious to know which other ones are broken? I read eww and w3m. > > It is not only about eww AND w3m. The point is not if things are broken > or not, it is to provide a good API for all bookmarks (and future kind > of bookmarks). > > > > What do the Emacs maintainers think about this as a matter of taste, > > easy adoption for other bookmark users, and idiomatic usage? > > Now Eli and other maintainers will decide what is the best for emacs. > You may not have seen it but there is already precedent for bookmark-handler properties in bookmark.el in bookmark-insert for the 'bookmark-inhibit property on a handler. It could contain a list of inhibitions.
[Message part 2 (text/html, inline)]
GNU bug tracking system
Copyright (C) 1999 Darren O. Benham,
1997,2003 nCipher Corporation Ltd,
1994-97 Ian Jackson.