GNU bug report logs - #75354
29.4; eww buffer is not displayed correctly when used from bookmark-jump

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 75354 in the body.
You can then email your comments to 75354 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Thierry Volpiatto <thievol <at> posteo.net>
To: bug-gnu-emacs <at> gnu.org
Subject: 29.4; eww buffer is not displayed correctly when used from
 bookmark-jump 
Date: Sat, 04 Jan 2025 16:20:19 +0000

The eww buffer is displayed in two windows whereas it should be
displayed only in other-window when used like this:

--8<---------------cut here---------------start------------->8---
(require 'bookmark)
(require 'eww)
(let ((bookmark-alist '(("La langue française"
			 (location . "https://www.lalanguefrancaise.com/")
			 (handler . eww-bookmark-jump)))))
  (bookmark-jump (car bookmark-alist) #'switch-to-buffer-other-window))
--8<---------------cut here---------------end--------------->8---

It is not a bug in `bookmark-jump` but in `eww-bookmark-jump` (the eww
bookmark handler) which uses wrongly the function `eww` which itself uses
`pop-to-buffer-same-window`.  Probably a lower level function should be
used (if one?).



In GNU Emacs 29.4 (build 1, x86_64-pc-linux-gnu, X toolkit, cairo
 version 1.16.0, Xaw3d scroll bars) of 2024-12-26 built on IPad-S340
Windowing system distributor 'The X.Org Foundation', version 11.0.12101004
System Description: Linux Mint 21.3

Configured using:
 'configure CFLAGS=-O8 --bindir=/usr/local/sbin/emacs-29.4 --with-cairo
 --with-x-toolkit=lucid --with-modules --without-tree-sitter
 --without-native-compilation'

Configured features:
ACL CAIRO DBUS FREETYPE GIF GLIB GMP GNUTLS GPM GSETTINGS HARFBUZZ JPEG
JSON LCMS2 LIBOTF LIBSELINUX LIBSYSTEMD LIBXML2 M17N_FLT MODULES NOTIFY
INOTIFY PDUMPER PNG RSVG SECCOMP SOUND SQLITE3 THREADS TIFF
TOOLKIT_SCROLL_BARS X11 XAW3D XDBE XIM XINPUT2 XPM LUCID ZLIB

Important settings:
  value of $LANG: fr_FR.UTF-8
  locale-coding-system: utf-8-unix

Major mode: 

Minor modes in effect:
  emms-mode-line-mode: t
  emms-playing-time-display-mode: t
  emms-playing-time-mode: t
  server-mode: t
  psession-mode: t
  psession-savehist-mode: t
  register-preview-mode: t
  global-git-gutter-mode: t
  display-time-mode: t
  winner-mode: t
  tv-save-place-mode: t
  helm-epa-mode: t
  helm-descbinds-mode: t
  helm-top-poll-mode: t
  helm-adaptive-mode: t
  helm-mode: t
  helm-minibuffer-history-mode: t
  helm-ff-icon-mode: t
  shell-dirtrack-mode: t
  helm-popup-tip-mode: t
  dired-async-mode: t
  minibuffer-depth-indicate-mode: t
  gcmh-mode: t
  tooltip-mode: t
  global-eldoc-mode: t
  eldoc-mode: t
  show-paren-mode: t
  mouse-wheel-mode: t
  file-name-shadow-mode: t
  global-font-lock-mode: t
  font-lock-mode: t
  column-number-mode: t
  line-number-mode: t
  transient-mark-mode: t
  auto-composition-mode: t
  auto-encryption-mode: t
  auto-compression-mode: t

Load-path shadows:
None found.

Features:
(shadow emacsbug url-http url-gw url-cache url-auth gnus-async
gnus-bcklg gnus-ml disp-table nndraft nnmh nnfolder gnus-agent gnus-srvr
gnus-score score-mode nnvirtual nntp gnus-cache helm-emms helm-ring
tramp-sh cl-print helm-ls-hg vc-hg isl vc-rcs log-view pcvs-util cc-mode
cc-fonts cc-guess cc-menus cc-cmds cc-styles cc-align cc-engine cc-vars
cc-defs view smerge-mode diff helm-dabbrev cl-indent char-fold shortdoc
emacs-news-mode epa-file network-stream nsm mailalias qp epa-mail
face-remap gnus-cite smiley w3m-form w3m-symbol config-w3m w3m timezone
w3m-hist bookmark-w3m w3m-ems w3m-favicon w3m-image w3m-fb tab-line
w3m-proc w3m-util mm-archive mail-extr textsec uni-scripts idna-mapping
ucs-normalize uni-confusable textsec-check addressbook-bookmark
tv-mu4e-config advice gnus-and-mu4e mu4e-patch mu4e-contrib eshell
esh-cmd esh-ext esh-opt esh-proc esh-io esh-arg esh-module esh-groups
esh-util mu4e mu4e-org mu4e-notification notifications mu4e-main
smtpmail mu4e-view mu4e-mime-parts crm mu4e-headers mu4e-thread
mu4e-actions mu4e-compose mu4e-draft gnus-msg mu4e-search mu4e-lists
mu4e-bookmarks mu4e-mark mu4e-message flow-fill hl-line mu4e-contacts
mu4e-update mu4e-folders mu4e-context mu4e-query-items mu4e-server
mu4e-modeline mu4e-vars mu4e-helpers mu4e-config mu4e-window ido
mu4e-obsolete helm-skitour helm-command helm-elisp helm-eval edebug
debug backtrace helm-x-files helm-for-files helm-bookmark helm-info
bookmark image-file image-converter emms-config emms-idapi-browser
emms-idapi emms-idapi-musicbrainz emms-mpris emms-librefm-stream
emms-librefm-scrobbler emms-playlist-limit emms-i18n emms-history
emms-score emms-stream-info emms-metaplaylist-mode emms-bookmarks
emms-cue emms-mode-line-icon emms-browser sort emms-volume
emms-volume-sndioctl emms-volume-mixerctl emms-volume-pulse
emms-volume-amixer emms-playlist-sort emms-last-played emms-player-xine
emms-player-mpd tq emms-lyrics emms-url emms-streams emms-show-all
emms-tag-editor emms-tag-tracktag emms-mark emms-mode-line emms-cache
emms-info-native emms-info-native-spc emms-info-native-mp3
emms-info-native-ogg emms-info-native-opus emms-info-native-flac
emms-info-native-vorbis bindat emms-info-exiftool emms-info-tinytag
emms-info-metaflac emms-info-opusinfo emms-info-ogginfo
emms-info-mp3info emms-playlist-mode emms-player-vlc emms-player-mpv
emms-playing-time emms-info emms-later-do emms-player-mplayer
emms-player-simple emms-source-playlist emms-source-file locate
emms-setup emms emms-compat emms-auto helm-external helm-net
tramp-archive tramp-gvfs tramp-cache time-stamp zeroconf ffap
helm-ls-git vc-git diff-mode vc vc-dispatcher flymake-shellcheck
flymake-proc flymake project warnings sh-script smie treesit executable
make-mode org-indent org-element org-persist org-id org-refile avl-tree
generator oc-basic ol-eww eww url-queue thingatpt mm-url ol-rmail ol-mhe
ol-irc ol-info ol-gnus nnselect gnus-art mm-uu mml2015 mm-view mml-smime
smime gnutls dig gnus-sum shr pixel-fill kinsoku url-file svg dom
gnus-group gnus-undo gnus-start gnus-dbus dbus xml gnus-cloud nnimap
nnmail mail-source utf7 nnoo gnus-spec gnus-int gnus-range message
sendmail yank-media puny rfc822 mml mml-sec mm-decode mm-bodies
mm-encode mail-parse rfc2231 rfc2047 rfc2045 ietf-drums mailabbrev
gmm-utils mailheader gnus-win gnus nnheader gnus-util mail-utils range
mm-util mail-prsvr ol-docview doc-view jka-compr ol-bibtex bibtex
ol-bbdb ol-w3m ol-doi org-link-doi org-config ob-gnuplot org-crypt
org-protocol org ob ob-tangle ob-ref ob-lob ob-table ob-exp org-macro
org-src ob-comint org-pcomplete org-list org-footnote org-faces
org-entities noutline outline ob-emacs-lisp ob-core ob-eval org-cycle
org-table ol org-fold org-fold-core org-keys oc org-loaddefs find-func
org-version org-compat org-macs bug-reference cus-start naquadah-theme
solar cal-dst holidays holiday-loaddefs appt diary-lib diary-loaddefs
cal-menu calendar cal-loaddefs server bm cl-extra imenu tv-utils
psession frameset register-preview pcase git-gutter mule-util
dired-extension time winner describe-variable help-fns radix-tree
help-mode tv-save-place.el init-helm epa derived epg rfc6068 epg-config
helm-epa helm-descbinds cus-edit pp icons wid-edit helm-sys
helm-adaptive helm-mode helm-misc helm-files image-dired
image-dired-tags image-dired-external image-dired-util xdg image-mode
exif filenotify tramp tramp-loaddefs trampver tramp-integration files-x
tramp-compat rx shell pcomplete parse-time iso8601 time-date
helm-buffers all-the-icons all-the-icons-faces data-material
data-weathericons data-octicons data-fileicons data-faicons
data-alltheicons helm-occur helm-tags helm-locate helm-grep wgrep-helm
wgrep grep compile text-property-search comint ansi-osc ring helm-regexp
format-spec ansi-color helm-utils helm-help helm-types
helm-extensions-autoloads helm-autoloads helm helm-global-bindings
helm-easymenu edmacro kmacro helm-core helm-source helm-multi-match
helm-lib dired-async async dired-aux dired dired-loaddefs isl-autoloads
mb-depth avoid cus-load gcmh easy-mmode all-the-icons-autoloads
bash-completion-autoloads info ledger-mode-autoloads
markdown-mode-autoloads w3m-load w3m-autoloads package browse-url url
url-proxy url-privacy url-expand url-methods url-history url-cookie
generate-lisp-file url-domsuf url-util mailcap url-handlers url-parse
auth-source cl-seq eieio eieio-core cl-macs password-cache json subr-x
map byte-opt gv bytecomp byte-compile url-vars cl-loaddefs cl-lib rmc
iso-transl tooltip cconv eldoc paren electric uniquify ediff-hook
vc-hooks lisp-float-type elisp-mode mwheel term/x-win x-win
term/common-win x-dnd tool-bar dnd fontset image regexp-opt fringe
tabulated-list replace newcomment text-mode lisp-mode prog-mode register
page tab-bar menu-bar rfn-eshadow isearch easymenu timer select
scroll-bar mouse jit-lock font-lock syntax font-core term/tty-colors
frame minibuffer nadvice seq simple cl-generic indonesian philippine
cham georgian utf-8-lang misc-lang vietnamese tibetan thai tai-viet lao
korean japanese eucjp-ms cp51932 hebrew greek romanian slovak czech
european ethiopic indian cyrillic chinese composite emoji-zwj charscript
charprop case-table epa-hook jka-cmpr-hook help abbrev obarray oclosure
cl-preloaded button loaddefs theme-loaddefs faces cus-face macroexp
files window text-properties overlay sha1 md5 base64 format env
code-pages mule custom widget keymap hashtable-print-readable backquote
threads dbusbind inotify lcms2 dynamic-setting system-font-setting
font-render-setting cairo x-toolkit xinput2 x multi-tty
make-network-process emacs)

Memory information:
((conses 16 1612641 255300)
 (symbols 48 43903 5)
 (strings 32 321582 36530)
 (string-bytes 1 9242506)
 (vectors 16 125121)
 (vector-slots 8 2615479 161054)
 (floats 8 5249 2732)
 (intervals 56 129833 3766)
 (buffers 976 196))
<#secure method=pgpmime mode=sign>

-- 
Thierry

Message #8 received at 75354 <at> debbugs.gnu.org (full text, mbox):

From: Thierry Volpiatto <thievol <at> posteo.net>
To: 75354 <at> debbugs.gnu.org
Subject: bug#75354: (29.4; eww buffer is not displayed correctly when used
 from bookmark-jump )
Date: Wed, 08 Jan 2025 07:40:30 +0000

[Message part 1 (text/plain, inline)]

I could fix the problem by modifying bookmark--jump-via:

diff --git a/lisp/bookmark.el b/lisp/bookmark.el
index 0048330e790..a474bed652e 100644
--- a/lisp/bookmark.el
+++ b/lisp/bookmark.el
@@ -1261,28 +1261,29 @@ DISPLAY-FUNCTION is called with the current buffer as argument.
 After calling DISPLAY-FUNCTION, set window point to the point specified
 by BOOKMARK-NAME-OR-RECORD, if necessary, run `bookmark-after-jump-hook',
 and then show any annotations for this bookmark."
-  (bookmark-handle-bookmark bookmark-name-or-record)
-  ;; Store `point' now, because `display-function' might change it.
-  (let ((point (point)))
-    (save-current-buffer
-      (funcall display-function (current-buffer)))
-    (let ((win (get-buffer-window (current-buffer) 0)))
-      (if win (set-window-point win point))))
-  ;; FIXME: we used to only run bookmark-after-jump-hook in
-  ;; `bookmark-jump' itself, but in none of the other commands.
-  (when bookmark-fringe-mark
-    (let ((overlays (overlays-in (pos-bol) (1+ (pos-bol))))
-          temp found)
-      (while (and (not found) (setq temp (pop overlays)))
-        (when (eq 'bookmark (overlay-get temp 'category))
-          (setq found t)))
-      (unless found
-        (bookmark--set-fringe-mark))))
-  (run-hooks 'bookmark-after-jump-hook)
-  (if bookmark-automatically-show-annotations
+  (let (buf)
+    (save-window-excursion
+      (bookmark-handle-bookmark bookmark-name-or-record)
+      (setq buf (current-buffer)))
+    (let ((point (with-current-buffer buf (point))))
+      (funcall display-function buf)
+      (when-let ((win (get-buffer-window buf 0)))
+        (set-window-point win point)))
+    ;; FIXME: we used to only run bookmark-after-jump-hook in
+    ;; `bookmark-jump' itself, but in none of the other commands.
+    (when bookmark-fringe-mark
+      (let ((overlays (overlays-in (pos-bol) (1+ (pos-bol))))
+            temp found)
+        (while (and (not found) (setq temp (pop overlays)))
+          (when (eq 'bookmark (overlay-get temp 'category))
+            (setq found t)))
+        (unless found
+          (bookmark--set-fringe-mark))))
+    (run-hooks 'bookmark-after-jump-hook)
+    (when bookmark-automatically-show-annotations
       ;; if there is an annotation for this bookmark,
       ;; show it in a buffer.
-      (bookmark-show-annotation bookmark-name-or-record)))
+      (bookmark-show-annotation bookmark-name-or-record))))
 
 
 ;;;###autoload

