Re: Washing GitHub emails to include inline patch?

[Kyle Meyer <kyle@kyleam.com>]

William Casarin <jb55@jb55.com> writes:

> I now mainly review GitHub PRs via the patch fetching snippet you sent,
> but there's a tedious disconnect between notmuch and magit. In the above
> scenario, I would have to:
>
>   1. cd to the project
>   2. open magit
>   2. fetch
>   3. checkout the branch
>   4. start reviewing
>
> If I want to start a review on GitHub:
>
>   5. go back to the mail buffer
>   6. open the GitHub link
>   7. try to find lines where I had a comment
>   8. make comment, start review
>
> Steps 1-4 are way too long for the number of code reviews I do at work
> and on public projects.

Fair enough.  5-8 are my pain points.

> Perhaps I could write a script that quickly jump from steps 1 to 4. I
> think this would still be too slow compared to simply fetching the patch
> and having a nicer view, at least until we have full magit code reviews.
> I guess I could just wait until that's finished.

I think you could get 1-4 nearly as quick as calling the patch fetching
command.  How about something like this?

--8<---------------cut here---------------start------------->8---

(defun km/notmuch-github-pr-number ()
  "Return the PR number for this message."
  (let (pr)
    (with-current-notmuch-show-message
     (goto-char (point-min))
     (if (re-search-forward "https://github\\.com/.*/pull/\\([0-9]+\\)" nil t)
         (setq pr (match-string-no-properties 1))
       (user-error "Could not find PR number")))
    pr))

;; This function could be anything that figures out the project based
;; on the current notmuch message.  Or, if you use projectile and
;; don't mind getting queried each time, it could just read a project
;; from `projectile-relevant-known-projects'.
(defun km/notmuch-repo-from-message ()
  "Return the repository that this message is associated with."
  (let ((fname (notmuch-show-get-filename)))
    (or (and fname
             (string-match-p "magit-list" fname)
             "~/src/emacs/magit")
        (user-error "Could not determine repo"))))

(defun km/notmuch-visit-pr-in-magit (&optional dont-fetch)
  "Show the Magit log for this message's PR.
If DONT-FETCH is non-nil, do not fetch first."
  (interactive "P")
  (let* ((pr (km/notmuch-github-pr-number))
         (repo (km/notmuch-repo-from-message))
         (default-directory repo))
    ;; "origin" is hard-coded below, but it could of course be
    ;; anything.  You could also have an alist that maps repo ->
    ;; remote.
    ;;
    ;; This assumes that you've added
    ;;
    ;;    fetch = +refs/pull/*/head:refs/pull/origin/*
    ;;
    ;; to origin's in ".git/config".  You could drop that assumption
    ;; passing a more explicit refspec to the fetch call.
    (unless dont-fetch
      (magit-call-git "fetch" "origin"))
    (magit-log (list (concat "master..refs/pull/origin/" pr)))))
--8<---------------cut here---------------end--------------->8---

Call M-x km/notmuch-visit-pr-in-magit in a notmuch-show buffer for a
GitHub PR, modifying km/notmuch-repo-from-message so that it returns the
full path to the local repository.

-- 
Kyle