From cfde555bfde177f70fe11504138e55c143817f96 Mon Sep 17 00:00:00 2001 From: Wanderson Ferreira Date: Sun, 3 Oct 2021 13:52:26 -0300 Subject: [PATCH] respond to inline comments --- README.md | 6 +++ github-review.el | 90 ++++++++++++++++++++++++++++++++++---- test/github-review-test.el | 27 ++++++++---- 3 files changed, 106 insertions(+), 17 deletions(-) diff --git a/README.md b/README.md index 26446ab..2ce2872 100644 --- a/README.md +++ b/README.md @@ -127,6 +127,12 @@ configure the endpoint of your GitHub Enterprise installation, this should look `github-review-view-comments-in-code-lines-outdated` to `t`, however we cannot guarantee correct placement of these comments in the review buffer. + You can also enable replies to inline comments by setting + `github-review-reply-inline-comments` to `t`, this feature only works if + `github-review-view-comments-in-code-lines` is also set to `t`. This way, if + you include a comment right after a previous received comment in the diff + buffer, your new comment will be sent as a reply. + ## Notice *I am providing code in the repository to you under an open source license. Because this is my personal repository, the license you receive to my diff --git a/github-review.el b/github-review.el index 5a9c9e3..60a2475 100644 --- a/github-review.el +++ b/github-review.el @@ -68,6 +68,11 @@ "Flag to enable displaying outdated comments in code lines." :group 'github-review) +(defcustom github-review-reply-inline-comments nil + "Flag to enable replies to inline comments. + +This flag will only be valid if `github-review-view-comments-in-code-lines' is set to `t`.") + (defconst github-review-diffheader '(("Accept" . "application/vnd.github.v3.diff")) "Header for requesting diffs from GitHub.") @@ -140,7 +145,7 @@ return a deferred object" reviews(first: 50) { nodes { author { login } bodyText state comments(first: 50) - { nodes { bodyText originalPosition position outdated path} }} + { nodes { bodyText originalPosition position outdated path databaseId} }} } } } }" .repo .owner .num))) @@ -175,6 +180,31 @@ CALLBACK will be called back when done" :errorback #'github-review-errback :callback callback))) +(defun github-review-post-review-replies (pr-alist replies callback) + "Submit replies to review comments inline." + (let-alist pr-alist + (-map + (lambda (comment) + (let* ((path (a-get comment 'path)) + (position (a-get comment 'position)) + (comment-id (alist-get (s-concat path + ":" + (number-to-string position)) + github-review-pos->databaseid + nil nil 'equal)) + (body (a-get comment 'body))) + (ghub-post (format "/repos/%s/%s/pulls/%s/comments/%s/replies" + .owner .repo .num comment-id) + nil + :payload (a-alist 'body body) + :headers github-review-diffheader + :auth 'github-review + :host (github-review-api-host pr-alist) + :callback (lambda (&rest _)) + :errorback #'github-review-errback))) + replies) + (funcall callback))) + ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; ;; Code review file parsing ;; ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; @@ -195,7 +225,7 @@ CALLBACK will be called back when done" (defun github-review-previous-comment? (l) "Return t if L, a string, is a comment from previous review." - (string-prefix-p "~ " l)) + (string-prefix-p "~" l)) (defun github-review-is-start-of-file-hunk? (l) "Return t if L, a string that start with 'diff' marking the start of a file hunk." @@ -248,8 +278,11 @@ ACC is an alist accumulating parsing state." (let* ((top-level? (equal nil .pos)) (in-file? (not top-level?))) (cond - ;; Previous comments are ignored and don't affect the parsing - ((github-review-previous-comment? l) acc) + ;; Previous comments are marked if not top level and will be used to distinguish replies from new code comments + ((github-review-previous-comment? l) + (if .pos + (a-assoc acc 'previous-comment? t) + acc)) ;; First cgithub-review-hunk ((and top-level? (github-review-hunk? l)) @@ -274,14 +307,18 @@ ACC is an alist accumulating parsing state." ;; For such comments we report it on on the first line (a-alist 'position (max .pos 1) 'path .path - 'body (github-review-comment-text l)) + 'body (github-review-comment-text l) + 'reply? .previous-comment?) .comments))) ;; Header before the filenames, restart the position ((github-review-is-start-of-file-hunk? l) (a-assoc acc 'pos nil)) ;; Any other line in a file - (in-file? (a-assoc acc 'pos (+ 1 .pos))) + (in-file? + (a-assoc acc + 'pos (+ 1 .pos) + 'previous-comment? nil)) (t acc))))) @@ -290,7 +327,8 @@ ACC is an alist accumulating parsing state." (let* ((acc (a-alist 'path nil 'pos nil 'body "" - 'comments ())) + 'comments () + 'previous-comment? nil)) (parsed-data (-reduce-from #'github-review-parse-line acc lines)) (parsed-comments (a-get parsed-data 'comments)) (parsed-body (s-trim-right (a-get parsed-data 'body))) @@ -347,8 +385,31 @@ This function infers the PR name based on the current filename" (message "Submitting review, this may take a while ...") (let* ((pr-alist (github-review-pr-from-fname (buffer-file-name))) (parsed-review (github-review-parsed-review-from-current-buffer)) + (comments (a-get parsed-review 'comments)) + (regular-comments (->> comments + (-filter + (lambda (c) + (not (a-get c 'reply?)))) + (-map + (lambda (c) + (a-dissoc c 'reply?))))) + (reply-comments (-filter + (lambda (c) + (a-get c 'reply?)) + comments)) (head-sha (a-get pr-alist 'sha)) - (review (a-assoc parsed-review 'commit_id head-sha 'event kind))) + (review (a-assoc parsed-review + 'commit_id head-sha + 'event kind + 'comments regular-comments))) + + (when github-review-reply-inline-comments + (github-review-post-review-replies + pr-alist + reply-comments + (lambda (&rest _) + (message "Done submitting review replies")))) + (github-review-post-review pr-alist review (lambda (&rest _) @@ -378,6 +439,9 @@ Github API provides only the originalPosition in the query.") (defun github-review--get-how-many-comments-written (path) (or (a-get github-review-comment-pos path) 0)) +(defvar github-review-pos->databaseid () + "Hold databaseID to each path and comment combination") + (defun github-review-place-review-comments (gitdiff review) (if (not (a-get-in review (list 'comments 'nodes))) gitdiff @@ -416,6 +480,15 @@ Github API provides only the originalPosition in the query.") 0 (length body-lines)))) + ;; save databaseID to each path and comment combination + (setf (alist-get (s-concat + path ":" + (number-to-string + (or position original-pos))) + github-review-pos->databaseid + nil nil 'equal) + (a-get comment 'databaseId)) + ;; include comments on buffer for this path (let* ((result (-concat @@ -470,6 +543,7 @@ Github API provides only the originalPosition in the query.") (if github-review-view-comments-in-code-lines (progn (setq github-review-comment-pos ()) + (setq github-review-pos->databaseid ()) (-reduce-from (lambda (acc-gitdiff node) (github-review-place-review-comments acc-gitdiff node)) diff --git a/test/github-review-test.el b/test/github-review-test.el index 78ed55f..21f99a6 100644 --- a/test/github-review-test.el +++ b/test/github-review-test.el @@ -84,7 +84,8 @@ index 8ad537d..0000000 (comments ((position . 5) (body . "Comment test") - (path . "bar"))))) + (path . "bar") + (reply? . nil))))) (defconst example-review-deleted-comment-haskell "# comment test ~ remove bad @@ -165,10 +166,12 @@ index 9eced0230..4512bb335 100644 (comments ((position . 5) (body . "Comment test") - (path . "bar")) + (path . "bar") + (reply? . nil)) ((position . 6) (body . "here too") - (path . "hledger-lib/Hledger/Reports/MultiBalanceReport.hs"))))) + (path . "hledger-lib/Hledger/Reports/MultiBalanceReport.hs") + (reply? . nil))))) (defconst examplediff "# This is a global comment at the top of the file # with multiple @@ -243,23 +246,28 @@ index 58baa4b..eae7707 100644 (comments ((position . 1) (body . "comment on zeroth line\ncomment on first line") - (path . "content/reference/google-closure-library.adoc")) + (path . "content/reference/google-closure-library.adoc") + (reply? . nil)) ((position . 5) (body . "And a comment inline about\na specific line\n```with some\ncode```") - (path . "content/reference/google-closure-library.adoc")) + (path . "content/reference/google-closure-library.adoc") + (reply? . nil)) ((position . 6) (body . "Some other comment inline") - (path . "content/reference/google-closure-library.adoc"))))) + (path . "content/reference/google-closure-library.adoc") + (reply? . nil))))) (defconst complex-review-expected-no-comment-on-zeroth-and-first-line '((body . "This is a global comment at the top of the file\nwith multiple\nlines") (comments ((position . 5) (body . "And a comment inline about\na specific line\n```with some\ncode```") - (path . "content/reference/google-closure-library.adoc")) + (path . "content/reference/google-closure-library.adoc") + (reply? . t)) ((position . 6) (body . "Some other comment inline") - (path . "content/reference/google-closure-library.adoc"))))) + (path . "content/reference/google-closure-library.adoc") + (reply? . nil))))) (describe "github-review-parse-review-lines" @@ -352,7 +360,8 @@ index 58baa4b..eae7707 100644 (before-all (setq github-review-comment-pos ()) (setq github-review-view-comments-in-code-lines nil) - (setq github-review-view-comments-in-code-lines-outdated nil)) + (setq github-review-view-comments-in-code-lines-outdated nil) + (setq github-review-reply-inline-comments nil)) (it "can include PR comments made in code lines" (expect (github-review-place-review-comments example-diff-before-comments-in-code-line review-with-comments) :to-equal