Reviewing GitHub PRs in Emacs

My Emacs config's todo-list has long had an item about finding some way to review GitHub PRs without having to leave Emacs and when the forge issue that I subscribe to came alive again I thought it was time to see if I can improve my config.

I found three packages for doing reviews

• code-review
• github-review
• emacs-pr-review

I've tried the first one before but at the time it didn't seem to work at all. Apparently that's improved somewhat, though there's a PR with a change that's necessary to make it work.¹ The first two don't support comments on multiple lines of a PR, there are issues/discussions for both

• code-review: Code suggestion on multiple lines
• github-review: Multi-line code comments

The last one, emacs-pr-review does support commenting on multiple lines, but it lacks a nice way of opening a review from magit. What I can do is

1. position the cursor on a PR in the magit status view, then
2. copy the the PR's URL using forge-copy-url-at-point-as-kill, and
3. open the PR by calling pr-review and pasting the PR's URL.

Which I did for a few days until I got tired of it and wrote a function to cut out they copy/paste part.

(defun mes/pr-review-via-forge ()
  (interactive)
  (if-let* ((target (forge--browse-target))
            (url (if (stringp target) target (forge-get-url target)))
            (rev-url (pr-review-url-parse url)))
      (pr-review url)
    (user-error "No PR to review at point")))

I've bound it to a key in magit-mode-map to make it easier.

I have to say I'm not completely happy with emacs-pr-review, so if either of the other two sort out commenting on multiple lines I'll check them out again.

My full setup for pr-review is here.

Servant and a weirdness in Keycloak

When writing a small tool to interface with Keycloak I found an endpoint that require the content type to be application/json while the body should be plain text. (The details are in the issue.) Since servant assumes that the content type and the content match (I know, I'd always thought that was a safe assumption to make too) it doesn't work with ReqBody '[JSON] Text. Instead I had to create a custom type that's a combination of JSON and PlainText, something that turned out to required surprisingly little code:

data KeycloakJSON deriving (Typeable)

instance Accept KeycloakJSON where
    contentType _ = "application" // "json"

instance MimeRender KeycloakJSON Text where
    mimeRender _ = fromStrict . encodeUtf8

The bug has already been fixed in Keycloak, but I'm sure there are other APIs with similar weirdness so maybe this will be useful to someone else.

Followup on secrets in my work notes

I got the following question on my post on how I handle secrets in my work notes:

Sounds like a nice approach for other secrets but how about :dbconnection for Orgmode and sql-connection-alist?

I have to admit I'd never come across the variable sql-connection-alist before. I've never really used sql-mode for more than editing SQL queries and setting up code blocks for running them was one of the first things I used yasnippet for.

I did a little reading and unfortunately it looks like sql-connection-alist can only handle string values. However, there is a variable sql-password-search-wallet-function, with the default value of sql-auth-source-search-wallet, so using auth-source is already supported for the password itself.

There seems to be a lack of good tutorials for setting up sql-mode in a secure way – all articles I found place the password in clear-text in the config – filling that gap would be a nice way to contribute to the Emacs community. I'm sure it'd prompt me to re-evaluate incorporating sql-mode in my workflow.

Improving how I handle secrets in my work notes

At work I use org-mode to keep notes about useful ways to query our systems, mostly that involves using the built-in SQL support to access DBs and ob-http to send HTTP requests. In both cases I often need to provide credentials for the systems. I'm embarrassed to admit it, but for a long time I've taken the easy path and kept all credentials in clear text. Every time I've used one of those code blocks I've thought I really ought to find a better way of handling these secrets one of these days. Yesterday was that day.

I ended up with two functions that uses auth-source and its ~/.authinfo.gpg file.

(defun mes/auth-get-pwd (host)
  "Get the password for a host (authinfo.gpg)"
  (-> (auth-source-search :host host)
      car
      (plist-get :secret)
      funcall))

(defun mes/auth-get-key (host key)
  "Get a key's value for a host (authinfo.gpg)

Not usable for getting the password (:secret), use 'mes/auth-get-pwd'
for that."
  (-> (auth-source-search :host host)
      car
      (plist-get key)))

It turns out that the library can handle more keys than the documentation suggests so for DB entries I'm using a machine (:host) that's a bit shorter and easier to remember than the full AWS hostname. Then I keep the DB host and name in dbhost (:dbhost) and dbname (:dbname) respectively. That makes an entry look like this:

machine db.svc login user port port password pwd dbname dbname dbhost dbhost

If I use it in a property drawer it looks like this

:PROPERTIES:
:header-args:sql: :engine postgresql
:header-args:sql+: :dbhost (mes/auth-get-key "db.svc" :dbhost)
:header-args:sql+: :dbport (string-to-number (mes/auth-get-key "db.svc" :port))
:header-args:sql+: :dbuser (mes/auth-get-key "db.svc" :user)
:header-args:sql+: :dbpassword (mes/auth-get-pwd "db.svc")
:header-args:sql+: :database (mes/auth-get-key "db.svc" :dbname)
:END:

Removing symlink question

I'm not sure why, but all of a sudden I started getting this question every time emacs starts

Symbolic link to Git-controlled source file; follow link?

After some searching I found out that it's VC asking. I'm guessing this comes from straight's very liberal use of symlinks. Though I'm still a little surprised at VC kicking in when reading the config.

Anyway, there are two variables to consider, vc-follow-symlinks and vc-handled-backends. I opted to modify the latter one, and since I don't use VC at all I'm turning it off completely.

(setopt vc-handled-backends nil)