From 4cf4f237f26d96cf8aa1467d7bb0a94f424b25e1 Mon Sep 17 00:00:00 2001 From: Andrei Date: Sat, 27 Aug 2022 17:57:17 +0300 Subject: [PATCH 1/3] RCasR initial commit to enrich GH details returned. --- github-review.el | 38 +++++++++++++++++++++++++++++++++----- 1 file changed, 33 insertions(+), 5 deletions(-) diff --git a/github-review.el b/github-review.el index 7f8286b..94367ff 100644 --- a/github-review.el +++ b/github-review.el @@ -126,6 +126,16 @@ return a deferred object" (let ((query (format "query { repository(name: \"%s\", owner: \"%s\") { pullRequest(number: %s){ + reviewThreads(first: 100) { edges { node {comments(first: 10) { + nodes { + pullRequestReview { + id + } + body + } + } + }}} + baseRef { target{ oid } } headRef { target{ oid } } title bodyText @@ -140,6 +150,17 @@ return a deferred object" (query-with-comments (format "query { repository(name: \"%s\", owner: \"%s\") { pullRequest(number: %s){ + reviewThreads(first: 100) { edges { node {comments(first: 10) { + nodes { + pullRequestReview { + id + } + body + } + } + }}} + baseRef { target{ oid } } + baseRefname headRef { target{ oid } } title bodyText @@ -149,7 +170,7 @@ return a deferred object" reviews(first: 50) { nodes { author { login } bodyText state comments(first: 50) - { nodes { bodyText originalPosition position outdated path databaseId} }} + { nodes { bodyText originalPosition position outdated path databaseId originalCommit { abbreviatedOid } replyTo { author { login } databaseId } resourcePath} }} } } } }" .repo .owner .num))) @@ -374,7 +395,7 @@ ACC is an alist accumulating parsing state." 'repo (match-string 2 url) 'owner (match-string 1 url))))) -(defun github-review-save-diff (pr-alist diff) +(defun github-review-save-diff (pr-alist diff pr-base) "Save a DIFF (string) to a temp file named after pr specified by PR-ALIST." (let-alist pr-alist (find-file (format "%s/%s___%s___%s___%s.diff" github-review-review-folder .owner .repo .num .sha)) @@ -584,12 +605,16 @@ Github API provides only the originalPosition in the query.") (setq github-review-comment-pos ()) (setq github-review-pos->databaseid ()) (-reduce-from - (lambda (acc-gitdiff node) - (github-review-place-review-comments acc-gitdiff node)) + ;;(lambda (acc-gitdiff node) + ;; (github-review-place-review-comments acc-gitdiff node)) + #'acc-diff-node (a-get gitdiff 'message) .reviews.nodes)) (a-get gitdiff 'message))))) +(defun acc-diff-node (acc-gitdiff node) + (github-review-place-review-comments acc-gitdiff node)) + ;;;;;;;;;;;;;;;;;;;;; ;; User facing API ;; ;;;;;;;;;;;;;;;;;;;;; @@ -601,12 +626,15 @@ Github API provides only the originalPosition in the query.") (deferred:parallel (lambda () (github-review-get-diff-deferred pr-alist)) (lambda () (github-review-get-pr-info-deferred pr-alist))) + ;;(deferred:sync! (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))))) + (github-review-format-diff (-first-item x) .data.repository.pullRequest) + (a-assoc pr-alist 'sha .data.repository.pullRequest.baseRef.target.oid))))) + ;;) (deferred:error it (lambda (err) (message "Got an error from the GitHub API %S!" err))))) From bce7b0e79acd07b17dc6340e6d4e155563e304da Mon Sep 17 00:00:00 2001 From: Andrei Date: Tue, 6 Sep 2022 00:00:06 +0300 Subject: [PATCH 2/3] Intermediate commit. todo: create review-comment revs on the review branch. --- github-review.el | 65 ++++++++++++++++++++++++++++++++++++++++++------ 1 file changed, 57 insertions(+), 8 deletions(-) diff --git a/github-review.el b/github-review.el index 94367ff..b1524b3 100644 --- a/github-review.el +++ b/github-review.el @@ -160,15 +160,15 @@ return a deferred object" } }}} baseRef { target{ oid } } - baseRefname + baseRefName headRef { target{ oid } } title bodyText comments(first:50) { - nodes { author { login } bodyText } + nodes { author { login } bodyText} } reviews(first: 50) { - nodes { author { login } bodyText state + nodes { author { login } bodyText state databaseId commit { abbreviatedOid } comments(first: 50) { nodes { bodyText originalPosition position outdated path databaseId originalCommit { abbreviatedOid } replyTo { author { login } databaseId } resourcePath} }} } } @@ -401,11 +401,24 @@ ACC is an alist accumulating parsing state." (find-file (format "%s/%s___%s___%s___%s.diff" github-review-review-folder .owner .repo .num .sha)) (erase-buffer) (insert diff) + (let-alist pr-base + (insert + (concat "\n\n" + ";;; Local Variables:\n" ";;; gh-base-id: " + .sha.baseRef.target.oid "\n")) + ;; fetch .sha.baseRefName branch + ;; check it out into a WT + ;; for each commit in RCs + ;; create the-branch + ;; find and sort RCs(commit).comments into C[] + ;; to apply C[i] one by one + ) (save-buffer) (github-review-mode) ;; Use `C-c C-c' in diff-mode to go to source code (setq default-directory forge-current-dir) - (goto-char (point-min)))) + (goto-char (point-min)) + )) (defun github-review-parsed-review-from-current-buffer () "Return a code review given the current buffer containing a diff." @@ -576,6 +589,8 @@ Github API provides only the originalPosition in the query.") gitdiff comments)))) +(defvar my-review-buf nil "the review code branch magit status buffer") + (defun github-review-format-diff (gitdiff pr) "Formats a GITDIFF and PR to save it for review." (let-alist pr @@ -602,19 +617,53 @@ Github API provides only the originalPosition in the query.") "\n")) (if github-review-view-comments-in-code-lines (progn + ;; (magit-fetch-branch REMOTE BRANCH ARGS) + ;; (magit-worktree-branch PATH BRANCH START-POINT &optional FORCE) + (magit-fetch-branch "origin" "+lc--mvp-review-commit:lc--mvp-review-commit__review" '()) + (save-excursion + (setq my-review-buf + (magit-worktree-checkout + "/media/local_17/src/emacs/git/WTs/GHR_A" + "lc--mvp-review-commit__review"))) + (setq github-review-comment-pos ()) (setq github-review-pos->databaseid ()) (-reduce-from - ;;(lambda (acc-gitdiff node) - ;; (github-review-place-review-comments acc-gitdiff node)) - #'acc-diff-node + (lambda (acc-gitdiff node) + (mygit-commit-node-to-review-branch node .baseRefName) + (github-review-place-review-comments acc-gitdiff node)) (a-get gitdiff 'message) .reviews.nodes)) (a-get gitdiff 'message))))) (defun acc-diff-node (acc-gitdiff node) + (mygit-commit-node-to-review-branch node) (github-review-place-review-comments acc-gitdiff node)) +(defun mygit-commit-node-to-review-branch (review baserefname) + (unless (not (a-get-in review (list 'comments 'nodes))) + (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)) + (comments (a-get-in review (list 'comments 'nodes))) + (default-shift-pos 1) + (commit-id (a-get-in review (list 'commit 'abbreviatedOid))) + (review-id (a-get-in review (list 'databaseId)))) + ;; buffer may generally change with another review in the list + (set-buffer my-review-buf) + ;;( for each comment apply it ) + (save-excursion + (magit-show-commit commit-id) + (-map #'handle-one-comment comments) + ) + (message (concat "Review of '" (or body "") "' having github id " (number-to-string review-id) " over commit " commit-id " in the branch " baserefname)) + ))) + +(defun handle-one-comment (comment) + (message (concat "Comment:" + (let-alist comment .bodyText))) +) ;;;;;;;;;;;;;;;;;;;;; ;; User facing API ;; ;;;;;;;;;;;;;;;;;;;;; @@ -633,7 +682,7 @@ Github API provides only the originalPosition in the query.") (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) - (a-assoc pr-alist 'sha .data.repository.pullRequest.baseRef.target.oid))))) + (a-assoc pr-alist 'sha .data.repository.pullRequest))))) ;;) (deferred:error it (lambda (err) From 4d84c0715c31d2587a35bb5e8bf6b5d1006defce Mon Sep 17 00:00:00 2001 From: Andrei Date: Wed, 29 Mar 2023 22:14:59 +0300 Subject: [PATCH 3/3] more details at fetching etc. --- github-review.el | 262 +++++++++++++++++++++++++++++++++++++++-------- 1 file changed, 217 insertions(+), 45 deletions(-) diff --git a/github-review.el b/github-review.el index b1524b3..4d0931a 100644 --- a/github-review.el +++ b/github-review.el @@ -162,6 +162,10 @@ return a deferred object" baseRef { target{ oid } } baseRefName headRef { target{ oid } } + headRefName + commits(first:100) { nodes { + commit { message abbreviatedOid } + }} title bodyText comments(first:50) { @@ -170,7 +174,7 @@ return a deferred object" reviews(first: 50) { nodes { author { login } bodyText state databaseId commit { abbreviatedOid } comments(first: 50) - { nodes { bodyText originalPosition position outdated path databaseId originalCommit { abbreviatedOid } replyTo { author { login } databaseId } resourcePath} }} + { nodes { createdAt author { login } updatedAt bodyText originalPosition position outdated path databaseId originalCommit { abbreviatedOid } replyTo { author { login } databaseId } resourcePath} }} } } } }" .repo .owner .num))) @@ -399,26 +403,27 @@ ACC is an alist accumulating parsing state." "Save a DIFF (string) to a temp file named after pr specified by PR-ALIST." (let-alist pr-alist (find-file (format "%s/%s___%s___%s___%s.diff" github-review-review-folder .owner .repo .num .sha)) - (erase-buffer) - (insert diff) - (let-alist pr-base - (insert - (concat "\n\n" - ";;; Local Variables:\n" ";;; gh-base-id: " - .sha.baseRef.target.oid "\n")) - ;; fetch .sha.baseRefName branch - ;; check it out into a WT - ;; for each commit in RCs - ;; create the-branch - ;; find and sort RCs(commit).comments into C[] - ;; to apply C[i] one by one - ) + (let ((inhibit-read-only t) + (erase-buffer) + (insert diff)) + (let-alist pr-base + (insert + (concat "\n\n" + ";;; Local Variables:\n" ";;; gh-base-id: " + .sha.baseRef.target.oid "\n")) + ;; fetch .sha.baseRefName branch + ;; check it out into a WT + ;; for each commit in RCs + ;; create the-branch + ;; find and sort RCs(commit).comments into C[] + ;; to apply C[i] one by one + )) (save-buffer) (github-review-mode) ;; Use `C-c C-c' in diff-mode to go to source code - (setq default-directory forge-current-dir) + ;;(setq default-directory forge-current-dir) (goto-char (point-min)) - )) + )) (defun github-review-parsed-review-from-current-buffer () "Return a code review given the current buffer containing a diff." @@ -589,7 +594,14 @@ Github API provides only the originalPosition in the query.") gitdiff comments)))) -(defvar my-review-buf nil "the review code branch magit status buffer") +(defvar github-review-view-comments-in-code-lines-old-style nil "embed sources code comments into the total diff") +(defvar my-status-review-base nil "the review code branch magit status buffer") +(defvar my-ghr-worktree-path "/media/local_17/src/emacs/git/WTs/GHR_A") +(defclass my-ghr-comment () + ((gh-node :initarg :gh-node) + (commit-id :initarg :commit-id) + (parent :initarg :parent) ;; databaseId + (kids :initarg :kids))) (defun github-review-format-diff (gitdiff pr) "Formats a GITDIFF and PR to save it for review." @@ -616,31 +628,177 @@ Github API provides only the originalPosition in the query.") (-map #'github-review-format-review reviews))) "\n")) (if github-review-view-comments-in-code-lines - (progn - ;; (magit-fetch-branch REMOTE BRANCH ARGS) - ;; (magit-worktree-branch PATH BRANCH START-POINT &optional FORCE) - (magit-fetch-branch "origin" "+lc--mvp-review-commit:lc--mvp-review-commit__review" '()) + (let ((comment-tree '()) + (commit-list) + (review-alist '())) (save-excursion - (setq my-review-buf - (magit-worktree-checkout - "/media/local_17/src/emacs/git/WTs/GHR_A" - "lc--mvp-review-commit__review"))) + ;; (magit-fetch-branch REMOTE BRANCH ARGS) + ;; (magit-fetch-branch "origin" (concat "+" .headRefName ":" .headRefName) '()) + (magit-fetch-branch "origin" (concat "" .headRefName ":" .headRefName) '()) + (setq my-status-review-base + (let () + ;; TODO: configure WTs + ;;(magit-with-toplevel + ;;(magit-worktree-checkout my-ghr-worktree-path .headRefName) + ;;(magit-branch-create (concat .headRefName "-review") .headRefName) ;;"lc--mvp-review-commit__review" + ;;(magit-reset-hard (concat .headRefName "-review")) + ;; (magit-checkout (concat .headRefName "-review")) + ;;(magit-worktree-branch my-ghr-worktree-path (concat .headRefName "-review") .headRefName t) + + (magit-diff-visit-directory my-ghr-worktree-path nil) + ;;(magit-reset-hard (concat .headRefName "-review")) + (current-buffer) + ;; ) + ))) (setq github-review-comment-pos ()) (setq github-review-pos->databaseid ()) - (-reduce-from - (lambda (acc-gitdiff node) - (mygit-commit-node-to-review-branch node .baseRefName) - (github-review-place-review-comments acc-gitdiff node)) - (a-get gitdiff 'message) - .reviews.nodes)) + ;; TODO: sort .reviews.nodes + ;; 1. build sorted commit-id list + (setq commit-list (seq-map #'(lambda (el) (a-get-in el (list 'commit 'abbreviatedOid))) .commits.nodes)) + ;; 2. ...d + ;; (set-sort ... + (setq review-alist + (cl-loop for commit-id in commit-list collect + (list commit-id + (seq-sort #'(lambda (a b) (< (a-get-in a (list 'databaseId)) (a-get-in b (list 'databaseId)))) + (cl-loop for r-node in .reviews.nodes append + (seq-filter + #'(lambda (el) + (when + (string= + commit-id + (a-get-in el (list 'originalCommit 'abbreviatedOid))) + (let () + (nconc el (list (cons 'review-db-id (a-get r-node 'databaseId)))) + t))) + ;; cut other branches from type PullRequestReview includ databaseId + (a-get-in r-node (list 'comments 'nodes)))))))) + (cl-loop for rev in review-alist do + (let ((commit-id (car rev)) + (comment-s (car (cdr rev)))) + (save-excursion + (set-buffer my-status-review-base) + (magit-reset-hard "HEAD") ; minor cleanup from past activities on this WT + (when + (magit-call-git "diff" "--exit-code" (concat commit-id "..." .headRefName)) + ;; internal commit + (if (magit-commit-p (concat .headRefName "-review-" commit-id)) + (magit-checkout (concat .headRefName "-review-" commit-id)) + (magit-branch-or-checkout (concat .headRefName "-review-" commit-id) commit-id)) + (magit-show-commit commit-id) + ;;(-map #'(lambda (comment) (handle-one-comment comment commit-id headrefname)) comments) + (message + (concat "Review commets over commit " commit-id " in the branch " .headRefName)) + (cl-map nil #'(lambda (comment) (handle-one-comment comment commit-id .headRefName)) comment-s) + ;;(basic-save-buffer nil) + ;;(magit-refresh) + ;;(let ((buffer-save-without-query t)) + (magit-save-repository-buffers t) + (magit-stash-save (cl-concatenate 'string "Review comments for " commit-id) nil t nil t 'index))))) + + (if github-review-view-comments-in-code-lines-old-style + (-reduce-from + (lambda (acc-gitdiff node) + (github-review-place-review-comments acc-gitdiff node)) + (a-get gitdiff 'message) + .reviews.nodes) ;; could do better ... review-alist + ;; else if old + (a-get gitdiff 'message))) + ;; else if (a-get gitdiff 'message))))) +;; commit-id determs the review branch, which original-commit-id +;; is to find the review comment anchor. +(defun handle-one-comment (comment commit-id headrefname) + (let* ((path (a-get comment 'path)) + (reply-to-id (a-get-in comment (list 'replyTo 'databaseId))) + (id-comment (a-get-in comment (list 'databaseId))) + (updated-at (a-get comment 'updatedAt)) + (author (a-get-in comment (list 'author 'login))) + ;; todo/fixme: this is a wrong handling. orig-pos is for review-sub-branches + (original-pos (a-get comment 'originalPosition)) + ;;(-position (a-get comment 'position)) + ;;(position (when (numberp -position) -position)) + (position (a-get comment 'position)) + (outdated (a-get comment 'outdated)) + (adjusted-pos) + ;; todo: refine into review-sub-branches, one per original commit + (original-commit-id (a-get-in comment (list 'originalCommit 'abbreviatedOid))) + ;;(buf-name (concat path "-" original-commit-id)) + (buf-name (concat path "-" original-commit-id)) + (comment-lines (split-string (a-get comment 'bodyText) "\n")) + (review-id (a-get comment 'review-db-id))) + + ;;;(if (null outdated) + (let () + (setq adjusted-pos (+ original-pos 7)) ;; GH's + git "constant" + + (message (concat "### Comment from review " + (number-to-string review-id) + " over " original-commit-id "id = " + (number-to-string id-comment) + (if (null reply-to-id) "" (concat " as reply-to " (number-to-string reply-to-id))) + " at " updated-at ": " + (let-alist comment .bodyText) + " applies on line " (number-to-string original-pos))) + + ;; todo/fixme set-buffer of the caller still holds + ;; todo: support review per-commit sub-branches + ;; (when (not (string= original-commit-id commit-id)) + ;; ;; assert original is parent + ;; (magit-branch-or-checkout (concat headrefname "-review-" original-commit-id))) + + (when (null (get-buffer buf-name)) + (get-buffer-create buf-name) + (with-current-buffer buf-name + ;;(magit-with-toplevel + (magit-git-insert "show" original-commit-id "--oneline" "--" "github-review.el") + ;;) + (diff-mode))) + ;; TODO: build review comment graph (a forest) with the ROOT message on the top. + ;; Graph should be first complete "in memory" and inserted into the sources + ;; at the end of all reviews processing. + (with-current-buffer buf-name + (goto-line adjusted-pos) + (save-excursion + (diff-goto-source) ;; TODO: account already created hunks that shift @@ base line + (let ((beg)) + (newline) + (forward-line -1) + (insert comment-start) + (insert (concat "### Comment over " original-commit-id "id = " + (number-to-string id-comment) + " by " author + " at " updated-at + " adjusted pos = " (number-to-string adjusted-pos) ": " + (let-alist comment .bodyText))) + (setq beg (point)) + (newline) + (mapcar #'(lambda (str) (insert str) (newline)) comment-lines) + (comment-region beg (point)) + (basic-save-buffer nil))) + ) + ) + ;; todo: handle with review-sub-branches + ;; maybe (cl-assert (null position)) ? + ;;;) ; outdated + ) + ) + (defun acc-diff-node (acc-gitdiff node) (mygit-commit-node-to-review-branch node) (github-review-place-review-comments acc-gitdiff node)) -(defun mygit-commit-node-to-review-branch (review baserefname) +(defun insert-node-into-tree (tree node) + (let ((commit-id (a-get-in review (list 'commit 'abbreviatedOid)))) + (if (null tree) + (setf (car tree) (list commit-id node)) + (if ...))) + + ) + +(defun mygit-commit-node-to-review-branch (review headrefname comment-tree) (unless (not (a-get-in review (list 'comments 'nodes))) (let* ((at (a-get-in review (list 'author 'login))) (body (a-get review 'bodyText)) @@ -650,20 +808,26 @@ Github API provides only the originalPosition in the query.") (default-shift-pos 1) (commit-id (a-get-in review (list 'commit 'abbreviatedOid))) (review-id (a-get-in review (list 'databaseId)))) - ;; buffer may generally change with another review in the list - (set-buffer my-review-buf) ;;( for each comment apply it ) (save-excursion + ;; buffer may generally change with another review in the list + ;; Todo: Sub-branch. + (set-buffer my-status-review-base) + (when (magit-call-git "diff" "--exit-code" + (concat commit-id "..." headrefname)) + ;; internal commit + (magit-branch-or-checkout (concat headrefname "-review-" commit-id) commit-id)) + + ;; todo: for the review print (to where?) Author, createdAt, body-lines (magit-show-commit commit-id) - (-map #'handle-one-comment comments) - ) - (message (concat "Review of '" (or body "") "' having github id " (number-to-string review-id) " over commit " commit-id " in the branch " baserefname)) + (-map #'(lambda (comment) (handle-one-comment comment commit-id headrefname)) comments) + ) + (message + (concat "Review of '" (or body "") "' having github id " + (number-to-string review-id) " over commit " commit-id + " in the branch " headrefname)) ))) -(defun handle-one-comment (comment) - (message (concat "Comment:" - (let-alist comment .bodyText))) -) ;;;;;;;;;;;;;;;;;;;;; ;; User facing API ;; ;;;;;;;;;;;;;;;;;;;;; @@ -683,10 +847,11 @@ Github API provides only the originalPosition in the query.") (a-assoc pr-alist 'sha .data.repository.pullRequest.headRef.target.oid) (github-review-format-diff (-first-item x) .data.repository.pullRequest) (a-assoc pr-alist 'sha .data.repository.pullRequest))))) - ;;) (deferred:error it (lambda (err) - (message "Got an error from the GitHub API %S!" err))))) + (message "Got an error from the GitHub API %S!" err))) + ;;) ;; sync! + )) ;;;###autoload @@ -709,7 +874,14 @@ Github API provides only the originalPosition in the query.") (defun github-review-start (url) "Start review given PR URL." (interactive "sPR URL: ") - (github-review-start-internal (github-review-pr-from-url url))) + (let ((saved-frame (selected-frame)) + (saved-win-conf (current-window-configuration))) + (window-configuration-to-register 99) + (github-review-start-internal (github-review-pr-from-url url)) + (select-frame saved-frame) + (jump-to-register 99) + (set-window-configuration saved-win-conf)) + ) ;;;###autoload (defun github-review-approve ()