Skip to content

Support more rendering customizations #11

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Support more rendering customizations #11

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
4014652
Support more rendering customizations
gavv Mar 15, 2025
a9508ff
Diff rendering customizations
gavv Mar 15, 2025
907798c
Add space to prompt
gavv Mar 15, 2025
e4de8f7
Use defcustom instead of defvar
gavv Mar 15, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
69 changes: 67 additions & 2 deletions pr-review-common.el
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,11 @@
"Face used for author names."
:group 'pr-review)

(defface pr-review-check-face
'((t :inherit pr-review-author-face))
"Face used for check names."
:group 'pr-review)

(defface pr-review-timestamp-face
'((t :slant italic))
"Face used for timestamps."
Expand All @@ -80,7 +85,7 @@
:group 'pr-review)

(defface pr-review-thread-item-title-face
'((t :inherit bold))
'((t :inherit magit-section-secondary-heading))
"Face used for title of review thread item."
:group 'pr-review)

Expand All @@ -89,21 +94,55 @@
"Face used for the beginning of thread diff hunk."
:group 'pr-review)

(defface pr-review-thread-diff-body-face
'((t))
"Extra face added to the body of thread diff hunk."
:group 'pr-review)

(defface pr-review-thread-diff-end-face
'((t :overline t :extend t :inherit font-lock-comment-face))
"Face used for the beginning of thread diff hunk."
:group 'pr-review)

(defface pr-review-thread-comment-face
'((t))
"Extra face added to review thread comments."
:group 'pr-review)

(defface pr-review-in-diff-thread-title-face
'((t :inherit font-lock-comment-face))
"Face used for the title of the in-diff thread title."
"Face used for the title of the in-diff review-thread-link title.
Used if `pr-review-always-use-blocks' is nil (default)."
:group 'pr-review)

(defface pr-review-in-diff-thread-begin-face
'((t :underline t :extend t :inherit bold-italic))
"Face used for start line of review-thread-link block in the diff.
Used if `pr-review-always-use-blocks' is t."
:group 'pr-review)

(defface pr-review-in-diff-thread-body-face
'((t))
"Extra face added to the comment body of review-thread-link block in the diff.
Used if `pr-review-always-use-blocks' is t."
:group 'pr-review)

(defface pr-review-in-diff-thread-end-face
'((t :overline t :extend t :height 0.5 :inherit bold-italic))
"Face used for end line of review-thread-link block in the diff.
Used if `pr-review-always-use-blocks' is t."
:group 'pr-review)

(defface pr-review-in-diff-pending-begin-face
'((t :underline t :extend t :inherit bold-italic))
"Face used for start line of pending-thread in the diff."
:group 'pr-review)

(defface pr-review-in-diff-pending-body-face
'((t))
"Extra face added to the comment body of pending-thread in the diff."
:group 'pr-review)

(defface pr-review-in-diff-pending-end-face
'((t :overline t :extend t :height 0.5 :inherit bold-italic))
"Face used for end line of pending-thread in the diff."
Expand Down Expand Up @@ -163,5 +202,31 @@
:type 'regexp
:group 'pr-review)

(defcustom pr-review-diff-font-lock-syntax 'hunk-also
"This value is assigned to `diff-font-lock-syntax' to fontify hunk with diff-mode.
Set to nil to disable source language syntax highlighting."
:type '(choice (const :tag "Don't highlight syntax" nil)
(const :tag "Hunk-based only" hunk-only)
(const :tag "Highlight syntax" t)
(const :tag "Allow hunk-based fallback" hunk-also))
:group 'pr-review)

(defcustom pr-review-diff-hunk-limit 4
"Maximum number of lines shown for diff hunks in review threads."
:type 'number
:group 'pr-review)

(defcustom pr-review-always-use-blocks nil
"Always use multi-line blocks, even for elements that are usually on one line.
With this option, in-diff review thread links becomes blocks."
:type 'boolean
:group 'pr-review)

(defcustom pr-review-always-use-buttons nil
"Always use separate buttons, and don't make blocks clickable.
With this option, buttons are added to diff snippets inside review threads."
:type 'boolean
:group 'pr-review)

(provide 'pr-review-common)
;;; pr-review-common.el ends here
167 changes: 118 additions & 49 deletions pr-review-render.el
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -115,13 +115,15 @@
(t
(shr-generic td)))))))))

(defun pr-review--insert-html (body &optional indent)
(defun pr-review--insert-html (body &optional indent extra-face)
"Insert html content BODY.
INDENT is an optional number, if provided,
INDENT count of spaces are added at the start of every line."
INDENT count of spaces are added at the start of every line.
If EXTRA-FACE is given, it is added to the inserted text in addition to other faces."
(let ((shr-indentation (* (or indent 0) pr-review--char-pixel-width))
(shr-external-rendering-functions '((div . pr-review--shr-tag-div)))
(start (point))
end
dom)
(with-temp-buffer
(insert body)
Expand All @@ -133,7 +135,8 @@ INDENT count of spaces are added at the start of every line."
(goto-char start)
(shr-insert-document dom)
;; delete the inserted " "
(delete-char 1))
(delete-char 1)
(setq end (point)))
(when (> shr-indentation 0)
;; shr-indentation does not work for images and code block
;; let's fix it: prepend space for any lines that does not starts with a space
Expand All @@ -145,7 +148,10 @@ INDENT count of spaces are added at the start of every line."
(eq 'space (car-safe (get-text-property (point) 'display))))
(beginning-of-line)
(insert (propertize " " 'display `(space :width (,shr-indentation)))))
(forward-line))))))
(forward-line))))
;; additional face for the inserted region
(when extra-face
(pr-review--decorate-region start end extra-face))))

(defun pr-review--fontify (body lang-mode &optional margin)
"Fontify content BODY as LANG-MODE, return propertized string.
Expand All @@ -154,7 +160,7 @@ MARGIN count of spaces are added at the start of every line."
(with-current-buffer
(get-buffer-create (format " *pr-review-fontification:%s*" lang-mode))
(let ((inhibit-modification-hooks nil)
(diff-font-lock-syntax 'hunk-also))
(diff-font-lock-syntax pr-review-diff-font-lock-syntax))
(erase-buffer)
(insert "\n" ;; insert a newline at first line (and ignore later)
;; to workaround markdown metadata syntax: https://github.com/jrblevin/markdown-mode/issues/328
Expand Down Expand Up @@ -191,11 +197,29 @@ MARGIN count of spaces are added at the start of every line."
(setq res (replace-regexp-in-string (rx bol) (make-string margin ?\s) res)))
res)))


(defun pr-review--insert-fontified (body lang-mode &optional margin)
(defun pr-review--insert-fontified (body lang-mode &optional margin extra-face)
"Fontify BODY as LANG-MODE with MARGIN and insert it, see `pr-review--fontify'."
(insert (pr-review--fontify body lang-mode margin)))

(let ((start (point))
end)
(insert (pr-review--fontify body lang-mode margin))
(setq end (point))
(when extra-face
(pr-review--decorate-region start end extra-face))))

(defun pr-review--decorate-region (start end face)
"Add extra FACE to the region in buffer from START to END."
(let ((inhibit-read-only t))
(save-excursion
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it seems that add-face-text-property does exactly what this does

(goto-char start)
(while (< (point) end)
(let ((next-change (or (next-single-property-change (point) 'face (current-buffer) end)
end)))
(let* ((old-faces (get-text-property (point) 'face))
(new-faces (if (listp old-faces)
(append old-faces (list face))
(list old-faces face))))
(put-text-property (point) next-change 'face new-faces)
(goto-char next-change)))))))

(defun pr-review--insert-diff (diff)
"Insert pull request diff DIFF, wash it using magit."
Expand Down Expand Up @@ -294,7 +318,8 @@ It will be inserted at the beginning."
(format "%s:%s" .side .line))
"\n")
'face 'pr-review-in-diff-pending-begin-face))
(pr-review--insert-fontified .body 'gfm-mode)
(pr-review--insert-fontified .body 'gfm-mode nil
'pr-review-in-diff-pending-body-face)
(insert (propertize " \n" 'face 'pr-review-in-diff-pending-end-face))
(setq end (point))))
(when beg
Expand All @@ -309,30 +334,61 @@ It will be inserted at the beginning."
(save-excursion
(when (pr-review--goto-diff-line
.path .diffSide .line)
(forward-line)
(insert
(propertize
(concat (format "> %s comments from " (length .comments.nodes))
(string-join
(seq-uniq
(mapcar (lambda (cmt)
(concat "@" (alist-get 'login (alist-get 'author cmt))))
.comments.nodes))
", ")
(when .isResolved " - RESOLVED")
" ")
'face 'pr-review-in-diff-thread-title-face
'pr-review-eldoc-content (let-alist (car .comments.nodes)
(concat (pr-review--propertize-username .author.login)
": " .body))))
(insert-button
"Go to thread"
'face 'pr-review-button-face
'action (lambda (_)
(push-mark)
(pr-review--goto-section-with-value .id)
(recenter)))
(insert (propertize "\n" 'face 'pr-review-in-diff-thread-title-face)))))))
(let ((text
(format "%s comments from %s"
(length .comments.nodes)
(string-join
(seq-uniq
(mapcar (lambda (cmt)
(concat "@" (alist-get 'login (alist-get 'author cmt))))
.comments.nodes))
", ")))
(hint
(let-alist (car .comments.nodes)
(concat (pr-review--propertize-username .author.login)
": " .body)))
(action (lambda (_)
(push-mark)
(pr-review--goto-section-with-value .id)
(recenter))))
(forward-line)
(if pr-review-always-use-blocks
;; Render block.
(progn
(insert (propertize
(concat "> REVIEW THREAD"
(when .isResolved " - RESOLVED")
"\n")
'face 'pr-review-in-diff-thread-begin-face
'pr-review-eldoc-content hint))
(insert (propertize
(format "%s\n" text)
'face 'pr-review-in-diff-thread-body-face))
(insert-button "Go to thread ↑"
'face '(pr-review-button-face
pr-review-in-diff-thread-body-face)
'action action)
(insert (propertize
"\n"
'face 'pr-review-in-diff-thread-body-face))
(insert (propertize
" \n"
'face 'pr-review-in-diff-thread-end-face)))
;; Render single line.
(insert (propertize
(concat "> "
text
(when .isResolved " - RESOLVED")
" ")
'face 'pr-review-in-diff-thread-title-face
'pr-review-eldoc-content hint))
(insert-button "Go to thread"
'face '(pr-review-button-face
pr-review-in-diff-thread-title-face)
'action action)
(insert (propertize
"\n"
'face 'pr-review-in-diff-thread-title-face)))))))))