-- 
Thierry

[signature.asc (application/pgp-signature, inline)]

Message #11 received at 75354 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Thierry Volpiatto <thievol <at> posteo.net>
Cc: 75354 <at> debbugs.gnu.org
Subject: Re: bug#75354: (29.4;
 eww buffer is not displayed correctly when used from bookmark-jump )
Date: Wed, 08 Jan 2025 15:10:10 +0200

> From: Thierry Volpiatto <thievol <at> posteo.net>
> Date: Wed, 08 Jan 2025 07:40:30 +0000
> 
> I could fix the problem by modifying bookmark--jump-via:

Thanks.

This is contrary to what you originally wrote (with which I agree):

> It is not a bug in `bookmark-jump` but in `eww-bookmark-jump` (the eww
> bookmark handler) which uses wrongly the function `eww` which itself uses
> `pop-to-buffer-same-window`.  Probably a lower level function should be
> used (if one?).

By contrast, the change you propose now will affect all the uses of
bookmarks, everywhere, which is riskier, given how many different
variants of bookmark usage are out there.

Why did you change your mind about the preferred way of fixing this?

Message #14 received at 75354 <at> debbugs.gnu.org (full text, mbox):

From: Thierry Volpiatto <thievol <at> posteo.net>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: Thierry Volpiatto <thievol <at> posteo.net>, 75354 <at> debbugs.gnu.org
Subject: Re: bug#75354: (29.4; eww buffer is not displayed correctly when
 used from bookmark-jump )
