From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.io!.POSTED.blaine.gmane.org!not-for-mail From: Stefan Monnier Newsgroups: gmane.emacs.devel Subject: Re: [PATCH] Let bookmark handlers do everything, including display the destination Date: Mon, 29 Aug 2022 11:28:37 -0400 Message-ID: References: <8735dt5si6.fsf@red-bean.com> Mime-Version: 1.0 Content-Type: text/plain Injection-Info: ciao.gmane.io; posting-host="blaine.gmane.org:116.202.254.214"; logging-data="25421"; mail-complaints-to="usenet@ciao.gmane.io" User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/29.0.50 (gnu/linux) Cc: Drew Adams , "emacs-devel@gnu.org" To: Karl Fogel Original-X-From: emacs-devel-bounces+ged-emacs-devel=m.gmane-mx.org@gnu.org Mon Aug 29 17:29:55 2022 Return-path: Envelope-to: ged-emacs-devel@m.gmane-mx.org Original-Received: from lists.gnu.org ([209.51.188.17]) by ciao.gmane.io with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1oSgi0-0006Ja-Kz for ged-emacs-devel@m.gmane-mx.org; Mon, 29 Aug 2022 17:29:52 +0200 Original-Received: from localhost ([::1]:60840 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1oSghz-0000To-Kl for ged-emacs-devel@m.gmane-mx.org; Mon, 29 Aug 2022 11:29:51 -0400 Original-Received: from eggs.gnu.org ([2001:470:142:3::10]:40764) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1oSggv-0007sv-9n for emacs-devel@gnu.org; Mon, 29 Aug 2022 11:28:45 -0400 Original-Received: from mailscanner.iro.umontreal.ca ([132.204.25.50]:20055) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1oSggs-00029b-DE for emacs-devel@gnu.org; Mon, 29 Aug 2022 11:28:44 -0400 Original-Received: from pmg3.iro.umontreal.ca (localhost [127.0.0.1]) by pmg3.iro.umontreal.ca (Proxmox) with ESMTP id 0D64944156F; Mon, 29 Aug 2022 11:28:41 -0400 (EDT) Original-Received: from mail01.iro.umontreal.ca (unknown [172.31.2.1]) by pmg3.iro.umontreal.ca (Proxmox) with ESMTP id 43C2244088B; Mon, 29 Aug 2022 11:28:39 -0400 (EDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=iro.umontreal.ca; s=mail; t=1661786919; bh=AHqGNZ/p9LC8RY9kk96zvxnNl7v0WEYcXOg9KiHB/zM=; h=From:To:Cc:Subject:In-Reply-To:References:Date:From; b=lL7Q8C0D+Ybe+BD3Wd97ErB34UBv/V+ANCdvTj/fjzLw6HdWXKcjbUdzA/AAhUNYG g+JJLbp2+0O/qLrqec5no8ImssarFaA8odMt1BKVTmgXTvQaLTGH9GxujCj5T7fq5i X1/UyX2m0PIby3hl6ZaCFCSa3phTvH/Ve22liuR38cHxdusAwBq+mWN8dbtA/AcRnf VdG8x3MYZkD0bzDkZlDRsxEKRwBS8BwZc8yxpNhe1uZhTDJD2tylnCiccOybYy7d9G TXajupEYAVDcuYrHdebmEO5syD6/8nKNpYILBS1S6JW7/fmW+QI6EwiNHHSgJDPK5H Y6ZmkL5G1VvWw== Original-Received: from pastel (unknown [45.72.195.111]) by mail01.iro.umontreal.ca (Postfix) with ESMTPSA id 0090E1203D5; Mon, 29 Aug 2022 11:28:38 -0400 (EDT) In-Reply-To: <8735dt5si6.fsf@red-bean.com> (Karl Fogel's message of "Thu, 18 Aug 2022 13:14:09 -0500") Received-SPF: pass client-ip=132.204.25.50; envelope-from=monnier@iro.umontreal.ca; helo=mailscanner.iro.umontreal.ca X-Spam_score_int: -42 X-Spam_score: -4.3 X-Spam_bar: ---- X-Spam_report: (-4.3 / 5.0 requ) BAYES_00=-1.9, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, RCVD_IN_DNSWL_MED=-2.3, SPF_HELO_NONE=0.001, SPF_PASS=-0.001, T_SCC_BODY_TEXT_LINE=-0.01 autolearn=ham autolearn_force=no X-Spam_action: no action X-BeenThere: emacs-devel@gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: "Emacs development discussions." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: emacs-devel-bounces+ged-emacs-devel=m.gmane-mx.org@gnu.org Original-Sender: "Emacs-devel" Xref: news.gmane.io gmane.emacs.devel:294286 Archived-At: >>Specifically, instead of `bookmark--jump-via' systematically >>invoking its DISPLAY-FUNCTION argument, it saves the >>argument value in variable `bookmark-jump-display-function'. >>And then the default handler, `bookmark-default-handler' >>invokes that function. > > *nod* > > There's some parallelism-unsafety going on here, obviously. I'd love a way > to wrap all this in a clean closure, instead of setting a global variable > which then retains that setting until the next time we happen to set it. Indeed :-( > But I don't really see a way to do tihngs more cleanly than with the > carry-out variable you propose. If there's some creative solution for > connecting `bookmark--jump-via' to the "corresponding" handler call that > happens later, I don't see it, lexically or dynamically. Maybe is just how > Emacs usually handles these kinds of situations? Hmm... I must be missing something. Why can't `bookmark--jump-via` let-bind `bookmark-jump-display-function` around the call to `bookmark-handle-bookmark`? Regarding the patch, here are some comments: > + To display the destination, HANDLER can call the function that's the > + value of variable `bookmark-jump-display-function', which is set by > + `bookmark-jump' to automatically accommodate other-window etc. > + displaying that depends on the jump command. For example: > + > + (funcall bookmark-jump-display-function (current-buffer)) > + > + Or HANDLER can directly call another display function. For example: > + > + (switch-to-buffer-other-tab BUFFER) I skipped most of the rest of the docstring's massaging, which I'm not sure is an improvement but with which I guess I can live. But the above goes significantly beyond what one usually expects from a docstring. It would fit much better in a Texinfo manual instead. > + Or HANDLER can invoke `bookmark-default-handler'. That displays the > + destination. It then moves to the recorded buffer position, POS, > + repositioning point, if necessary, to match the recorded context > + strings STR-BEFORE-POS and STR-AFTER-POS. For instance, the Info > + handler, `Info-bookmark-jump', does this at its end: And this is (or should be) redundant with the docstring of `bookmark-default-handler` so better keep the link and the the readers jump to it when they need/want to know what that function does. > +(defvar bookmark-jump-display-function nil > + "Function used currently to display a bookmark, or nil if no function.") Please give it a function as default value (e.g. `ignore`). > @@ -1350,6 +1391,9 @@ > ((and buf (get-buffer buf))) > (t ;; If not, raise error. > (signal 'bookmark-error-no-filename (list 'stringp file))))) > + (when bookmark-jump-display-function > + (save-current-buffer (funcall bookmark-jump-display-function > + (current-buffer)))) Why `save-current-buffer`? > (if place (goto-char place)) > ;; Go searching forward first. Then, if forward-str exists and > ;; was found in the file, we can search backward for behind-str. Hmm... `bookmark-default-handler` is also called via things like `bookmark-insert`, so I think `bookmark-insert` should explicitly bind `bookmark-jump-display-function` to `ignore`, and of course that begs the question of what to do for handlers which don't obey `bookmark-jump-display-function`. I think it's good to provide more control to the bookmark's handler, but there seems to be a need for the caller to control the display as well to some extent. BTW, your generalization of bookmarks to "anything" makes it all the more obvious to me that these should be unified with "registers" (not necessarily at the UI level, but a register should be able to hold anything that can stored as a bookmark, and vice versa). Stefan