From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.io!.POSTED.blaine.gmane.org!not-for-mail From: Daniel Mendler via "Bug reports for GNU Emacs, the Swiss army knife of text editors" Newsgroups: gmane.emacs.bugs Subject: bug#75052: 31; browse-url-transform-alist is not used by secondary browser Date: Fri, 03 Jan 2025 17:47:03 +0100 Message-ID: <87frlz3kvs.fsf@daniel-mendler.de> References: <87cyhi6y3p.fsf@daniel-mendler.de> <87zfkl28vf.fsf@daniel-mendler.de> Reply-To: Daniel Mendler Mime-Version: 1.0 Content-Type: text/plain Injection-Info: ciao.gmane.io; posting-host="blaine.gmane.org:116.202.254.214"; logging-data="7127"; mail-complaints-to="usenet@ciao.gmane.io" User-Agent: Gnus/5.13 (Gnus v5.13) Cc: 75052@debbugs.gnu.org To: Stefan Kangas Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Fri Jan 03 17:48:26 2025 Return-path: Envelope-to: geb-bug-gnu-emacs@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 1tTkqX-0001fk-QN for geb-bug-gnu-emacs@m.gmane-mx.org; Fri, 03 Jan 2025 17:48:26 +0100 Original-Received: from localhost ([::1] helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1tTkqE-0005Kf-Ob; Fri, 03 Jan 2025 11:48:07 -0500 Original-Received: from eggs.gnu.org ([2001:470:142:3::10]) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1tTkqB-0005KI-SS for bug-gnu-emacs@gnu.org; Fri, 03 Jan 2025 11:48:04 -0500 Original-Received: from debbugs.gnu.org ([2001:470:142:5::43]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1tTkqA-0007r8-Nu for bug-gnu-emacs@gnu.org; Fri, 03 Jan 2025 11:48:03 -0500 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=debbugs.gnu.org; s=debbugs-gnu-org; h=MIME-Version:Date:References:In-Reply-To:From:To:Subject; bh=19u/7gOH1wZT5xXVbI5uPjeOpBc50WhHuKN0UcRNLBM=; b=Q42BONngQiFzX5yGxNlKSk+jqW+7spEAGB1mxrETy94IOoZFA39Go4HUoc/eOugXX7TZLJP+1ATVD9eQe/5BEPT2DPiw8oYgXY8CqeEIGHo+iIwUlSk1vHL4Oqmk7ZOv/4243vUxGCHsHxcJbmeWTs9G6tFWUVVWIjM9JFnYX0BPgLjMzNJpqljAAjLfUNSfluEOa6QSm4VAI4d8/jLHPiN2rxs92ySiJ5hqhp62gSxKYY8MTvuFCC5BNa/O43PsaGaaMQrWrNX6knOoAti2VfoX0Dd+r27/bnPgKDmORsCYzBgp1W7UB/lzLrFv7+LpsPBJ6sPOMqbHyCAwkubMlA==; Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1tTkqA-00022w-Bg for bug-gnu-emacs@gnu.org; Fri, 03 Jan 2025 11:48:02 -0500 X-Loop: help-debbugs@gnu.org Resent-From: Daniel Mendler Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Fri, 03 Jan 2025 16:48:02 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 75052 X-GNU-PR-Package: emacs Original-Received: via spool by 75052-submit@debbugs.gnu.org id=B75052.17359228487692 (code B ref 75052); Fri, 03 Jan 2025 16:48:02 +0000 Original-Received: (at 75052) by debbugs.gnu.org; 3 Jan 2025 16:47:28 +0000 Original-Received: from localhost ([127.0.0.1]:51847 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1tTkpa-0001zs-RD for submit@debbugs.gnu.org; Fri, 03 Jan 2025 11:47:27 -0500 Original-Received: from server.qxqx.de ([2a01:4f8:c012:9177::1]:55211 helo=mail.qxqx.de) by debbugs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.84_2) (envelope-from ) id 1tTkpN-0001z8-7P for 75052@debbugs.gnu.org; Fri, 03 Jan 2025 11:47:25 -0500 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=daniel-mendler.de; s=key; h=Content-Type:MIME-Version:Message-ID:Date: References:In-Reply-To:Subject:Cc:To:From:Sender:Reply-To: Content-Transfer-Encoding:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Id: List-Help:List-Unsubscribe:List-Subscribe:List-Post:List-Owner:List-Archive; bh=19u/7gOH1wZT5xXVbI5uPjeOpBc50WhHuKN0UcRNLBM=; b=TAwYtEG+G162xviTG1+S3ajWeC dfYCFwQE6yqiS4FQdoI8f0vjqOOstzmJwhBw9qRVCJJL7ZLpaYjo1WUJgnAoTmeQY5Z5gH8/AUCU4 1DrN/1Aenau9iOguaaSNhUmCh9slJi/Yw9JvIrBPqBUZk6C4mftzubvKTZNk5iqK1UEA=; In-Reply-To: (Stefan Kangas's message of "Fri, 3 Jan 2025 10:18:09 -0600") X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list X-BeenThere: bug-gnu-emacs@gnu.org List-Id: "Bug reports for GNU Emacs, the Swiss army knife of text editors" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Original-Sender: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Xref: news.gmane.io gmane.emacs.bugs:298291 Archived-At: Stefan Kangas writes: > Daniel Mendler writes: > >> Hello Stefan, >> >> you recently added `browse-url-transform-alist' to `browse-url'. At a >> few places in Emacs browser functions are called directly. For direct >> browser function calls the transformation alist is not applied. >> >> Examples: >> >> - `browse-url-secondary-browser-function' is called by >> `browse-url-button-open' or by `package-browse-url' and others. >> - `browse-url-with-browser-kind' calls browser functions directly. > > Thanks for identifying this issue, and for the patch! > Sorry for the delay in reviewing it. Thanks for taking a look. The problem goes a little bit further even. `browse-url' contains some setup code (`setenv' etc), which is also relevant when invoking secondary browser functions. > I find the calls here rather unintuitive, for example I see calls like: > > (browse-url-with-function current-prefix-arg url) > (browse-url-with-function secondary url) > > The `current-prefix-arg' and `secondary' are not really functions, so > when I first read this code, I jumped at it and had to consult the > docstring. Maybe it would be more clear if the function is renamed to `browse-url-via`? Then the VIA argument could either be a function or a flag. (browse-url-via some-function url) (browse-url-via secondary url) The goal here is also to make it simpler for the caller to select browsers via a prefix argument, which happens at a few places. > I believe that the API would be simpler if we had two functions instead: > > (browse-url-with-function func url &rest args) > (browse-url-with-secondary-browser url &rest args) This would work too but would impose slightly more work on the callers, since they would then decide if they want to call `browse-url-with-secondary-browser' or `browse-url'. > We could also consider this idea: > >> An alternative could be to add keyword arguments to `browse-url' for >> consolidation: >> >> For example: >> (browse-url url :kind 'external) ;; replaces `browse-url-with-browser-kind' >> (browse-url url :via some-function) ;; alternative to `browse-url-with-function' >> (browse-url url :via secondary) > > First, it seems like `browse-url-with-browser-kind' is intended as a > user-facing command, so replacing it with a keyword argument to > browse-url won't work. Yes, indeed. > As for the rest, I'd rather use something more direct and boring, > because I find :via to be unclear. Here's what I'd do: > > (browse-url :function function) > (browse-url :secondary t) Okay. I find the :via quite readable, also the `browse-url-via` suggested above since it just follows the language flow. This is a matter of taste. I am not sure if we necessarily want to touch the `browse-url' calling convention. Another perhaps simpler and significantly more minimal approach would be to let-bind `browse-url-browser-function' to `browse-url-secondary-browser-function' at all call-sites instead of calling `browse-url-secondary-browser-function' directly. Inside the let, `browse-url' would be called as usual. However the problem with that approach is that it wouldn't be entirely backward compatible, since the `browse-url-handlers' take precedence over the `browse-url-browser-function'. In order to circumvent this problem we could introduce a new variable `browse-url-override-browser-function' which would take precedence in `browse-url'. `package-browse-url' would become the following: #+begin_src emacs-lisp (defun package-browse-url (desc &optional secondary) (interactive (list (package--query-desc) current-prefix-arg) package-menu-mode) (unless desc (user-error "No package here")) (let ((url (cdr (assoc :url (package-desc-extras desc)))) (browse-url-override-browser-function (and secondary browse-url-secondary-browser-function))) (unless url (user-error "No website for %s" (package-desc-name desc))) (browse-url url))) #+end_src However this small incompatibility may even be acceptable. In case URL handlers are defined, you may want them to take precedence in any case. I believe the problem is only pronounced in `browse-url-with-browser-kind' which under all circumstances wants to use the browser function it has chosen. In order to achieve that it could let-bind `browse-url-handlers' and `browse-url-default-handlers' to nil. The function would become this: #+begin_src emacs-lisp (defun browse-url-with-browser-kind (kind url &optional arg) (interactive ...) (let ((function (browse-url-select-handler url kind))) (unless function (setq function (seq-find (lambda (fun) (eq kind (browse-url--browser-kind fun url))) (list browse-url-browser-function browse-url-secondary-browser-function #'browse-url-default-browser #'eww)))) ;; Ensure that function is used (let ((browse-url-browser-function function) (browse-url-default-handlers nil) (browse-url-handlers)) (browse-url url arg)))) #+end_src Daniel