emacs: CL correction.
When merging I9bf0061eaeb9f157fe1a9015488733fa7adf5d30, the latest
patch was not merged in.
BUG=chromium:1101373
TEST=Evaluated, initialized values, and used new interface.
Change-Id: I99f69e7737ea71a8fcaff20c96bab61d9c972b53
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/dev-util/+/2329416
Tested-by: Aaron Massey <aaronmassey@chromium.org>
Reviewed-by: Jack Rosenthal <jrosenth@chromium.org>
Commit-Queue: Aaron Massey <aaronmassey@chromium.org>
diff --git a/contrib/emacs/gerrit-section.el b/contrib/emacs/gerrit-section.el
index e881689..3fecb52 100644
--- a/contrib/emacs/gerrit-section.el
+++ b/contrib/emacs/gerrit-section.el
@@ -8,10 +8,6 @@
(require 'magit-section)
(require 'repo-gerrit)
-(defface gerrit-patch
- `((t :inherit magit-section-heading))
- "patch")
-
(defface gerrit-filepath
`((t :inherit magit-branch-remote))
@@ -39,19 +35,20 @@
(evil-set-initial-state 'gerrit-section-mode 'emacs)))
-
(defun gerrit-refresh ()
"Refreshes Gerrit data from hosts."
(interactive)
(setf gerrit--refreshed t)
(gerrit-init)
- (when (get-buffer gerrit-buffer-name)
- (save-excursion
- (switch-to-buffer gerrit-buffer-name)
- (setf inhibit-read-only t)
- (erase-buffer)
- (setf inhibit-read-only t)))
- (gerrit-comments t))
+ (if (get-buffer gerrit-buffer-name)
+ (save-excursion
+ (let ((inhibit-read-only t))
+ (unless (equal (current-buffer)
+ (get-buffer gerrit-buffer-name))
+ (switch-to-buffer-other-window gerrit-buffer-name))
+ (erase-buffer)
+ (gerrit-comments t)))
+ (gerrit-comments)))
(defun gerrit-comments (&optional refresh)
@@ -88,24 +85,26 @@
(goto-char (point-min))
(gerrit-section-mode)
- (setf word-wrap t)
- (setf truncate-lines nil)))
- (if refresh
- (switch-to-buffer gerrit-buffer-name)
- (pop-to-buffer gerrit-buffer-name)))
+ (if refresh
+ (switch-to-buffer gerrit-buffer-name)
+ (pop-to-buffer gerrit-buffer-name))
+
+ (setf word-wrap t)
+ (setf truncate-lines nil))))
(define-button-type 'gerrit--filepath
'face 'gerrit-filepath)
+
(defun gerrit--navigate-to-comment (project-branch-pair
line
filepath-from-project-root
section-symbol)
"Navigates the user to a comment."
- ;; Keep section open for user's to refer back to during edits.
+ ;; Open a closed section for a user to refer back to after click.
(when (oref section-symbol hidden)
(magit-section-toggle section-symbol))
@@ -122,24 +121,27 @@
(defun gerrit--insert-comment-header (change
filepath-from-project-root
line
+ author
section-symbol)
- "Inserts comments for the given change and system filepath."
+ "Inserts section header for a comment in the summary buffer."
(let (header-text
button-p
(begin-pos (point)))
(cond ((equal "/PATCHSET_LEVEL" filepath-from-project-root)
(setf header-text
(propertize
- "Patch Comment"
+ (format
+ "Patch Comment - %s"
+ author)
'face 'gerrit-filepath)))
((equal "/COMMIT_MSG" filepath-from-project-root)
;; TODO future CL for navigating
;; to commit message lines.
(setf header-text
(propertize
- (format "Commit Message:%s\n"
- filepath-from-project-root
- line)
+ (format "Commit Message:%s - %s\n"
+ line
+ author)
'face 'gerrit-filepath)))
((equal "MERGE_LIST" filepath-from-project-root)
(message "There are merge list comments which we don't support"))
@@ -147,9 +149,10 @@
(t (progn
(setf header-text
(propertize
- (format "%s:%s\n"
+ (format "%s:%s - %s\n"
filepath-from-project-root
- line)
+ line
+ author)
'face 'gerrit-filepath))
(setf button-p t))))
@@ -169,6 +172,7 @@
(defun gerrit--insert-section-comments (change
filepath-from-project-root)
+ "Inserts the comments for a given change and filepath."
;; We want the smaller lines first.
(sort (gethash filepath-from-project-root
(gethash change gerrit--change-to-filepath-comments))
@@ -181,31 +185,46 @@
(gethash filepath-from-project-root
(gethash change gerrit--change-to-filepath-comments))
do
- ;; We don't care about our own comments.
- (unless (equal
- test-user ;; FIXME when test code is removed
- (gethash "email" (gethash "author" comment-info)))
- (let ((section-symbol (gensym))
- (line (gethash "line" comment-info))
- (begin-pos (point)))
+ (let ((section-symbol (gensym))
+ (line (gethash "line" comment-info))
+ (begin-pos (point)))
- (magit-insert-section
- ;; We use section symbols to toggle when navigating.
- section-symbol
- (gerrit-section-type nil t)
+ (magit-insert-section
+ ;; We use section symbols to toggle when navigating.
+ section-symbol
+ (gerrit-section-type nil t)
- (gerrit--insert-comment-header
- change
- filepath-from-project-root
- line
- section-symbol)
- (gerrit--section-insert-comment comment-info))))))
+ (gerrit--insert-comment-header
+ change
+ filepath-from-project-root
+ line
+ (gethash "name" (gethash "author" comment-info))
+ section-symbol)
+ (gerrit--section-insert-comment change filepath-from-project-root comment-info)))))
-(defun gerrit--section-insert-comment (comment-info)
- "Inserts the author, email, and body of a comment."
+(defun gerrit--section-insert-comment (change filepath-from-project-root comment-info)
+ "Inserts the author, link to, and body of a comment."
(magit-insert-section-body
- (insert (format "Author: %s\nEmail: %s\nComment: %s\n"
- (gethash "name" (gethash "author" comment-info))
- (gethash "email" (gethash "author" comment-info))
- (gethash "message" comment-info)))))
+ (insert
+ (format "Author: %s\nLink: %s\nComment: %s\n"
+ (format
+ "%s - %s"
+ (gethash "name" (gethash "author" comment-info))
+ (gethash "email" (gethash "author" comment-info)))
+
+ ;; TODO buttonize link.
+ (propertize
+ (format
+ "https://chromium-review.googlesource.com/c/%s/+/%s/%s/%s#%s"
+ (url-hexify-string (gethash "project" change))
+ (gethash "_number" change)
+ ;; Default is nil if comments only on a single patch.
+ (or (gethash "patch_set" comment-info) "1")
+ (url-hexify-string filepath-from-project-root)
+ ;; Here we default to going to the top of the file.
+ ;; if there is no line with a comment.
+ (or (gethash "line" comment-info) "1"))
+ 'face 'link)
+
+ (gethash "message" comment-info)))))
diff --git a/contrib/emacs/gerrit.el b/contrib/emacs/gerrit.el
index 596b984..6a6ab75 100644
--- a/contrib/emacs/gerrit.el
+++ b/contrib/emacs/gerrit.el
@@ -15,7 +15,7 @@
;; TODO Make our parser self-discoverable by project instead of a parameter.
(setq test-manifest-parser (expand-file-name "src/platform/dev/contrib/emacs/manifest_parser"
test-repo-root))
-(defun gerrit--test-init-all ()
+(defun gerrit-init ()
(gerrit--init-global-comment-map test-host test-user)
(gerrit--init-global-repo-project-path-map test-manifest-parser test-repo-manifest-path))
@@ -56,30 +56,22 @@
(json-parse-string (replace-regexp-in-string "^[[:space:]]*)]}'" "" (buffer-string))))
-(defun gerrit--fetch-unresolved-comments (host change)
- "Gets recent unresolved comments for open Gerrit CLs.
+(defun gerrit--fetch-comments (host change)
+ "Gets recent comments for open Gerrit CLs.
Returns a map of the form path => sequence of comments,
where path is the filepath from the gerrit project root
and each comment represents a CommentInfo entity from Gerrit"
- (let* ((response
- (request
- (format "https://%s/changes/%s~master~%s/comments"
- host
- (url-hexify-string (gethash "project" change))
- (gethash "change_id" change))
- :sync t
- :parser 'gerrit--request-response-json-parser))
- (out-map (request-response-data response)))
- ;; We only want the user to see unresolved comments.
- (loop for key in (hash-table-keys out-map) do
- ;; We explicitly check if not true because the value may be ':false'
- ;; which is technically evals to true as it is not nil.
- (delete-if (lambda (comment) (not (eql t (gethash "unresolved" comment))))
- (gethash key out-map)))
- out-map))
+ (request-response-data
+ (request
+ (format "https://%s/changes/%s~master~%s/comments"
+ host
+ (url-hexify-string (gethash "project" change))
+ (gethash "change_id" change))
+ :sync t
+ :parser 'gerrit--request-response-json-parser)))
-(defun gerrit--fetch-change-to-file-to-unresolved-comments (host user)
+(defun gerrit--fetch-change-to-file-to-comments (host user)
"Returns a map of maps of the form:
change => filepath => array(CommentInfo Map),
where filepath is from the nearest git root for a file.
@@ -87,14 +79,14 @@
(let ((out-map (make-hash-table :test 'equal)))
(loop for change across (gerrit--fetch-recent-changes host user) do
(setf (gethash change out-map)
- (gerrit--fetch-unresolved-comments host change)))
+ (gerrit--fetch-comments host change)))
out-map))
(defun gerrit--init-global-comment-map (host user)
"Inits `gerrit--change-to-filepath-comments`."
(setf gerrit--change-to-filepath-comments
- (gerrit--fetch-change-to-file-to-unresolved-comments
+ (gerrit--fetch-change-to-file-to-comments
host user)))