Date: Wed, 08 Jan 2025 13:52:32 +0000

[Message part 1 (text/plain, inline)]

Eli Zaretskii <eliz <at> gnu.org> writes:

>> From: Thierry Volpiatto <thievol <at> posteo.net>
>> Date: Wed, 08 Jan 2025 07:40:30 +0000
>> 
>> I could fix the problem by modifying bookmark--jump-via:
>
> Thanks.
>
> 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.

> Why did you change your mind about the preferred way of fixing this?

Because I couldn't find a way to fix this correctly by modifying only
the handler, thus the change would need to be done on each other
handlers which behave unexpectedly.

Here is the latest patch attached.

Thanks.

-- 
Thierry

[0001-Handle-correctly-DISPLAY-FUNCTION-arg-in-bookmark-ju.patch (text/x-diff, attachment)]

[signature.asc (application/pgp-signature, inline)]

Message #17 received at 75354 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Thierry Volpiatto <thievol <at> posteo.net>
Cc: 75354 <at> debbugs.gnu.org
Subject: Re: bug#75354: (29.4; eww buffer is not displayed correctly when
 used from bookmark-jump )
Date: Wed, 08 Jan 2025 16:03:34 +0200

> From: Thierry Volpiatto <thievol <at> posteo.net>
> Cc: Thierry Volpiatto <thievol <at> posteo.net>,  75354 <at> 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.

OK, thanks.  Let's leave this for a few days to give people time to
post comments if they have them.

Message #20 received at 75354 <at> debbugs.gnu.org (full text, mbox):

From: Thierry Volpiatto <thievol <at> posteo.net>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: Thierry Volpiatto <thievol <at> posteo.net>, 75354 <at> debbugs.gnu.org
Subject: Re: bug#75354: (29.4; eww buffer is not displayed correctly when
 used from bookmark-jump )
Date: Wed, 08 Jan 2025 14:47:26 +0000

[Message part 1 (text/plain, inline)]

Eli Zaretskii <eliz <at> gnu.org> writes:

>> From: Thierry Volpiatto <thievol <at> posteo.net>
>> Cc: Thierry Volpiatto <thievol <at> posteo.net>,  75354 <at> 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.
>
> OK, thanks.  Let's leave this for a few days to give people time to
> post comments if they have them.

Ok, I will make changes to commit message (needs * lisp/bookmark.el (...):
bla) and also when-let => when-let* to fit with emacs-30+.

Thanks.

-- 
Thierry

[signature.asc (application/pgp-signature, inline)]

Message #23 received at 75354 <at> debbugs.gnu.org (full text, mbox):

From: Thierry Volpiatto <thievol <at> posteo.net>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: Thierry Volpiatto <thievol <at> posteo.net>, 75354 <at> debbugs.gnu.org
Subject: Re: bug#75354: (29.4; eww buffer is not displayed correctly when
 used from bookmark-jump )
Date: Fri, 10 Jan 2025 05:56:40 +0000

[Message part 1 (text/plain, inline)]

Find here the last patch attached.

-- 
Thierry

[0001-Handle-correctly-DISPLAY-FUNCTION-arg-in-bookmark-ju.patch (text/x-diff, attachment)]

[signature.asc (application/pgp-signature, inline)]

Message #26 received at 75354 <at> debbugs.gnu.org (full text, mbox):

From: Thierry Volpiatto <thievol <at> posteo.net>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: Thierry Volpiatto <thievol <at> posteo.net>, 75354 <at> debbugs.gnu.org
Subject: Re: bug#75354: (29.4; eww buffer is not displayed correctly when
 used from bookmark-jump )
Date: Wed, 15 Jan 2025 06:43:34 +0000

[Message part 1 (text/plain, inline)]

Eli Zaretskii <eliz <at> gnu.org> writes:

>> From: Thierry Volpiatto <thievol <at> posteo.net>
>> Cc: Thierry Volpiatto <thievol <at> posteo.net>,  75354 <at> 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.
>
> OK, thanks.  Let's leave this for a few days to give people time to
> post comments if they have them.

Ping.

-- 
Thierry

[signature.asc (application/pgp-signature, inline)]

Message #29 received at 75354 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Thierry Volpiatto <thievol <at> posteo.net>
Cc: 75354 <at> debbugs.gnu.org
Subject: Re: bug#75354: (29.4; eww buffer is not displayed correctly when
 used from bookmark-jump )
Date: Wed, 15 Jan 2025 17:01:23 +0200

> From: Thierry Volpiatto <thievol <at> posteo.net>
> Cc: Thierry Volpiatto <thievol <at> posteo.net>,  75354 <at> debbugs.gnu.org
> Date: Wed, 15 Jan 2025 06:43:34 +0000
> 
> >> Tested here on many different kinds of bookmarks and work as expected
> >> unlike the current code.
> >
> > OK, thanks.  Let's leave this for a few days to give people time to
> > post comments if they have them.
> 
> Ping.

I didn't forget, don't worry, just waiting for some free time to
install.

Message #32 received at 75354 <at> debbugs.gnu.org (full text, mbox):

From: Thierry Volpiatto <thievol <at> posteo.net>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: Thierry Volpiatto <thievol <at> posteo.net>, 75354 <at> debbugs.gnu.org
Subject: Re: bug#75354: (29.4; eww buffer is not displayed correctly when
 used from bookmark-jump )
