Support more rendering customizations #11
Conversation
|
I'm not sure if the names for |
|
For reference, here is a snippet from my config: ;; pr-review
(pr-review-title-face ((t (:weight bold :foreground "DarkSeaGreen2" :height 200))))
;;
(pr-review-thread-item-title-face
((t (:weight bold :foreground "#e3c87d" :overline "gray50"))))
(pr-review-thread-diff-begin-face
((t (:foreground "#d4c9ae" :background "#1d2b47" :weight bold :extend t :italic t :height 0.9))))
(pr-review-thread-diff-body-face
((t (:background "#0c131f" :extend t))))
(pr-review-thread-diff-end-face
((t (:overline "#1d2b47" :background unspecified :extend t :height 0.9))))
(pr-review-thread-comment-face ((t (:foreground "#d8d8d8"))))
;;
(pr-review-in-diff-thread-begin-face
((t (:foreground "#d4c9ae" :background "#233352" :weight bold :extend t :italic t :height 0.9))))
(pr-review-in-diff-thread-body-face
((t (:foreground "#d2d2d2" :background "#0f1826" :extend t))))
(pr-review-in-diff-thread-end-face
((t (:overline "#233352" :extend t :height 0.9))))
;;
(pr-review-in-diff-pending-begin-face
((t (:foreground "#d4c9ae" :background "#233352" :weight bold :extend t :italic t :height 0.9))))
(pr-review-in-diff-pending-body-face
((t (:foreground "#d2d2d2" :background "#0f1826" :extend t))))
(pr-review-in-diff-pending-end-face
((t (:overline "#233352" :extend t :height 0.9))))
;;
(pr-review-timestamp-face ((t (:foreground "#b8a2a9" :italic t))))
(pr-review-author-face ((t (:foreground "#edddb2"))))
(pr-review-check-face ((t (:foreground "#ced9d7"))))
(pr-review-button-face ((t (:foreground "#b7c6c7" :underline t :italic t))))
(pr-review-link-face ((t (:foreground "#93a9ab" :underline t))))
(pr-review-branch-face ((t (:foreground "#ffbc40"))))
(pr-review-hash-face ((t (:foreground "#d6b274"))))
(pr-review-label-face ((t (:box (:line-width (-1 . -1) :style released-button)))))
;;
(pr-review-state-face ((t (:foreground "#b8e0e0" :bold t))))
(pr-review-info-state-face ((t (:foreground "#8c9a9c" :italic t))))
(pr-review-error-state-face ((t (:foreground "tomato" :bold t :italic t))))
(pr-review-success-state-face ((t (:foreground "#8ab046" :bold t :italic t)))) |
|
Thanks for your PR!
I'm open to these kind of appearance changes. However, personally I don't really like too many appearance related user options, since it would make maintainence hard. I think defaults' changes are OK though, since the style of this package is IMO already very opinionated. So, before introducing these alternative styles as options, I'd like to explore if there's a way to satisfy your requirement that can be used as default directly.
Does a custom face satisfy your requirement? With a custom face, you may set its color or even height, which can make them "more distinguishable". Or, do you think two lines of texts are sufficient? (maybe move the "go to thread" link to first line?)
Do you think the "go to diff" link can be moved to the same line as "reply to thread" and "resolve"? If so, then maybe it's OK to make it default visible since no extra space will be taken. For the clickable diff, maybe we can bind it in a keymap so that users can easily edit the keymap?
These changes all LGTM.
LGTM
LGTM |
| (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 |
There was a problem hiding this comment.
it seems that add-face-text-property does exactly what this does
Hi, thanks a lot for this package!
This PR adds a few more granular look and feel customizations. Default look is not affected, but it becomes possible to achieve different appearance.
Appearance
New options:
pr-review-always-use-blocks- If enabled, in-diff links to review threads are rendered as a block instead of a single line.This makes them more distinguishable, especially when you're scrolling big diffs. In this mode these blocks will have "Go to thread ↑" button.
pr-review-always-use-buttons- If enabled, diff blocks in review threads are not clickable, instead there is separate button for that.I often mis-click on a diff block when trying to select some text in it. In this mode these blocks will have "Go to diff ↓" button.
New faces:
pr-review-thread-diff-body-face- Face for content betweenpr-review-thread-diff-begin-faceandpr-review-thread-diff-end-face.pr-review-in-diff-pending-body-face- Face for content betweenpr-review-in-diff-pending-begin-faceandpr-review-in-diff-pending-end-facepr-review-in-diff-thread-begin-face,pr-review-in-diff-thread-body-face,pr-review-in-diff-thread-end-face- Faces for in-diff review thread link block whenpr-review-always-use-blocksis enabled.pr-review-thread-comment-face- Face for review comments text. Previously there were no face.pr-review-check-face- Separate face for CI checks (by default same aspr-review-author-face). Previouslypr-review-author-facewas used for both authors and checks.Changed faces:
pr-review-thread-item-title-facenow inheritsmagit-section-secondary-headinginstead ofboldNotable changes in code:
pr-review--insert-htmlandpr-review--insert-fontifiednow have additional argumentextra-face. When non-nil, this face is added to the inserted text.Both functions use
pr-review--decorate-regionhelper for that.pr-review--insert-in-diff-review-thread-linkis updated to support two modes, whenpr-review-always-use-blocksis true or false.pr-review--insert-review-thread-sectionis updated to support two modes, whenpr-review-always-use-buttonsis true or false.Diff hunks
New options:
pr-review-diff-font-lock-syntax- Allows to change diff fontification fromhunk-alsoto something else.In big diffs, syntax highlighting for diff hunks doesn't play well for me - there are too much colors and it becomes hard to read.
pr-review-diff-hunk-limit- Allows to overwrite default hunk size.Screenshots
Here are some screenshots from my fork + my theme: