Replying to inline comments

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

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.")
 
@@ -119,8 +123,26 @@ return a deferred object"
         nodes { author { login } bodyText state }
       } }
   }
+}" .repo .owner .num))
+          (query-with-comments (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
+          comments(first: 50)
+            { nodes { bodyText originalPosition databaseId } }}
+      } }
+  }
 }" .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)
@@ -141,14 +163,32 @@ 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
                :host (github-review-api-host pr-alist)
                :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 ;;
 ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
@@ -223,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))
@@ -248,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)))))
 
@@ -264,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))
@@ -321,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 _)
@@ -340,7 +403,65 @@ 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)
+      "")))
+
+(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
+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)))
+      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 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 (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)
+                                                        0
+                                                      (length body-lines))))
+                 (-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."
@@ -366,7 +487,16 @@ This function infers the PR name based on the current filename"
                  #'github-review-to-comments
                  (-map #'github-review-format-review reviews)))
                "\n"))
-     (a-get gitdiff 'message))))
+     (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))
+            (a-get gitdiff 'message)
+            .reviews.nodes))
+       (a-get gitdiff 'message)))))
 
 ;;;;;;;;;;;;;;;;;;;;;
 ;; User facing API ;;

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,34 @@ 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