Date: Wed, 15 Jan 2025 16:15:56 +0000

[Message part 1 (text/plain, inline)]

Eli Zaretskii <eliz <at> gnu.org> writes:

>> From: Thierry Volpiatto <thievol <at> posteo.net>
>> Cc: Thierry Volpiatto <thievol <at> posteo.net>,  75354 <at> debbugs.gnu.org
>> Date: Wed, 15 Jan 2025 06:43:34 +0000
>> 
>> >> Tested here on many different kinds of bookmarks and work as expected
>> >> unlike the current code.
>> >
>> > OK, thanks.  Let's leave this for a few days to give people time to
>> > post comments if they have them.
>> 
>> Ping.
>
> I didn't forget, don't worry, just waiting for some free time to
> install.

Ok ;-)

-- 
Thierry

[signature.asc (application/pgp-signature, inline)]

Message #37 received at 75354-done <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Thierry Volpiatto <thievol <at> posteo.net>
Cc: 75354-done <at> debbugs.gnu.org
Subject: Re: bug#75354: (29.4; eww buffer is not displayed correctly when
 used from bookmark-jump )
Date: Thu, 16 Jan 2025 18:43:47 +0200

> From: Thierry Volpiatto <thievol <at> posteo.net>
> Cc: Thierry Volpiatto <thievol <at> posteo.net>,  75354 <at> debbugs.gnu.org
> Date: Fri, 10 Jan 2025 05:56:40 +0000
> 
> Find here the last patch attached.

Thanks, installed on the master branch, and closing the bug.

Message #40 received at 75354-done <at> debbugs.gnu.org (full text, mbox):

From: Thierry Volpiatto <thievol <at> posteo.net>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: Thierry Volpiatto <thievol <at> posteo.net>, 75354-done <at> debbugs.gnu.org
Subject: Re: bug#75354: (29.4; eww buffer is not displayed correctly when
 used from bookmark-jump )
Date: Thu, 16 Jan 2025 17:20:44 +0000

[Message part 1 (text/plain, inline)]

Eli Zaretskii <eliz <at> gnu.org> writes:

>> From: Thierry Volpiatto <thievol <at> posteo.net>
>> Cc: Thierry Volpiatto <thievol <at> posteo.net>,  75354 <at> debbugs.gnu.org
>> Date: Fri, 10 Jan 2025 05:56:40 +0000
>> 
>> Find here the last patch attached.
>
> Thanks, installed on the master branch, and closing the bug.

Great thanks!

-- 
Thierry

[signature.asc (application/pgp-signature, inline)]

Message #47 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Thierry Volpiatto <thievol <at> posteo.net>
To: Ship Mints <shipmints <at> gmail.com>
Cc: Thierry Volpiatto <thievol <at> posteo.net>, 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: Thu, 13 Mar 2025 19:29:13 +0000

[Message part 1 (text/plain, inline)]

Ship Mints <shipmints <at> gmail.com> writes:

>> From: Thierry Volpiatto <thievol <at> posteo.net>
>> Cc: Thierry Volpiatto <thievol <at> posteo.net>,  75354 <at> 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?

> 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.

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).
  

> 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.

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...

> I'm happy to provide a patch once we agree on an approach.
>
> -Stephane

-- 
Thierry

[signature.asc (application/pgp-signature, inline)]

Message #50 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: Thu, 13 Mar 2025 15:34:04 -0400

[Message part 1 (text/plain, inline)]

On Thu, Mar 13, 2025 at 3:21 PM Thierry Volpiatto <thievol <at> posteo.net>
wrote:

> Ship Mints <shipmints <at> gmail.com> writes:
>
> >> From: Thierry Volpiatto <thievol <at> posteo.net>
> >> Cc: Thierry Volpiatto <thievol <at> posteo.net>,  75354 <at> 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.

-Stephane

[Message part 2 (text/html, inline)]

Message #53 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: Thu, 13 Mar 2025 15:48:04 -0400

[Message part 1 (text/plain, inline)]

On Thu, Mar 13, 2025 at 3:34 PM Ship Mints <shipmints <at> gmail.com> wrote:

> On Thu, Mar 13, 2025 at 3:21 PM Thierry Volpiatto <thievol <at> posteo.net>
> wrote:
>
>> Ship Mints <shipmints <at> gmail.com> writes:
>>
>> >> From: Thierry Volpiatto <thievol <at> posteo.net>
>> >> Cc: Thierry Volpiatto <thievol <at> posteo.net>,  75354 <at> 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.

[Message part 2 (text/html, inline)]

Message #56 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: Thu, 13 Mar 2025 15:56:15 -0400

[Message part 1 (text/plain, inline)]

On Thu, Mar 13, 2025 at 3:48 PM Ship Mints <shipmints <at> gmail.com> wrote:

> On Thu, Mar 13, 2025 at 3:34 PM Ship Mints <shipmints <at> gmail.com> wrote:
>
>> On Thu, Mar 13, 2025 at 3:21 PM Thierry Volpiatto <thievol <at> posteo.net>
>> wrote:
>>
>>> Ship Mints <shipmints <at> gmail.com> writes:
>>>
>>> >> From: Thierry Volpiatto <thievol <at> posteo.net>
>>> >> Cc: Thierry Volpiatto <thievol <at> posteo.net>,  75354 <at> 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.
>

One other point on this is that bufferlo and activities users, can use
bookmark-bmenu-list to invoke these bookmarks and the new
save-window-excursion wrapper will interfere with those uses also.  We
could recommend that those users stick to the native package invocation
functions but that would be a limitation that bookmarks did not themselves
have.

