Custom magit-section: assigned pull requests

[alexhenning]

Not an issue, but I thought it might be helpful for other people. This is an example how I created a custom magit-section for pull-requests assigned to me, so I can quickly focus in on the pull-requests that have been assigned to me:

Magit status buffer
...
Assigned pull requests (1)
#486 A PR that needs my attention

Pull requests (24)
#485 Awesome new PR with fancy features
...

Actual snippet:

(defun alexhenning/get-assigned-pullreqs (repo)
  (let* ((raw-ids (forge-sql [:select pullreq:id :from pullreq
                                      :inner :join pullreq_assignee :on (= pullreq_assignee:pullreq pullreq:id)
                                      :inner :join assignee :on (= pullreq_assignee:id assignee:id)
                                      :where (and (= pullreq:repository $s1) (isnull pullreq:closed) (= assignee:login $s2))
                                      :order-by [(desc updated)]]
                             (oref repo id) "alexhenning"))
         (ids (apply 'vector (mapcar 'car raw-ids)))
         (pullreqs (forge-sql [:select * :from 'pullreq
                                       :where id :in $v1
                                       :order-by [(desc updated)]]
                       ids)))
    (cl-sort (mapcar (lambda (row)
                       (closql--remake-instance 'forge-pullreq (forge-db) row))
                     pullreqs)
             (cdr forge-topic-list-order)
             :key (lambda (it) (eieio-oref it (car forge-topic-list-order))))))

(defun alexhenning/insert-assigned-pullreqs ()
  (when-let ((repo (forge-get-repository nil))
             (- (not (oref repo sparse-p)))
             (pullreqs (alexhenning/get-assigned-pullreqs (forge-get-repository nil))))
    (magit-insert-section (pullreqs nil t)
      (magit-insert-heading
        (format "%s (%s)"
                (propertize "Assigned pull requests" 'face 'magit-section-heading)
                (length pullreqs)))
      (magit-insert-section-body
        (let ((width (length (number-to-string (oref (car pullreqs) number))))
              (prefix (forge--topic-type-prefix (car pullreqs))))
          (dolist (pullreq pullreqs)
            (forge-insert-pullreq pullreq width prefix)))
        (insert ?\n)))))

(magit-add-section-hook 'magit-status-sections-hook 'alexhenning/insert-assigned-pullreqs 'forge-insert-pullreqs)

If there's interest, I can look at cleaning this up to contribute in a form that's a bit more customizable. I can see it being useful as either as assigned PRs with a customizable user or as a more generic way to add pull-request sections based on sql queries.

P.S. @tarsius thanks for forge, it's fantastic!

[vermiculus]

You should definitely submit this as a pull request 👍

As far as making it generic, I think it would be most beneficial/useful (without going overboard) to have defmethod implementations that retrieve a list of issues/pullreqs assigned to a user. The section-insertion code can then provide the currently-logged-in user.

[tarsius]

I plan to allow listing different subsets of topics eventually. So far I was planning on only supporting displaying those sets in separate tabulated-list-mode buffers (as you get if you press RET on "Issues" or "Pull requests"), but now I am also considering doing that directly in the status buffer. However instead of providing alternative section inserters I think I favor teaching the existing inserters to display different sets depending on settings that can set using configuration popups (similar to L and D for log and diff arguments) and to display the settings at the end of the list section heading.

You should definitely submit this as a pull request

So no, I don't think I would want to merge something like the above implementation at this time.

[vermiculus]

However instead of providing alternative section inserters I think I favor teaching the existing inserters to display different sets depending on settings that can set using configuration popups

There's something lost here though, no? The ability to have multiple groupings of issues/pullreqs is reasonable:

• PRs waiting on my review
• PRs for which I'm otherwise responsible
• PRs I've created

You could definitely go overboard on this and create for yourself a bad experience, but I wouldn't say the idea is overall a bad one.

[alexhenning]

I'm currently finding having all PRs + PRs assigned to me as two separate sections very useful for my workflow. @tarsius I don't think what you describe would cover this particular use-case very well. Thankfully, emacs is customizable, so I'll stick my solution in my .emacs for now.

I definitely look forward to be able filter pull-requests in the tabulated list view too. I've been wanting to filter on PR age, CI status, review status, and labels on some projects. I'm not sure what the best way is, but I'd be interested in contributing to making that happen.

[benjaminfrank]

This is really useful, thanks @alexhenning.
Our workflow uses certain labels, e.g., review required, ready to merge, etc.
Having multiple groups to track PR state is very helpful for me so I do agree with @vermiculus
#82 also looks also very interesting for such a workflow

[vermiculus]

I have a proposed implementation for this (vermiculus@fd30bbe), but I foolishly based it on #95 and I don't have the mental wherewithal at this time of night to figure out how to rebase it back onto master. If someone knows how to do this, I can open a PR for more concrete discussion. I think the patch I have is valuable in its own right (it reduces duplication) and it makes it trivial to write your own section-inserters.

Here's my log right now; I haven't a clue how to rebase generalize-topic-listing onto master such that post-context (i.e., #95) is not included in history. Otherwise, it'll have to wait until #95 is merged as to not include #95's commits in the new PR.

[tarsius]

Otherwise, it'll have to wait until #95 is merged

Shouldn't take too long. I am just trying to get something else done over the next 2-3 days.

[vermiculus]

100% understand 😄 Take your time.

I figured out my problem above – literally I wanted to "rebase [...] such that [commits] aren't included in history" – that's legit just an interactive rebase to drop the offending commits. Truly speaks to the empowerment of Magit that I would never have attempted this with git itself (and also a testament to how out-of-it I was by the end – life has pushed any open-source work I want to do into the wee hours of the night…)

A pull request with the specific changes is incoming; review at your leisure. In that PR, I will discuss how both the option proposed here (multiple sections with different configurations) and my understanding of your preferred option (one section whose selection of topics is configurable via defcustom) are both made trivial by the patch.

[vermiculus]

Ping to those subscribed to this issue: the above has been merged. Those changes should make it more straightforward to implement something like this. There's room enough for both approaches – probably with @tarsius's thought (below) as the default since it is more discoverable than hunting down section-inserters and throwing them into the status-sections-hook.

However instead of providing alternative section inserters I think I favor teaching the existing inserters to display different sets depending on settings that can set using configuration popups (similar to L and D for log and diff arguments) and to display the settings at the end of the list section heading.

[tarsius]

I've added new section inserters (forge-insert-assigned-{issues,pullreqs}) and commands (forge-list-assigned-{issues,pullreqs). They aren't on any hook resp. don't have a key binding for now.

[ztlevi]

@tarsius I got this when I try to use forge-insert-assigned-issues. Basically I just hook these functions and it turns out I need to set the user. But after I set the user, it doesn't give me these assigned sections.

          (magit-add-section-hook 'magit-status-sections-hook 'forge-insert-assigned-pullreqs nil t)
          (magit-add-section-hook 'magit-status-sections-hook 'forge-insert-assigned-issues nil t)

I'm on Ubuntu 16, Emacs 26.3
Packages are installed via straight, so my magit is on master branch with commit 8b3172fc495d83830573461f877ed390e6408e0b. Forge is also on master branch with commit 63cbf81.

[tarsius]

I was unable to reproduce that with the current version of ghub. Are you using the latest ghub? I also cannot see how this could happen and my guess is: outdated *.elc files. Try recompiling everything or at least ghub and forge.

@raxod502 Does straight have a "re-compile everything" command? (Borg has make clean all.)

[raxod502]

@tarsius Yes, M-x straight-rebuild-all.

[ztlevi]

You're right. After rebuild all packages, it works. Thanks!