bug#74781: [PATCH] Add `browse-url-qutebrowser'

[Eli Zaretskii]

> Date: Wed, 11 Dec 2024 08:04:29 +0100
> From:  Daniel Mendler via "Bug reports for GNU Emacs,
>  the Swiss army knife of text editors" <bug-gnu-emacs@gnu.org>
> 
> The browser launcher supports the NEW-WINDOW argument and
> `browse-url-qutebrowser-new-window-is-tab' to open tabs.  Furthermore
> opening new URLs is speed up via Unix socket IPC if available.

Thanks.

> Furthermore opening new URLs is speed up via Unix socket IPC if
> available.                      ^^^^^^^^

Typo: "sped up"

> (browse-url-qutebrowser-program, browse-url-qutebrowser-arguments,
> browse-url-qutebrowser-new-window-is-tab): New customizables.

Our conventions are to format these kinds of identifier lists
differently:

  (browse-url-qutebrowser-program, browse-url-qutebrowser-arguments)
  (browse-url-qutebrowser-new-window-is-tab): New customizables.

IOW, the list should be closed at EOL, and then reopened on the next
line.  This makes searching for changes of a function/variable easier.

> +(defcustom browse-url-qutebrowser-program "qutebrowser"
> +  "The name by which to invoke Qutebrowser."
> +  :type 'string)

Please add a :version tag.

> +(defcustom browse-url-qutebrowser-arguments nil
> +  "A list of strings to pass to Qutebrowser when it starts up."
> +  :type '(repeat (string :tag "Argument")))

Same here.

> +(defcustom browse-url-qutebrowser-new-window-is-tab nil
> +  "Whether to open up new windows in a tab or a new window.

The first line of this doc string should mention Qutebrowser, so that
the various apropos commands could find this variable when the user
wants to look up something Qutebrowser-related.

> +If non-nil, then open the URL in a new tab rather than a new window if
> +`browse-url-qutebrowser' is asked to open it in a new window."

The "is asked to open it in a new window" part confused me: asked by
whom?  Is this something that Emacs does, or something else?

> +  :type 'boolean)

The :version tag is missing.

> +(defun browse-url-qutebrowser-send (cmd)
> +  "Send CMD to Qutebrowser via IPC."
> +  (let* ((dir (getenv "XDG_RUNTIME_DIR"))
                 ^^^^^^^^^^^^^^^^^^^^^^^^^^
Why not use xdg-runtime-dir instead?

> +         (sock (and dir (expand-file-name
> +                         (format "qutebrowser/ipc-%s" (md5 
> (user-login-name)))
> +                         dir))))

I think Qutebrowser is available on Windows, where we don't (yet)
support local sockets.  So I think there should be some kind of test
for running on Windows, and falling back to alternatives.

Last, but not least: I think the fact that Emacs will now support
Qutebrowser warrants a NEWS entry.