[Message part 2 (text/html, inline)]

Message #59 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: Thu, 13 Mar 2025 16:45:46 -0400

[Message part 1 (text/plain, inline)]

On Thu, Mar 13, 2025 at 3:56 PM Ship Mints <shipmints <at> gmail.com> wrote:

> On Thu, Mar 13, 2025 at 3:48 PM Ship Mints <shipmints <at> gmail.com> wrote:
>
>> On Thu, Mar 13, 2025 at 3:34 PM Ship Mints <shipmints <at> gmail.com> wrote:
>>
>>> On Thu, Mar 13, 2025 at 3:21 PM Thierry Volpiatto <thievol <at> posteo.net>
>>> wrote:
>>>
>>>> Ship Mints <shipmints <at> gmail.com> writes:
>>>>
>>>> >> From: Thierry Volpiatto <thievol <at> posteo.net>
>>>> >> Cc: Thierry Volpiatto <thievol <at> posteo.net>,  75354 <at> 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.
>>
>
> One other point on this is that bufferlo and activities users, can use
> bookmark-bmenu-list to invoke these bookmarks and the new
> save-window-excursion wrapper will interfere with those uses also.  We
> could recommend that those users stick to the native package invocation
> functions but that would be a limitation that bookmarks did not themselves
> have.
>

Perhaps we can also consider introducing bookmark-record keys that
influence bookmark-jump behavior so specialized bookmark-record makers can
indicate what they need.

[Message part 2 (text/html, inline)]

Message #62 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Thierry Volpiatto <thievol <at> posteo.net>
To: Ship Mints <shipmints <at> gmail.com>
Cc: Thierry Volpiatto <thievol <at> posteo.net>, 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: Fri, 14 Mar 2025 04:53:49 +0000

[Message part 1 (text/plain, inline)]

Ship Mints <shipmints <at> gmail.com> writes:

>     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

After a quick look at source code of activities I see that it tries hard
to fix the bug this change fixed, there is even comments, so I suggest
these packages adapt their bookmark code to new bookmark--jump-via, IMO
this will reduce the code and make it simpler.


-- 
Thierry

[signature.asc (application/pgp-signature, inline)]

Message #65 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: Fri, 14 Mar 2025 06:50:02 -0400

[Message part 1 (text/plain, inline)]

On Fri, Mar 14, 2025 at 12:46 AM Thierry Volpiatto <thievol <at> posteo.net>
wrote:

> Ship Mints <shipmints <at> gmail.com> writes:
>
> >     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
>
> After a quick look at source code of activities I see that it tries hard
> to fix the bug this change fixed, there is even comments, so I suggest
> these packages adapt their bookmark code to new bookmark--jump-via, IMO
> this will reduce the code and make it simpler.
>

Thanks for taking a look.  Those are just the ones that immediately came to
mind.  There may be many other users.  I stand by my suggestions that we
either or both:

- Add defcustoms and/or defvars to control this new behavior that breaks
bookmarks.  bookmarks are *not* documented to be merely for files, not
documented to avoid window and frame behavior (some of our bookmarks also
perform frame operations compounding the issue with this new behavior).

- Add bookmark-record properties/keys to control bookmark behavior so that
bookmark-record-make functions can decide for themselves which behavior to
trigger.

- Add a defustom to set the default display-func so that
pop-to-buffer-same-window can be overridden.  I consider that this is
missing as an oversight in the implementation that does not forbid common
use cases for which bookmarks have been adopted.

I'm happy to submit patches for this to make everyone's bookmark use more
general and less painful.  In the same vein, I've submitted patches to
correct an oversight in window-state-get that makes using bookmark-records
that contain window-state objects more reliable.

Don't you agree this is a better approach than a wholesale behavior change?

[Message part 2 (text/html, inline)]

Message #68 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Thierry Volpiatto <thievol <at> posteo.net>
To: Ship Mints <shipmints <at> gmail.com>
Cc: Thierry Volpiatto <thievol <at> posteo.net>, 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: Fri, 14 Mar 2025 12:04:27 +0000

[Message part 1 (text/plain, inline)]

Ship Mints <shipmints <at> gmail.com> writes:

> 1.  ( ) text/plain          (*) text/html           
>
> On Fri, Mar 14, 2025 at 12:46 AM Thierry Volpiatto <thievol <at> posteo.net> wrote:
>
>     Ship Mints <shipmints <at> gmail.com> writes:
>    
>     >     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
>    
>     After a quick look at source code of activities I see that it tries hard
>     to fix the bug this change fixed, there is even comments, so I suggest
>     these packages adapt their bookmark code to new bookmark--jump-via, IMO
>     this will reduce the code and make it simpler.
>
> Thanks for taking a look.  Those are just the ones that immediately came to mind.  There may be many other users.  I stand by my suggestions that we either or both:
>
> - Add defcustoms and/or defvars to control this new behavior that breaks bookmarks.  bookmarks are *not* documented to be merely for files, not documented to avoid window and frame
> behavior (some of our bookmarks also perform frame operations compounding the issue with this new behavior).
>
> - Add bookmark-record properties/keys to control bookmark behavior so that bookmark-record-make functions can decide for themselves which behavior to trigger.

The old behavior was wrong, working by chance in many places, so no I
don't think bookmark handlers should relay on it anymore.