(defun pr-review--insert-in-diff-checkrun-annotation (annotation)
"Insert CheckRun ANNOTATION inside the diff section."
Expand Down Expand Up @@ -363,7 +419,7 @@ It will be inserted at the beginning."
.path (when .line (if .startLine
(format ":%s-%s" .startLine .line)
(format ":%s" .line))))
'face 'magit-section-secondary-heading)
'face 'pr-review-thread-item-title-face)
(when (eq t .isResolved)
(concat " - " (propertize "RESOLVED" 'face 'pr-review-info-state-face)))
(when (eq t .isOutdated)
Expand All @@ -372,21 +428,33 @@ It will be inserted at the beginning."
(make-string pr-review-section-indent-width ?\s)
(propertize " \n" 'face 'pr-review-thread-diff-begin-face))
(let ((diffhunk-lines (split-string (alist-get 'diffHunk top-comment) "\n"))
(action (lambda (_)
(push-mark)
(let-alist review-thread
(pr-review--goto-diff-line .path .diffSide .line)
(recenter))))
beg end)
(setq beg (point))
(while (> (length diffhunk-lines) 4) ;; diffHunk may be very long, only keep last 4 lines
;; diffHunk may be very long, only keep last N lines
(while (> (length diffhunk-lines) pr-review-diff-hunk-limit)
(setq diffhunk-lines (cdr diffhunk-lines)))
(pr-review--insert-fontified (string-join diffhunk-lines "\n") 'diff-mode
pr-review-section-indent-width)
pr-review-section-indent-width
'pr-review-thread-diff-body-face)
(setq end (point))
(make-button beg end
'face nil
'help-echo "Click to go to the line in diff."
'action (lambda (_)
(push-mark)
(let-alist review-thread
(pr-review--goto-diff-line .path .diffSide .line)
(recenter)))))
(if pr-review-always-use-buttons
;; Render separate button.
(progn
(insert-button "Go to diff ↓"
'face '(pr-review-button-face
pr-review-thread-diff-body-face)
'action action)
(insert (propertize "\n" 'face 'pr-review-thread-diff-body-face)))
;; The whole diff is a button.
(make-button beg end
'face nil
'help-echo "Click to go to the line in diff."
'action action)))
(insert (propertize " \n" 'face 'pr-review-thread-diff-end-face))
(mapc (lambda (cmt)
(let-alist cmt
Expand All @@ -399,7 +467,8 @@ It will be inserted at the beginning."
(pr-review--propertize-username .author.login)
" - "
(pr-review--format-timestamp .createdAt))
(pr-review--insert-html .bodyHTML (* 2 pr-review-section-indent-width))
(pr-review--insert-html .bodyHTML (* 2 pr-review-section-indent-width)
'pr-review-thread-comment-face)
(insert "\n"))))
(let-alist review-thread .comments.nodes))

Expand Down Expand Up @@ -634,7 +703,7 @@ It will be inserted at the beginning."
(mapc (lambda (required-context)
(unless (member required-context valid-context-or-names)
(insert required-item-bullet-point
(propertize required-context 'face 'pr-review-author-face)
(propertize required-context 'face 'pr-review-check-face)
": "
(propertize "EXPECTED" 'face 'pr-review-error-state-face)
"\n")))
Expand All @@ -646,7 +715,7 @@ It will be inserted at the beginning."
(insert (concat (if (member .name required-contexts)
required-item-bullet-point
"- ")
(propertize .name 'face 'pr-review-author-face)
(propertize .name 'face 'pr-review-check-face)
": "
(pr-review--propertize-keyword .status)
(when .conclusion
Expand All @@ -659,7 +728,7 @@ It will be inserted at the beginning."
(insert (concat (if (member .context required-contexts)
required-item-bullet-point
"- ")
(propertize .context 'face 'pr-review-author-face)
(propertize .context 'face 'pr-review-check-face)
": "
(pr-review--propertize-keyword .state)
(when .description
Expand Down
2 changes: 1 addition & 1 deletion pr-review.el
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -241,7 +241,7 @@ It's used as the default value of `pr-review'."
(when default-pr-path
(apply #'format " (default: %s/%s/%s)"
default-pr-path))
":"))))
": "))))
(if (string-empty-p input-url)
(or default-url "")
input-url))
Expand Down