diff --git a/pr-review-common.el b/pr-review-common.el index f5a0415..666a847 100644 --- a/pr-review-common.el +++ b/pr-review-common.el @@ -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." @@ -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) @@ -89,14 +94,43 @@ "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 @@ -104,6 +138,11 @@ "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." @@ -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 diff --git a/pr-review-render.el b/pr-review-render.el index 055455e..d4b35a3 100644 --- a/pr-review-render.el +++ b/pr-review-render.el @@ -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) @@ -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 @@ -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. @@ -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 @@ -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 + (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." @@ -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 @@ -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." @@ -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) @@ -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 @@ -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)) @@ -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"))) @@ -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 @@ -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 diff --git a/pr-review.el b/pr-review.el index 8ed0cb4..cb859ec 100644 --- a/pr-review.el +++ b/pr-review.el @@ -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))