vermiculus / magithub

**DEPRECATED - please use Forge instead!** -- Magit-based interfaces to GitHub
GNU General Public License v3.0
579 stars 63 forks source link

Show CI status for recent commits in log view #76

Open ChrisBlom opened 7 years ago

ChrisBlom commented 7 years ago

Showing the CI status for the current commit is really useful, i'd love to also be able to inspect the CI status of commits in the log view of magit. Currently I have a binding to open the relevant CI url to inspect this, but i'd prefer to view it within emacs.

I've tried to add a simple lookup to magit-log-wash-rev in magic-log.el to display the CI status after the commit, but is quite slow because it calls hub ci-status for each commit.

Is there a better way to add the CI status to the log?

(defun hub-status (hash)
  (pcase (s-trim (shell-command-to-string (concat "hub ci-status " hash)))
    ("success"   "OK  ")
    ("error"     "ERR ")
    ("failure"   "FAIL")
    ("pending"   "... ")
    ("no status" "    ")))

(cl-defun magit-log-wash-rev (style abbrev)
  (when (derived-mode-p 'magit-log-mode)
    (cl-incf magit-log-count))
  (looking-at (pcase style
                (`log        magit-log-heading-re)
                (`cherry     magit-log-cherry-re)
                (`module     magit-log-module-re)
                (`reflog     magit-log-reflog-re)
                (`stash      magit-log-stash-re)
                (`bisect-vis magit-log-bisect-vis-re)
                (`bisect-log magit-log-bisect-log-re)))
  (magit-bind-match-strings
      (hash msg refs graph author date gpg cherry _ refsub side) nil
    (let ((align (not (member "--stat" (cadr magit-refresh-args))))
          (non-graph-re (if (eq style 'bisect-vis)
                            magit-log-bisect-vis-re
                          magit-log-heading-re)))
      (magit-delete-line)
      ;; `git reflog show' output sometimes ends with an incomplete
      ;; element (which has no basis in the data stored in the file).
      (when (and (eq style 'reflog) (not date))
        (cl-return-from magit-log-wash-rev t))
      (magit-insert-section section
        (commit hash)
        (pcase style
          (`stash      (setf (magit-section-type section) 'stash))
          (`module     (setf (magit-section-type section) 'module-commit))
          (`bisect-log (setq hash (magit-rev-parse "--short" hash))))
        (when cherry
          (when (and (derived-mode-p 'magit-refs-mode)
                     magit-refs-show-commit-count)
            (insert (make-string magit-refs-indent-cherry-lines ?\s)))
          (insert (propertize cherry 'face (if (string= cherry "-")
                                               'magit-cherry-equivalent
                                             'magit-cherry-unmatched)))
          (insert ?\s))
        (when side
          (insert (propertize side 'face (if (string= side "<")
                                             'magit-cherry-equivalent
                                           'magit-cherry-unmatched)))
          (insert ?\s))
        (when align
          (insert (propertize hash 'face 'magit-hash) ?\s))
        (when graph
          (insert graph))
        (unless align
          (insert (propertize hash 'face 'magit-hash) ?\s))
        (insert
         (propertize (hub-status hash) 'face 'magit-hash))
        (when (and refs (not magit-log-show-refname-after-summary))
          (insert (magit-format-ref-labels refs) ?\s))
        (when refsub
          (insert (format "%-2s " (1- magit-log-count)))
          (insert (magit-reflog-format-subject
                   (substring refsub 0 (if (string-match-p ":" refsub) -2 -1)))))
        (when msg
          (insert (propertize msg 'face
                              (pcase (and gpg (aref gpg 0))
                                (?G 'magit-signature-good)
                                (?B 'magit-signature-bad)
                                (?U 'magit-signature-untrusted)
                                (?X 'magit-signature-expired)
                                (?Y 'magit-signature-expired-key)
                                (?R 'magit-signature-revoked)
                                (?E 'magit-signature-error)))))
        (when (and refs magit-log-show-refname-after-summary)
          (insert ?\s)
          (insert (magit-format-ref-labels refs)))
        (insert ?\n)
        (when (memq style '(log reflog stash))
          (goto-char (line-beginning-position))
          (when (and refsub
                     (string-match "\\`\\([^ ]\\) \\+\\(..\\)\\(..\\)" date))
            (setq date (+ (string-to-number (match-string 1 date))
                          (* (string-to-number (match-string 2 date)) 60 60)
                          (* (string-to-number (match-string 3 date)) 60))))
          (save-excursion
            (backward-char)
            (magit-log-format-margin author date)))
        (when (and (eq style 'cherry)
                   (magit-buffer-margin-p))
          (save-excursion
            (backward-char)
            (apply #'magit-log-format-margin
                   (split-string (magit-rev-format "%aN%x00%ct") "\0"))))
        (when (and graph
                   (not (eobp))
                   (not (looking-at non-graph-re)))
          (when (looking-at "")
            (magit-insert-heading)
            (delete-char 1)
            (magit-insert-section (commit-header)
              (forward-line)
              (magit-insert-heading)
              (re-search-forward "")
              (backward-delete-char 1)
              (forward-char)
              (insert ?\n))
            (delete-char 1))
          (if (looking-at "^\\(---\\|\n\s\\|\ndiff\\)")
              (let ((limit (save-excursion
                             (and (re-search-forward non-graph-re nil t)
                                  (match-beginning 0)))))
                (unless (magit-section-content magit-insert-section--current)
                  (magit-insert-heading))
                (delete-char (if (looking-at "\n") 1 4))
                (magit-diff-wash-diffs (list "--stat") limit))
            (when align
              (setq align (make-string (1+ abbrev) ? )))
            (when (and (not (eobp)) (not (looking-at non-graph-re)))
              (when align
                (setq align (make-string (1+ abbrev) ? )))
              (while (and (not (eobp)) (not (looking-at non-graph-re)))
                (when align
                  (save-excursion (insert align)))
                (magit-make-margin-overlay)
                (forward-line))
              ;; When `--format' is used and its value isn't one of the
              ;; predefined formats, then `git-log' does not insert a
              ;; separator line.
              (save-excursion
                (forward-line -1)
                (looking-at "[-_/|\\*o. ]*"))
              (setq graph (match-string 0))
              (unless (string-match-p "[/\\]" graph)
                (insert graph ?\n))))))))
  t)
vermiculus commented 7 years ago

I don't know of a good way to advise a function such that it 'inserts' functionality (as you do with magit-log-wash-rev), but I'll note that getting the statuses for a given commit has been made much simpler (and no longer relies on hub!).

I'm not sure how I feel about redefining magit functions. @tarsius, do you think there's a better way to insert a column into the reflog? Maybe a system similar to how sections are inserted into magit-status?

Otherwise, we could always just have our own view in the meantime that shows the SHA, the commit message, and the overall status at that point.

ChrisBlom commented 7 years ago

@vermiculus i agree that overriding magit function is ugly, it was just the only way i could get it working at the time

If someone could explain to me how to insert a column in the reflog, I can take a shot at adding the CI status to the reflog without overriding magit's functions.

vermiculus commented 7 years ago

I'm not saying there's a better way; just that it's too fragile to release 😄 It's still a clever idea – I just hope there's support for it. We'll wait and see what tarsius thinks. A PR to magit to support this kind of thing is probably the only way we'll be able to do it as a plugin.