> - Add a defustom to set the default display-func so that pop-to-buffer-same-window can be overridden.  I consider that this is missing as an oversight in the implementation that does not
> forbid common use cases for which bookmarks have been adopted.
>
> I'm happy to submit patches for this to make everyone's bookmark use more general and less painful.  In the same vein, I've submitted patches to correct an oversight in window-state-get
> that makes using bookmark-records that contain window-state objects more reliable.
>
> Don't you agree this is a better approach than a wholesale behavior change?

No, I still think you should fix your code according to the new
behavior.  The old bookmark handlers (yours included) are implemented on
a buggy bookmark--jump-via, they should be rewrited to fit with new
code.  But this is just my opinion, let see what others think.

-- 
Thierry

[signature.asc (application/pgp-signature, inline)]

Message #71 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: Fri, 14 Mar 2025 16:39:44 -0400

[Message part 1 (text/plain, inline)]

On Fri, Mar 14, 2025 at 7:57 AM Thierry Volpiatto <thievol <at> posteo.net>
wrote:

> Ship Mints <shipmints <at> gmail.com> writes:
>
> > 1.  ( ) text/plain          (*) text/html
> >
> > On Fri, Mar 14, 2025 at 12:46 AM Thierry Volpiatto <thievol <at> posteo.net>
> wrote:
> >
> >     Ship Mints <shipmints <at> gmail.com> writes:
> >
> >     >     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
> >
> >     After a quick look at source code of activities I see that it tries
> hard
> >     to fix the bug this change fixed, there is even comments, so I
> suggest
> >     these packages adapt their bookmark code to new bookmark--jump-via,
> IMO
> >     this will reduce the code and make it simpler.
> >
> > Thanks for taking a look.  Those are just the ones that immediately came
> to mind.  There may be many other users.  I stand by my suggestions that we
> either or both:
> >
> > - Add defcustoms and/or defvars to control this new behavior that breaks
> bookmarks.  bookmarks are *not* documented to be merely for files, not
> documented to avoid window and frame
> > behavior (some of our bookmarks also perform frame operations
> compounding the issue with this new behavior).
> >
> > - Add bookmark-record properties/keys to control bookmark behavior so
> that bookmark-record-make functions can decide for themselves which
> behavior to trigger.
>
> The old behavior was wrong, working by chance in many places, so no I
> don't think bookmark handlers should relay on it anymore.
>
> > - Add a defustom to set the default display-func so that
> pop-to-buffer-same-window can be overridden.  I consider that this is
> missing as an oversight in the implementation that does not
> > forbid common use cases for which bookmarks have been adopted.
> >
> > I'm happy to submit patches for this to make everyone's bookmark use
> more general and less painful.  In the same vein, I've submitted patches to
> correct an oversight in window-state-get
> > that makes using bookmark-records that contain window-state objects more
> reliable.
> >
> > Don't you agree this is a better approach than a wholesale behavior
> change?
>
> No, I still think you should fix your code according to the new
> behavior.  The old bookmark handlers (yours included) are implemented on
> a buggy bookmark--jump-via, they should be rewrited to fit with new
> code.  But this is just my opinion, let see what others think.
>

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.  When
jumping to the outermost bookmark, that window-configuration that was saved
in that context gets restored when control returns to the outermost
call, frustrating attempts to manage these at lower levels.  These use
cases are legitimate uses of bookmarks and I have software that has
depended on bookmarks being able to contain any objects for a long time.

bookmark--jump-via save-window-excursion behavior needs to be optional.

Please.

I stand by my suggestions to handle this.

[Message part 2 (text/html, inline)]

Message #74 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Thierry Volpiatto <thievol <at> posteo.net>
To: Ship Mints <shipmints <at> gmail.com>
Cc: Thierry Volpiatto <thievol <at> posteo.net>, 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: Sat, 15 Mar 2025 05:44:42 +0000

[Message part 1 (text/plain, inline)]

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.

Thanks.

-- 
Thierry

[signature.asc (application/pgp-signature, inline)]

Message #77 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: Sat, 15 Mar 2025 10:18:34 -0400

[Message part 1 (text/plain, inline)]

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.

[Message part 2 (text/html, inline)]

Message #80 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: Sun, 16 Mar 2025 17:10:56 -0400

[Message part 1 (text/plain, inline)]

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.

-Stephane

[Message part 2 (text/html, inline)]

[0001-Consult-bookmark-property-inhibit-bookmark-jump-wind.patch (application/octet-stream, attachment)]

Message #83 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: Mon, 17 Mar 2025 06:35:10 -0400

[Message part 1 (text/plain, inline)]

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.

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.

[Message part 2 (text/html, inline)]

Message #86 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: Mon, 17 Mar 2025 06:41:32 -0400

[Message part 1 (text/plain, inline)]

On Mon, Mar 17, 2025 at 6:35 AM Ship Mints <shipmints <at> gmail.com> wrote:

> 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.
>
> 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)))))

[Message part 2 (text/html, inline)]

Message #89 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: Mon, 17 Mar 2025 06:44:02 -0400

[Message part 1 (text/plain, inline)]

On Mon, Mar 17, 2025 at 6:41 AM Ship Mints <shipmints <at> gmail.com> wrote:

> On Mon, Mar 17, 2025 at 6:35 AM Ship Mints <shipmints <at> gmail.com> wrote:
>
>> 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.
>>
>> 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))

[Message part 2 (text/html, inline)]

Message #92 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Thierry Volpiatto <thievol <at> posteo.net>
To: Ship Mints <shipmints <at> gmail.com>
Cc: Thierry Volpiatto <thievol <at> posteo.net>, 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 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)]

