From c04cee3cbc4235641902a7b7677fb8e5b919dbdb Mon Sep 17 00:00:00 2001 From: Wanderson Ferreira Date: Fri, 1 Oct 2021 20:45:51 -0300 Subject: [PATCH 01/16] first working version --- github-review.el | 76 ++++++++++++++++++++++++++++++++++++++++++------ 1 file changed, 67 insertions(+), 9 deletions(-) diff --git a/github-review.el b/github-review.el index aecf221..09809fb 100644 --- a/github-review.el +++ b/github-review.el @@ -116,7 +116,8 @@ return a deferred object" nodes { author { login } bodyText } } reviews(first: 50) { - nodes { author { login } bodyText state } + nodes { author { login } bodyText state + comments(first: 50) { nodes { bodyText diffHunk originalPosition } }} } } } }" .repo .owner .num))) @@ -342,9 +343,59 @@ This function infers the PR name based on the current filename" (let-alist review (format "Reviewed by @%s[%s]: %s" .author.login .state .bodyText))) +(setq github-review-comment-pos ()) + +(defun github-review-place-review-comments (gitdiff review) + (if (not (a-get-in review (list 'comments 'nodes))) + gitdiff + (let* ((at (a-get-in review (list 'author 'login))) + (body (a-get review 'bodyText)) + (body-lines (split-string body "\n")) + (state (a-get review 'state)) + + (diffs (split-string gitdiff "\n")) + (comments (a-get-in review (list 'comments 'nodes))) + (default-shift-pos 5) + (diffs-new + (-reduce-from + (lambda (acc-diff comment) + (let* ((original-pos (a-get comment 'originalPosition)) + (adjusted-pos (+ original-pos + default-shift-pos + (or (a-get github-review-comment-pos original-pos) 0))) + (comment-lines (split-string (a-get comment 'bodyText) "\n")) + (diff-splitted (-split-at adjusted-pos acc-diff))) + + (push (cons original-pos (+ (or (a-get github-review-comment-pos original-pos) 0) + (length comment-lines) + (length body-lines))) + github-review-comment-pos) + (-concat + (-first-item diff-splitted) + (list "" + (format "~ Reviewed by @%s[%s]: %s" at state + (if (string-empty-p body) + (-first-item comment-lines) + (-first-item body-lines)))) + (-map + (lambda (commentLine) (s-concat "~ " (s-trim-left commentLine))) + (-concat + (-drop 1 body-lines) + (if (string-empty-p body) + (-drop 1 comment-lines) + comment-lines))) + (-second-item diff-splitted)))) + diffs + comments))) + (s-join + "\n" + diffs-new)))) + (defun github-review-format-diff (gitdiff pr) "Formats a GITDIFF and PR to save it for review." + (message "AQUI sim") (let-alist pr + (setq github-review-comment-pos ()) (concat (github-review-to-comments .title) "\n~" @@ -359,14 +410,20 @@ This function infers the PR name based on the current filename" #'github-review-to-comments (-map #'github-review-format-top-level-comment .comments.nodes))) "\n")) - (when-let ((reviews (-reject (lambda (x) (string= (a-get x 'body) "")) .reviews.nodes))) - (concat (s-join - "\n" - (-map - #'github-review-to-comments - (-map #'github-review-format-review reviews))) - "\n")) - (a-get gitdiff 'message)))) + (-reduce-from + (lambda (acc-gitdiff node) + (github-review-place-review-comments acc-gitdiff node)) + (a-get gitdiff 'message) + .reviews.nodes) + ;; (when-let ((reviews (-reject (lambda (x) (string= (a-get x 'body) "")) .reviews.nodes))) + ;; (concat (s-join + ;; "\n" + ;; (-map + ;; #'github-review-to-comments + ;; (-map #'github-review-format-review reviews))) + ;; "\n")) + ;; (a-get gitdiff 'message) + ))) ;;;;;;;;;;;;;;;;;;;;; ;; User facing API ;; @@ -382,6 +439,7 @@ This function infers the PR name based on the current filename" (deferred:nextc it (lambda (x) (let-alist (-second-item x) + (github-review-save-diff (a-assoc pr-alist 'sha .data.repository.pullRequest.headRef.target.oid) (github-review-format-diff (-first-item x) .data.repository.pullRequest))))) From 500b44af5edc1d8df3012b89494a53c8a100cf58 Mon Sep 17 00:00:00 2001 From: Wanderson Ferreira Date: Fri, 1 Oct 2021 21:04:40 -0300 Subject: [PATCH 02/16] fixed bug in positioning --- github-review.el | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/github-review.el b/github-review.el index 09809fb..43469c6 100644 --- a/github-review.el +++ b/github-review.el @@ -343,7 +343,7 @@ This function infers the PR name based on the current filename" (let-alist review (format "Reviewed by @%s[%s]: %s" .author.login .state .bodyText))) -(setq github-review-comment-pos ()) +(setq github-review-comment-pos nil) (defun github-review-place-review-comments (gitdiff review) (if (not (a-get-in review (list 'comments 'nodes))) @@ -362,14 +362,13 @@ This function infers the PR name based on the current filename" (let* ((original-pos (a-get comment 'originalPosition)) (adjusted-pos (+ original-pos default-shift-pos - (or (a-get github-review-comment-pos original-pos) 0))) + (or github-review-comment-pos 0))) (comment-lines (split-string (a-get comment 'bodyText) "\n")) (diff-splitted (-split-at adjusted-pos acc-diff))) - (push (cons original-pos (+ (or (a-get github-review-comment-pos original-pos) 0) - (length comment-lines) - (length body-lines))) - github-review-comment-pos) + (setq github-review-comment-pos (+ (or github-review-comment-pos 0) + (length comment-lines) + (length body-lines))) (-concat (-first-item diff-splitted) (list "" @@ -393,9 +392,8 @@ This function infers the PR name based on the current filename" (defun github-review-format-diff (gitdiff pr) "Formats a GITDIFF and PR to save it for review." - (message "AQUI sim") (let-alist pr - (setq github-review-comment-pos ()) + (setq github-review-comment-pos nil) (concat (github-review-to-comments .title) "\n~" From 2c1e03051182d493cd5019c0be087077a8b2e022 Mon Sep 17 00:00:00 2001 From: Wanderson Ferreira Date: Fri, 1 Oct 2021 21:10:38 -0300 Subject: [PATCH 03/16] include general comments --- github-review.el | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/github-review.el b/github-review.el index 43469c6..8e72516 100644 --- a/github-review.el +++ b/github-review.el @@ -341,7 +341,9 @@ This function infers the PR name based on the current filename" (defun github-review-format-review (review) "Format a REVIEW object to string." (let-alist review - (format "Reviewed by @%s[%s]: %s" .author.login .state .bodyText))) + (if (not (string-empty-p .bodyText)) + (format "Reviewed by @%s[%s]: %s" .author.login .state .bodyText) + ""))) (setq github-review-comment-pos nil) @@ -408,6 +410,15 @@ This function infers the PR name based on the current filename" #'github-review-to-comments (-map #'github-review-format-top-level-comment .comments.nodes))) "\n")) + "\n" + (when-let ((reviews (-reject (lambda (x) (string= (a-get x 'body) "")) .reviews.nodes))) + (concat (s-join + "\n" + (-map + #'github-review-to-comments + (-map #'github-review-format-review reviews))) + "\n")) + "\n" (-reduce-from (lambda (acc-gitdiff node) (github-review-place-review-comments acc-gitdiff node)) From 7aa87df60d0f31d68e6d43ff5e6953e0547ce01c Mon Sep 17 00:00:00 2001 From: Wanderson Ferreira Date: Fri, 1 Oct 2021 21:19:47 -0300 Subject: [PATCH 04/16] improve clarity --- github-review.el | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/github-review.el b/github-review.el index 8e72516..bcff4db 100644 --- a/github-review.el +++ b/github-review.el @@ -331,12 +331,12 @@ This function infers the PR name based on the current filename" (defun github-review-to-comments (text) "Convert TEXT, a string to a string where each line is prefixed by ~." - (s-join "\n" (-map (lambda (x) (concat "~ " x)) (s-split "\n" text)))) + (s-join "\n" (-concat (list " ") (-map (lambda (x) (concat "~ " x)) (s-split "\n" text))))) (defun github-review-format-top-level-comment (com) "Format a top level COM objectto string." (let-alist com - (format "@%s: %s" .author.login .bodyText))) + (format "Commented by @%s: %s" .author.login .bodyText))) (defun github-review-format-review (review) "Format a REVIEW object to string." From b062d7d23dc2ce7f845e60d286e2557d63eee22a Mon Sep 17 00:00:00 2001 From: Wanderson Ferreira Date: Fri, 1 Oct 2021 21:20:41 -0300 Subject: [PATCH 05/16] remove comments --- github-review.el | 11 +---------- 1 file changed, 1 insertion(+), 10 deletions(-) diff --git a/github-review.el b/github-review.el index bcff4db..b95d0be 100644 --- a/github-review.el +++ b/github-review.el @@ -423,16 +423,7 @@ This function infers the PR name based on the current filename" (lambda (acc-gitdiff node) (github-review-place-review-comments acc-gitdiff node)) (a-get gitdiff 'message) - .reviews.nodes) - ;; (when-let ((reviews (-reject (lambda (x) (string= (a-get x 'body) "")) .reviews.nodes))) - ;; (concat (s-join - ;; "\n" - ;; (-map - ;; #'github-review-to-comments - ;; (-map #'github-review-format-review reviews))) - ;; "\n")) - ;; (a-get gitdiff 'message) - ))) + .reviews.nodes)))) ;;;;;;;;;;;;;;;;;;;;; ;; User facing API ;; From 4ee9ed32f82a862561237002fe827974d0340010 Mon Sep 17 00:00:00 2001 From: Wanderson Ferreira Date: Sat, 2 Oct 2021 08:34:22 -0300 Subject: [PATCH 06/16] remove unused diffHunk from graphql query --- github-review.el | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/github-review.el b/github-review.el index b95d0be..417f110 100644 --- a/github-review.el +++ b/github-review.el @@ -117,7 +117,8 @@ return a deferred object" } reviews(first: 50) { nodes { author { login } bodyText state - comments(first: 50) { nodes { bodyText diffHunk originalPosition } }} + comments(first: 50) + { nodes { bodyText originalPosition } }} } } } }" .repo .owner .num))) From e9a32e33165347c2b54e4036cf2d06e6dafce6d4 Mon Sep 17 00:00:00 2001 From: Wanderson Ferreira Date: Sat, 2 Oct 2021 08:34:37 -0300 Subject: [PATCH 07/16] guard feature behind flag --- github-review.el | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/github-review.el b/github-review.el index 417f110..2deb170 100644 --- a/github-review.el +++ b/github-review.el @@ -60,6 +60,10 @@ :group 'github-review :type 'string) +(defcustom github-review-view-comments-in-code-lines nil + "Flag to enable displaying comments in code lines." + :group 'github-review) + (defconst github-review-diffheader '(("Accept" . "application/vnd.github.v3.diff")) "Header for requesting diffs from GitHub.") @@ -419,12 +423,13 @@ This function infers the PR name based on the current filename" #'github-review-to-comments (-map #'github-review-format-review reviews))) "\n")) - "\n" - (-reduce-from - (lambda (acc-gitdiff node) - (github-review-place-review-comments acc-gitdiff node)) - (a-get gitdiff 'message) - .reviews.nodes)))) + (if github-review-view-comments-in-code-lines + (-reduce-from + (lambda (acc-gitdiff node) + (github-review-place-review-comments acc-gitdiff node)) + (a-get gitdiff 'message) + .reviews.nodes) + (a-get gitdiff 'message))))) ;;;;;;;;;;;;;;;;;;;;; ;; User facing API ;; From 7ed15f0a0adf9bd95a4f1d8b5197374d4afb3dac Mon Sep 17 00:00:00 2001 From: Wanderson Ferreira Date: Sat, 2 Oct 2021 08:35:37 -0300 Subject: [PATCH 08/16] unecessary newline --- github-review.el | 1 - 1 file changed, 1 deletion(-) diff --git a/github-review.el b/github-review.el index 2deb170..53d1cae 100644 --- a/github-review.el +++ b/github-review.el @@ -415,7 +415,6 @@ This function infers the PR name based on the current filename" #'github-review-to-comments (-map #'github-review-format-top-level-comment .comments.nodes))) "\n")) - "\n" (when-let ((reviews (-reject (lambda (x) (string= (a-get x 'body) "")) .reviews.nodes))) (concat (s-join "\n" From e4cc0147df63075ed4264447879443b9bef0d761 Mon Sep 17 00:00:00 2001 From: Wanderson Ferreira Date: Sat, 2 Oct 2021 08:35:53 -0300 Subject: [PATCH 09/16] remove unecessary new line --- github-review.el | 1 - 1 file changed, 1 deletion(-) diff --git a/github-review.el b/github-review.el index 53d1cae..d4bd32a 100644 --- a/github-review.el +++ b/github-review.el @@ -444,7 +444,6 @@ This function infers the PR name based on the current filename" (deferred:nextc it (lambda (x) (let-alist (-second-item x) - (github-review-save-diff (a-assoc pr-alist 'sha .data.repository.pullRequest.headRef.target.oid) (github-review-format-diff (-first-item x) .data.repository.pullRequest))))) From e227389a765f43d4ca47c6efb8a2675070217fc9 Mon Sep 17 00:00:00 2001 From: Wanderson Ferreira Date: Sat, 2 Oct 2021 08:38:31 -0300 Subject: [PATCH 10/16] docs to var --- github-review.el | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/github-review.el b/github-review.el index d4bd32a..05b5cba 100644 --- a/github-review.el +++ b/github-review.el @@ -350,7 +350,10 @@ This function infers the PR name based on the current filename" (format "Reviewed by @%s[%s]: %s" .author.login .state .bodyText) ""))) -(setq github-review-comment-pos nil) +(defvar github-review-comment-pos nil + "Variable to count how many comments in code lines were added in the diff. +This is necessary to adjust the new comments to the correct position in the diff given that +Github API provides only the originalPosition in the query.") (defun github-review-place-review-comments (gitdiff review) (if (not (a-get-in review (list 'comments 'nodes))) From bf94ed005a9fed8142eff336d7cc1f24df0a44c3 Mon Sep 17 00:00:00 2001 From: Wanderson Ferreira Date: Sat, 2 Oct 2021 13:33:05 -0300 Subject: [PATCH 11/16] guard new query behind flag --- github-review.el | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/github-review.el b/github-review.el index 05b5cba..c751a8e 100644 --- a/github-review.el +++ b/github-review.el @@ -111,6 +111,20 @@ return a deferred object" (defun github-review-get-pr-info (pr-alist callback) (let-alist pr-alist (let ((query (format "query { + repository(name: \"%s\", owner: \"%s\") { + pullRequest(number: %s){ + headRef { target{ oid } } + title + bodyText + comments(first:50) { + nodes { author { login } bodyText } + } + reviews(first: 50) { + nodes { author { login } bodyText state } + } } + } +}" .repo .owner .num)) + (query-with-comments (format "query { repository(name: \"%s\", owner: \"%s\") { pullRequest(number: %s){ headRef { target{ oid } } @@ -126,7 +140,9 @@ return a deferred object" } } } }" .repo .owner .num))) - (ghub-graphql query + (ghub-graphql (if github-review-view-comments-in-code-lines + query-with-comments + query) '() :auth 'github-review :host (github-review-api-host pr-alist) From 74c0ec1674b8e6e8979b7be0e4d22872b1cf2583 Mon Sep 17 00:00:00 2001 From: Wanderson Ferreira Date: Sat, 2 Oct 2021 14:05:11 -0300 Subject: [PATCH 12/16] readme instructions --- README.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/README.md b/README.md index ccd9ac9..4ba3279 100644 --- a/README.md +++ b/README.md @@ -119,6 +119,10 @@ machine api.github.com login yourlogin^github-review password MYTOKENGOESHERE If you use GitHub Enterprise, you can use the `github-review-host` custom variable to configure the endpoint of your GitHub Enterprise installation, this should look like `api.git.mycompany.com`. +- By default, `github-review` fetches only top level comments in a pull request. + You can set `github-review-view-comments-in-code-lines` to `t` to also fetch + comments made between code lines. + ## 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 From 395359e7e865b5de06c62154e97f1216fcc5d198 Mon Sep 17 00:00:00 2001 From: Wanderson Ferreira Date: Sat, 2 Oct 2021 19:39:39 -0300 Subject: [PATCH 13/16] move setq to guarded branch --- github-review.el | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/github-review.el b/github-review.el index c751a8e..3a237d9 100644 --- a/github-review.el +++ b/github-review.el @@ -419,7 +419,6 @@ Github API provides only the originalPosition in the query.") (defun github-review-format-diff (gitdiff pr) "Formats a GITDIFF and PR to save it for review." (let-alist pr - (setq github-review-comment-pos nil) (concat (github-review-to-comments .title) "\n~" @@ -442,11 +441,13 @@ Github API provides only the originalPosition in the query.") (-map #'github-review-format-review reviews))) "\n")) (if github-review-view-comments-in-code-lines - (-reduce-from - (lambda (acc-gitdiff node) - (github-review-place-review-comments acc-gitdiff node)) - (a-get gitdiff 'message) - .reviews.nodes) + (progn + (setq github-review-comment-pos nil) + (-reduce-from + (lambda (acc-gitdiff node) + (github-review-place-review-comments acc-gitdiff node)) + (a-get gitdiff 'message) + .reviews.nodes)) (a-get gitdiff 'message))))) ;;;;;;;;;;;;;;;;;;;;; From 6c2d092be9d0884a002bfe022a3a85797d6cb474 Mon Sep 17 00:00:00 2001 From: Wanderson Ferreira Date: Sat, 2 Oct 2021 22:10:20 -0300 Subject: [PATCH 14/16] test case and cleanup to not break previous tests --- github-review.el | 11 ++++---- test/github-review-test.el | 57 +++++++++++++++++++++++++++++++++++++- 2 files changed, 62 insertions(+), 6 deletions(-) diff --git a/github-review.el b/github-review.el index 3a237d9..54cc5f0 100644 --- a/github-review.el +++ b/github-review.el @@ -352,12 +352,12 @@ This function infers the PR name based on the current filename" (defun github-review-to-comments (text) "Convert TEXT, a string to a string where each line is prefixed by ~." - (s-join "\n" (-concat (list " ") (-map (lambda (x) (concat "~ " x)) (s-split "\n" text))))) + (s-join "\n" (-map (lambda (x) (concat "~ " x)) (s-split "\n" text)))) (defun github-review-format-top-level-comment (com) "Format a top level COM objectto string." (let-alist com - (format "Commented by @%s: %s" .author.login .bodyText))) + (format "@%s: %s" .author.login .bodyText))) (defun github-review-format-review (review) "Format a REVIEW object to string." @@ -394,11 +394,12 @@ Github API provides only the originalPosition in the query.") (setq github-review-comment-pos (+ (or github-review-comment-pos 0) (length comment-lines) - (length body-lines))) + (if (string-empty-p body) + 0 + (length body-lines)))) (-concat (-first-item diff-splitted) - (list "" - (format "~ Reviewed by @%s[%s]: %s" at state + (list (format "~ Reviewed by @%s[%s]: %s" at state (if (string-empty-p body) (-first-item comment-lines) (-first-item body-lines)))) diff --git a/test/github-review-test.el b/test/github-review-test.el index 457cf7c..6b41ea0 100644 --- a/test/github-review-test.el +++ b/test/github-review-test.el @@ -125,6 +125,37 @@ index 9eced0230..4512bb335 100644 +type MultiBalanceReport = PeriodicReport AccountName MixedAmount +type MultiBalanceReportRow = PeriodicReportRow AccountName MixedAmount + -- type alias just to remind us which AccountNames might be depth-clipped, below. + type ClippedAccountName = AccountName +") + + (defconst example-diff-before-comments-in-code-line "diff --git a/hledger-lib/Hledger/Reports/MultiBalanceReport.hs b/hledger-lib/Hledger/Reports/MultiBalanceReport.hs +index 9eced0230..4512bb335 100644 +--- a/hledger-lib/Hledger/Reports/MultiBalanceReport.hs ++++ b/hledger-lib/Hledger/Reports/MultiBalanceReport.hs + +-type MultiBalanceReport = PeriodicReport AccountLeaf MixedAmount +-type MultiBalanceReportRow = PeriodicReportRow AccountLeaf MixedAmount ++type MultiBalanceReport = PeriodicReport AccountName MixedAmount ++type MultiBalanceReportRow = PeriodicReportRow AccountName MixedAmount + + -- type alias just to remind us which AccountNames might be depth-clipped, below. + type ClippedAccountName = AccountName +") + + (defconst example-diff-after-comments-in-code-line "diff --git a/hledger-lib/Hledger/Reports/MultiBalanceReport.hs b/hledger-lib/Hledger/Reports/MultiBalanceReport.hs +index 9eced0230..4512bb335 100644 +--- a/hledger-lib/Hledger/Reports/MultiBalanceReport.hs ++++ b/hledger-lib/Hledger/Reports/MultiBalanceReport.hs + +-type MultiBalanceReport = PeriodicReport AccountLeaf MixedAmount +-type MultiBalanceReportRow = PeriodicReportRow AccountLeaf MixedAmount +~ Reviewed by @babar[COMMENTED]: Very interesting change +~ we should move forward ++type MultiBalanceReport = PeriodicReport AccountName MixedAmount ++type MultiBalanceReportRow = PeriodicReportRow AccountName MixedAmount +~ Reviewed by @babar[COMMENTED]: Change this code + -- type alias just to remind us which AccountNames might be depth-clipped, below. type ClippedAccountName = AccountName ") @@ -294,11 +325,35 @@ index 58baa4b..eae7707 100644 'bodyText "LGTM" 'state "APPROVED"))))) + (defconst review-with-comments + (a-alist 'author (a-alist 'login "babar") + 'state "COMMENTED" + 'bodyText "" + 'comments (a-alist + 'nodes + (list (a-alist 'bodyText "Very interesting change\nwe should move forward" + 'originalPosition 2) + (a-alist 'bodyText "Change this code" + 'originalPosition 4) + )))) + (describe "github-review-format-diff" (it "can format a simple diff" (expect (a-equal (github-review-format-diff simple-diff simple-pr)simple-context-expected-review))) (it "can format a diff with top level comments and review" - (expect (a-equal (github-review-format-diff simple-diff pr-with-tl-comments) expected-review-tl-comment))))) + (expect (a-equal (github-review-format-diff simple-diff pr-with-tl-comments) expected-review-tl-comment)))) + + (describe "github-review-place-review-comments" + (before-all + (setq github-review-comment-pos nil) + (setq github-review-view-comments-in-code-lines 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 + example-diff-after-comments-in-code-line)) + (it "`github-review-comment-pos' should have increased to 3 because we have 2 comments with 3 lines" + (expect github-review-comment-pos :to-equal 3)))) + (describe "entrypoints" (describe "github-review-start" :var (github-review-save-diff From 99d71fd153d00805e420a2455ca719b92b1f4dba Mon Sep 17 00:00:00 2001 From: Wanderson Ferreira Date: Sat, 2 Oct 2021 22:12:37 -0300 Subject: [PATCH 15/16] fix alignment --- test/github-review-test.el | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/test/github-review-test.el b/test/github-review-test.el index 6b41ea0..fd0a451 100644 --- a/test/github-review-test.el +++ b/test/github-review-test.el @@ -333,9 +333,8 @@ index 58baa4b..eae7707 100644 'nodes (list (a-alist 'bodyText "Very interesting change\nwe should move forward" 'originalPosition 2) - (a-alist 'bodyText "Change this code" - 'originalPosition 4) - )))) + (a-alist 'bodyText "Change this code" + 'originalPosition 4))))) (describe "github-review-format-diff" (it "can format a simple diff" From c9d53827992b24ed864519b80cdeecd86751e889 Mon Sep 17 00:00:00 2001 From: Wanderson Ferreira Date: Sun, 3 Oct 2021 00:43:11 -0300 Subject: [PATCH 16/16] wip --- github-review.el | 59 +++++++++++++++++++++++++++++++++++++++++++----- 1 file changed, 53 insertions(+), 6 deletions(-) diff --git a/github-review.el b/github-review.el index 54cc5f0..e8c6379 100644 --- a/github-review.el +++ b/github-review.el @@ -136,7 +136,7 @@ return a deferred object" reviews(first: 50) { nodes { author { login } bodyText state comments(first: 50) - { nodes { bodyText originalPosition } }} + { nodes { bodyText originalPosition databaseId } }} } } } }" .repo .owner .num))) @@ -163,7 +163,7 @@ PR-ALIST is an alist representing a PR REVIEW is the review alist CALLBACK will be called back when done" (let-alist pr-alist - (ghub-post (format "/repos/%s/%s/pulls/%s/reviews" .owner .repo .num) + (ghub-post (format "/repos/%s/%s/pulls/%s/review" .owner .repo .num) nil :auth 'github-review :payload review @@ -171,6 +171,24 @@ 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* ((position (a-get comment 'position)) + (comment-id (a-get github-review-pos->databaseid position)) + (body (a-get comment 'body))) + (ghub-post (format "/repos/%s/%s/pulls/%s/comments/%s/replie" .owner .repo .num comment-id) + nil + :payload (a-alist 'body body) + :headers github-review-diffheader + :auth 'github-review + :host "api.github.com" + :callback (lambda (&rest _)) + :errorback #'github-review-errback))) + replies))) + ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; ;; Code review file parsing ;; ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; @@ -245,7 +263,8 @@ ACC is an alist accumulating parsing state." (in-file? (not top-level?))) (cond ;; Previous comments are ignored and don't affect the parsing - ((github-review-previous-comment? l) acc) + ((github-review-previous-comment? l) + (a-assoc acc 'mark-previous-comment t)) ;; First cgithub-review-hunk ((and top-level? (github-review-hunk? l)) @@ -270,14 +289,17 @@ 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? .mark-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) + 'mark-previous-comment nil)) (t acc))))) @@ -286,6 +308,7 @@ ACC is an alist accumulating parsing state." (let* ((acc (a-alist 'path nil 'pos nil 'body "" + 'mark-previous-comment nil 'comments ())) (parsed-data (-reduce-from #'github-review-parse-line acc lines)) (parsed-comments (a-get parsed-data 'comments)) @@ -343,8 +366,26 @@ 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 (-map + (lambda (c) + (a-dissoc c 'reply?)) + (-filter + (lambda (c) (not (a-get c 'reply?))) + comments))) + (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))) + (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 _) @@ -366,6 +407,8 @@ This function infers the PR name based on the current filename" (format "Reviewed by @%s[%s]: %s" .author.login .state .bodyText) ""))) +(defvar github-review-pos->databaseid ()) + (defvar github-review-comment-pos nil "Variable to count how many comments in code lines were added in the diff. This is necessary to adjust the new comments to the correct position in the diff given that @@ -392,6 +435,9 @@ Github API provides only the originalPosition in the query.") (comment-lines (split-string (a-get comment 'bodyText) "\n")) (diff-splitted (-split-at adjusted-pos acc-diff))) + (push (cons original-pos (a-get comment 'databaseId)) + github-review-pos->databaseid) + (setq github-review-comment-pos (+ (or github-review-comment-pos 0) (length comment-lines) (if (string-empty-p body) @@ -444,6 +490,7 @@ Github API provides only the originalPosition in the query.") (if github-review-view-comments-in-code-lines (progn (setq github-review-comment-pos nil) + (setq github-review-pos->databaseid ()) (-reduce-from (lambda (acc-gitdiff node) (github-review-place-review-comments acc-gitdiff node))