On Thu, Mar 13, 2025 at 3:34 PM Ship Mints wrote: > On Thu, Mar 13, 2025 at 3:21 PM Thierry Volpiatto > wrote: > >> Ship Mints writes: >> >> >> From: Thierry Volpiatto >> >> Cc: Thierry Volpiatto , 75354@debbugs.gnu.org >> >> Date: Wed, 08 Jan 2025 13:52:32 +0000 >> >> >> >> > This is contrary to what you originally wrote (with which I agree): >> >> >> >> Yes, after deeper search I found that `bookmark--jump-via` is behaving >> >> like this AFAIU: >> >> - It calls the handler which creates a new buffer related to bookmark. >> >> - It then displays the current-buffer (the one before jumping to bmk) >> in >> >> a window according to DISPLAY-FUNCTION and make the bookmark buffer >> current. >> >> >> >> This approach is OK as long as the handler fn doesn't try do do one >> part >> >> of the job (window handling) itself, which is not the case at least >> with >> >> eww and w3m. It is as well counter intuitive, DISPLAY-FUNCTION should >> >> be called on the buffer generated by bookmark and not the contrary, so >> >> this change makes the code simpler and easier to understand. >> >> >> >> > By contrast, the change you propose now will affect all the uses of >> >> > bookmarks, everywhere, >> >> >> >> Yes, this is intended, in addition of fixing eww, it fixes w3m and also >> >> the quit function of eww (actually broken). >> >> >> >> > which is riskier, given how many different variants of bookmark usage >> >> > are out there. >> >> >> >> Tested here on many different kinds of bookmarks and work as expected >> >> unlike the current code. >> > >> > It took me a while to find time to focus on some bugs with Emacs 31 in >> > my workflow (reverting to Emacs 30 until I had time). >> > >> > I discovered that this change breaks all bookmark handlers that rely >> > on managing window-state or window-configuration. >> >> Which bookmark handlers? > > > At least these packages rely on bookmarks that store window state and > their handlers restore window state. There are workarounds I have > considered but they are distasteful, to say the least. > > bufferlo > activities > burly > > I help maintain bufferlo and this change broke bookmarks for all our users > who have tried master, including me. > > > I have no workaround yet this until we fix it, I have to do screwy >> > things to make 'bookmark--jump-via' ignore 'save-window-excursion'. >> > >> > What's the rationale for wrapping 'bookmark-handle-bookmark' with >> > 'save-window-excursion'? >> >> 1) The bookmark handler function setup a buffer. >> 2) When this buffer is ready we jump to this buffer with >> DISPLAY-FUNCTION. >> >> If you don't save the window excursion in 1) and buffer is not ready, >> the DISPLAY-FUNCTION may run from the buffer that was current before >> calling the handler function. >> > > Emacs is single threaded. What does it mean for a buffer to "not be > ready?" > > Previously we were calling DISPLAY-FUNCTION on current-buffer, assuming >> the handler fn had switched to a new buffer which should be current, >> when the new buffer was not current we were swicthing to the >> current-buffer the one that was current just before as explained above. >> >> So this change has fixed not only eww but some other bookmarks (like >> w3m). >> > > Yes, you said in the original bug report. > > > If this behavior is truly necessary (I'd like understand the use >> > case), I suggest binding a new defvar to bind and trigger this >> > behavior rather than affect all bookmark handlers. Unless eww can be >> > fixed. It could handle window configuration management in its own >> > bookmark handler. >> >> That's the point, you should not handle window management in your >> bookmark handler, this defeat bookmark--jump-via DISPLAY-FUNCTION. >> > > Yes, we should handle window configurations. Bookmarks are not limited to > just files or URLs. Any new limitations imposed on bookmarks that assume > so makes bookmarks worse, not better. > > This change also fixes old bugs where people were trying various things >> in their bookmark handlers to get things right including polling or >> running code under a timer etc... >> > > Right. Those are some of the distasteful workarounds that Adam Porter had > to resort to for activities in an effort to work around display-function > and now he has to deal with window-save-excursion restoring the the window > configuration *from our own calling sites*. If you think the new behavior > should be limited to interactive use, I could perhaps understand that, but > I still would suggest a defvar to control this so those of us who use > bookmarks programmatically can still do so. > In fact, it would be even better to optionally inhibit both save-window-excursion and display-function. As it is, programmatically, we invoke bookmark-jump with #'ignore as the display-function.