Message #95 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 11:02:35 -0400

[Message part 1 (text/plain, inline)]

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.  Perhaps that's a
matter of taste, and certainly not a dig at the idea.

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.

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?  We'd still need a method to inhibit display-func
in the interactive bookmark menu case.

-Stephane

[Message part 2 (text/html, inline)]

Message #98 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Thierry Volpiatto <thievol <at> posteo.net>
To: Ship Mints <shipmints <at> gmail.com>
Cc: Thierry Volpiatto <thievol <at> posteo.net>, 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 15:22:35 +0000

[Message part 1 (text/plain, inline)]

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.

>
>
> 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.

>   We'd still need a method to inhibit display-func in the interactive
> bookmark menu case.

No.


-- 
Thierry

[signature.asc (application/pgp-signature, inline)]

Message #101 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 11:27:42 -0400

[Message part 1 (text/plain, inline)]

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.  Perhaps a
little less 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.

>   We'd still need a method to inhibit display-func in the interactive
> > bookmark menu case.
>
> No.
>

If we do not go the cl-defgeneric route, we will.

What do the Emacs maintainers think about this as a matter of taste, easy
adoption for other bookmark users, and idiomatic usage?  I'm open.

[Message part 2 (text/html, inline)]

Message #104 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Thierry Volpiatto <thievol <at> posteo.net>
To: Ship Mints <shipmints <at> gmail.com>
Cc: Thierry Volpiatto <thievol <at> posteo.net>, 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 16:03:22 +0000

[Message part 1 (text/plain, inline)]

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.

-- 
Thierry

[signature.asc (application/pgp-signature, inline)]

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)]

Message #110 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 14:12:32 -0400

[Message part 1 (text/plain, inline)]

On Tue, Mar 18, 2025 at 12:28 Ship Mints <shipmints <at> gmail.com> wrote:

>
> 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.
>

I'll submit a patch to make that property into a list.  It was my code to
begin with and used only in shell-bookmark and I should have planned ahead.
Even if we don't use it for the above purposes.

>
>
>

[Message part 2 (text/html, inline)]

Message #113 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: Mon, 24 Mar 2025 15:16:20 -0400

[Message part 1 (text/plain, inline)]

On Tue, Mar 18, 2025 at 2:12 PM Ship Mints <shipmints <at> gmail.com> wrote:

> On Tue, Mar 18, 2025 at 12:28 Ship Mints <shipmints <at> gmail.com> wrote:
>
>>
>> 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.
>>
>
> I'll submit a patch to make that property into a list.  It was my code to
> begin with and used only in shell-bookmark and I should have planned ahead.
> Even if we don't use it for the above purposes.
>

I've attached a patch to bookmark-jump/bookmark--jump-via that supports
inhibiting window actions and display function using the 'bookmark-inhibit
handler function property list that Michael Albinus pushed to master last
week.

Usage for bookmark handler authors is simply:

(put #'xxx-bookmark-handler 'bookmark-inhibit '(insert
                                                jump-window-actions
                                                jump-display-func))

I tested this with a version of bufferlo waiting in the wings and it works
nicely.

If we want to add generic bookmark handler functions as a separate
enhancement, I'm all for that in the future.

-Stephane

[Message part 2 (text/html, inline)]

[0001-bookmark-jump-inhibit-window-actions-and-display-fun.patch (application/octet-stream, attachment)]

Message #116 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: Thu, 27 Mar 2025 08:11:39 -0400

[Message part 1 (text/plain, inline)]

On Mon, Mar 24, 2025 at 3:16 PM Ship Mints <shipmints <at> gmail.com> wrote:

> On Tue, Mar 18, 2025 at 2:12 PM Ship Mints <shipmints <at> gmail.com> wrote:
>
>> On Tue, Mar 18, 2025 at 12:28 Ship Mints <shipmints <at> gmail.com> wrote:
>>
>>>
>>> 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.
>>>
>>
>> I'll submit a patch to make that property into a list.  It was my code to
>> begin with and used only in shell-bookmark and I should have planned ahead.
>> Even if we don't use it for the above purposes.
>>
>
> I've attached a patch to bookmark-jump/bookmark--jump-via that supports
> inhibiting window actions and display function using the 'bookmark-inhibit
> handler function property list that Michael Albinus pushed to master last
> week.
>
> Usage for bookmark handler authors is simply:
>
> (put #'xxx-bookmark-handler 'bookmark-inhibit '(insert
>                                                 jump-window-actions
>                                                 jump-display-func))
>
> I tested this with a version of bufferlo waiting in the wings and it works
> nicely.
>
> If we want to add generic bookmark handler functions as a separate
> enhancement, I'm all for that in the future.
>

I'd like to see the latest approach I proposed installed.  We can add
fancier method overrides later.

We have a major version of bufferlo just about ready for release and I'd
like to ensure that it works well in Emacs 31/master.

We've already adopted many of the improvements in Emacs 31; including
expose-hidden-buffer, tab-bar fixes, window-state-get improvements,
define-ibuffer-op improvements.  For Emacs < 31, bufferlo advises
bookmark--jump-via to avoid display-func when called interactively but it
cannot easily avoid save-window-excursion in Emacs 31 without this patch.
Many users rely on bookmark-bmenu-list and bookmark-jump commands and we'd
love to be able to continue supporting those use cases.

Shall we, please?

-Stephane

[Message part 2 (text/html, inline)]

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.