From: Daniel Mendler via "Bug reports for GNU Emacs, the Swiss army knife of text editors" <bug-gnu-emacs@gnu.org>
To: Robert Pluim <rpluim@gmail.com>
Cc: 74781@debbugs.gnu.org
Subject: bug#74781: [PATCH] Add `browse-url-qutebrowser'
Date: Wed, 11 Dec 2024 10:07:08 +0100 [thread overview]
Message-ID: <87jzc61tc3.fsf@daniel-mendler.de> (raw)
In-Reply-To: <87ed2e8vzv.fsf@gmail.com> (Robert Pluim's message of "Wed, 11 Dec 2024 09:27:48 +0100")
Hi Robert,
thank you for looking at the patch. I have a few questions.
> Daniel> +(defcustom browse-url-qutebrowser-program "qutebrowser"
> Daniel> + "The name by which to invoke Qutebrowser."
> Daniel> + :type 'string)
> Daniel> +
> Daniel> +(defcustom browse-url-qutebrowser-arguments nil
> Daniel> + "A list of strings to pass to Qutebrowser when it starts up."
> Daniel> + :type '(repeat (string :tag "Argument")))
> Daniel> +
>
> :version tags please
Other similar variables have been added recently and lack the version
tags. I didn't add the tags for consistency with the other variables. I
wonder if the Emacs help system does recognize automatically when
variables got added?
> Daniel> (defcustom browse-url-new-window-flag nil
> Daniel> "Non-nil means always open a new browser window with appropriate browsers.
> Daniel> Passing an interactive argument to \\[browse-url], or specific browser
> Daniel> @@ -1294,6 +1308,60 @@ browse-url-epiphany-sentinel
> Daniel> browse-url-epiphany-program
> Daniel> (append browse-url-epiphany-startup-arguments (list url))))))
>
> Daniel> +(defun browse-url-qutebrowser-send (cmd)
> Daniel> + "Send CMD to Qutebrowser via IPC."
> Daniel> + (let* ((dir (getenv "XDG_RUNTIME_DIR"))
>
> We have `xdg-runtime-dir' in xdg.el
The goal was to avoid loading `xdg.el' unnecessarily for this trivial
function, which is just a wrapper around `getenv'. Do you suggest to use
`declare-function' and require xdg inside `browse-url-qutebrowser-send'?
> Daniel> + (sock (and dir (expand-file-name
> Daniel> + (format "qutebrowser/ipc-%s" (md5 (user-login-name)))
> Daniel> + dir))))
> Daniel> + (unless (file-exists-p sock)
> Daniel> + (error "No Qutebrowser IPC socket found"))
> Daniel> + (let ((proc
> Daniel> + (make-network-process
> Daniel> + :name "qutebrowser"
> Daniel> + :family 'local
> Daniel> + :service sock
> Daniel> + :coding 'utf-8)))
> Daniel> + (unwind-protect
> Daniel> + (process-send-string
> Daniel> + proc
> Daniel> + (concat
> Daniel> + (json-serialize `( :args [,cmd]
> Daniel> + :target_arg :null
> Daniel> + :protocol_version 1))
> Daniel> + "\n"))
> Daniel> + (delete-process proc)))))
> Daniel> +
>
> Perhaps Iʼm being paranoid, but maybe you should check the status of
> `proc' before sending it a string.
`process-send-string' throwns an error if opening the file failed, so a
status check won't be beneficial. Also note that `make-network-process'
fails if opening the socket file fails. Thanks again.
Daniel
next prev parent reply other threads:[~2024-12-11 9:07 UTC|newest]
Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-12-11 7:04 bug#74781: [PATCH] Add `browse-url-qutebrowser' Daniel Mendler via Bug reports for GNU Emacs, the Swiss army knife of text editors
2024-12-11 8:27 ` Robert Pluim
2024-12-11 9:07 ` Daniel Mendler via Bug reports for GNU Emacs, the Swiss army knife of text editors [this message]
2024-12-11 9:58 ` Robert Pluim
2024-12-11 10:45 ` Daniel Mendler via Bug reports for GNU Emacs, the Swiss army knife of text editors
2024-12-11 14:14 ` Björn Bidar via Bug reports for GNU Emacs, the Swiss army knife of text editors
2024-12-11 14:38 ` Daniel Mendler via Bug reports for GNU Emacs, the Swiss army knife of text editors
2024-12-11 16:16 ` Eli Zaretskii
2024-12-11 15:32 ` Eli Zaretskii
2024-12-11 15:36 ` Daniel Mendler via Bug reports for GNU Emacs, the Swiss army knife of text editors
2024-12-11 14:40 ` Eli Zaretskii
2024-12-11 15:12 ` Daniel Mendler via Bug reports for GNU Emacs, the Swiss army knife of text editors
2024-12-11 15:44 ` Robert Pluim
2024-12-11 15:54 ` Daniel Mendler via Bug reports for GNU Emacs, the Swiss army knife of text editors
2024-12-11 16:54 ` Eli Zaretskii
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
List information: https://www.gnu.org/software/emacs/
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=87jzc61tc3.fsf@daniel-mendler.de \
--to=bug-gnu-emacs@gnu.org \
--cc=74781@debbugs.gnu.org \
--cc=mail@daniel-mendler.de \
--cc=rpluim@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this public inbox
https://git.savannah.gnu.org/cgit/emacs.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).