Merge pull request #94 from pwntester/online_reviews

Online reviews

README.md

@@ -51,12 +51,11 @@ There is only an `Octo <object> <action> [arguments]` command:
 | | close | |
 | | checkout | |
 | | commits | |
-| | files | |
+| | changes | |
 | | diff | |
 | | merge | [commit\|rebase\|squash] [delete] |
 | | ready| |
 | | checks | |
-| | reviews | |
 | | reload | |
 | | browser | |
 | gist | list | [repo] [key=value]*<br>[Available keys](https://cli.github.com/manual/gh_gist_list):  `repo`\|`public`\|`secret`<br>Mappings:<br>`<CR>`: Append Gist to buffer<br>`<C-b>`: Opens Gist in web browser |
@@ -75,6 +74,10 @@ There is only an `Octo <object> <action> [arguments]` command:
 | card | add | |
 | | delete | |
 | | move | |
+| review| start| Start a new review |
+| | comments| View in-progress review comments |
+| | submit| Submit the review |
+| | threads | View other people review threads (comment+replies)|
 
 * If repo is not provided, it will be derived from `<cwd>/.git/config`.
 
@@ -98,10 +101,10 @@ Octo issue list neovim/neovim labels=bug,help\ wanted states=OPEN
 Just edit the issue title, description or comments as a regular buffer and use `:w(rite)` to sync the issue with GitHub.
 
 ## PR review
-- Change to the directory containing the repo/PR
 - Open the PR (eg: `Octo pr list` or `Octo pr edit XXX`)
-- If not already in the PR branch, checkout the PR with `Octo pr checkout`
 - Start a review with `Octo review start`
+- Quickfix will be populated with the changed files 
+- Change quickfix entries with `]q` and `[q` or by selecting an entry in the quickfix window
 - Add comments with `<space>ca` or `:OctoAddReviewComment` on single or multiple lines
 - Add suggestions with `<space>sa` or `:OctoAddReviewSuggestion` on single or multiple lines
 - A new split will open. Enter the comment and save it (`:w`). Optionally close the split
@@ -122,11 +125,9 @@ Just edit the issue title, description or comments as a regular buffer and use `
 
 ![](https://camo.githubusercontent.com/97aaf7efe7c8ff45cbc4359f28339fd9f9dd7ba3609fbd14b0649a979af15431/68747470733a2f2f692e696d6775722e636f6d2f71495a5a6b48342e706e67) 
 
-- Change to the directory containing the repo/PR
 - Open the PR (eg: `Octo pr list` or `Octo pr edit XXX`)
-- If not already in the PR branch, checkout the PR with `Octo pr checkout`
-- Open review threads view with `Octo pr reviews`
-- Quickfix will be populated with changed files 
+- Open review threads view with `Octo review view`
+- Quickfix will be populated with the changed files 
 - Change quickfix entries with `]q` and `[q` or by selecting an entry in the quickfix window
 - Jump between comments with `]c` and `[c`
 - You can reply to a comment, delete them, add/remove reactions, etc. as if you where in an Octo issue buffer

after/syntax/octo_issue.vim

@@ -15,7 +15,7 @@ hi def link OctoNvimCommentHeading PreProc
 hi def link OctoNvimCommentUser String 
 hi def link OctoNvimIssueOpen MoreMsg
 hi def link OctoNvimIssueClosed ErrorMsg
-hi def link OctoNvimIssueMerged DiffAdd
+hi def link OctoNvimIssueMerged Keyword
 hi def link OctoNvimIssueId Question
 hi def link OctoNvimIssueTitle PreProc
 hi def link OctoNvimEmpty Comment

lua/octo/commands.lua

@@ -65,7 +65,7 @@ local commands = {
     commits = function()
       menu.commits()
     end,
-    files = function()
+    changes = function()
       menu.changed_files()
     end,
     diff = function()
@@ -80,9 +80,6 @@ local commands = {
     ready = function()
       M.pr_ready_for_review()
     end,
-    reviews = function()
-      M.pr_reviews()
-    end,
     search = function(repo, ...)
       local opts = M.process_varargs(repo, ...)
       menu.pull_request_search(opts)
@@ -96,14 +93,17 @@ local commands = {
   },
   review = {
     start = function()
-      M.review_pr()
+      M.start_review()
     end,
     comments = function()
       menu.review_comments()
     end,
     submit = function()
       M.submit_review()
     end,
+    threads = function()
+      M.review_threads()
+    end,
   },
   gist = {
     list = function(...)
@@ -645,31 +645,21 @@ function M.show_pr_diff()
   )
 end
 
-function M.pr_reviews()
+function M.review_threads()
   local repo, number, _ = util.get_repo_number_pr()
   if not repo then
     return
   end
-
-  -- make sure CWD is in PR repo and branch
-  if not util.in_pr_branch() then
-    return
-  end
-
   local owner = vim.split(repo, "/")[1]
   local name = vim.split(repo, "/")[2]
   local query = format(graphql.review_threads_query, owner, name, number)
   gh.run(
     {
       args = {"api", "graphql", "-f", format("query=%s", query)},
-      --args = {"api", "graphql", "--paginate", "-f", format("query=%s", query)},
       cb = function(output, stderr)
         if stderr and not util.is_blank(stderr) then
           api.nvim_err_writeln(stderr)
         elseif output then
-          -- aggregate comments
-          -- local resp = util.aggregate_pages(output, "data.repository.pullRequest.reviewThreads.nodes.comments.nodes")
-          -- for now, I will just remove pagination on this query since 100 comments in a single thread looks enough for most cases
           local resp = json.parse(output)
           reviews.populate_reviewthreads_qf(repo, number, resp.data.repository.pullRequest.reviewThreads.nodes)
         end
@@ -700,27 +690,18 @@ function M.submit_review()
   api.nvim_buf_set_keymap(bufnr, "n", "<C-m>", ":lua require'octo.reviews'.submit_review('COMMENT')<CR>", mapping_opts)
   api.nvim_buf_set_keymap(bufnr, "n", "<C-r>", ":lua require'octo.reviews'.submit_review('REQUEST_CHANGES')<CR>", mapping_opts)
   vim.cmd [[normal G]]
-  vim.cmd [[startinsert]]
+  --vim.cmd [[startinsert]]
 end
 
-function M.review_pr()
+function M.start_review()
   local repo, number, pr = util.get_repo_number_pr()
   if not repo then
     return
   end
-  if not vim.fn.exists("*fugitive#repo") then
-    print("vim-fugitive required")
-    return
-  end
-  -- make sure CWD is in PR repo and branch
-  if not util.in_pr_branch() then
-    return
-  end
 
   reviews.review_comments = {}
+  reviews.review_files = {}
 
-  -- TODO: graphql
-  -- get list of changed files
   local url = format("repos/%s/pulls/%d/files", repo, number)
   gh.run(
     {
@@ -733,18 +714,18 @@ function M.review_pr()
           local changes = {}
           for _, result in ipairs(results) do
             local change = {
-              filename = result.filename,
+              path = result.filename,
               patch = result.patch,
               status = result.status,
               stats = format("+%d -%d ~%d", result.additions, result.deletions, result.changes)
             }
             table.insert(changes, change)
           end
           reviews.populate_changes_qf(changes, {
+            pull_request_repo = repo,
+            pull_request_number = number,
             pull_request_id = pr.id,
-            baseRefName = pr.baseRefName,
             baseRefSHA = pr.baseRefSHA,
-            headRefName = pr.headRefName,
             headRefSHA = pr.headRefSHA
           })
         end

lua/octo/graphql.lua

@@ -286,6 +286,9 @@ query($endCursor: String) {
               nodes{
                 id
                 body
+                commit {
+                  oid
+                }
                 author { login }
                 authorAssociation
                 originalPosition
@@ -873,16 +876,16 @@ query($endCursor: String) {
   search(query: "%s", type: USER, first: 100) {
     nodes {
       ... on User {
-        id 
+        id
         login
       }
       ... on Organization {
-        id 
+        id
         login
         teams(first:100, after: $endCursor) {
           totalCount
           nodes {
-            id 
+            id
             name
           }
           pageInfo {
@@ -896,4 +899,38 @@ query($endCursor: String) {
 }
 ]]
 
+M.changed_files_query =
+  [[
+query($endCursor: String) {
+  repository(owner: "%s", name: "%s") {
+    pullRequest(number: %d) {
+      files(first:100, after: $endCursor) {
+        nodes {
+          additions
+          deletions
+          path
+        }
+        pageInfo {
+          hasNextPage
+          endCursor
+        }
+      }
+    }
+  }
+}
+]]
+
+M.file_content_query =
+  [[
+query {
+  repository(owner: "%s", name: "%s") {
+    object(expression: "%s:%s") {
+      ... on Blob {
+        text
+      }
+    }
+  }
+}
+]]
+
 return M

lua/octo/init.lua

@@ -250,6 +250,13 @@ end
 
 function M.save_buffer()
   local bufnr = api.nvim_get_current_buf()
+
+  -- for comment buffers, dispatch it to the right module
+  if string.match(api.nvim_buf_get_name(bufnr), "octo://.+/.+/%d+/comment/.*") then
+    require"octo.reviews".save_review_comment()
+    return
+  end
+
   local ft = api.nvim_buf_get_option(bufnr, "filetype")
   local repo, number = util.get_repo_number({"octo_issue", "octo_reviewthread"})
   if not repo then
@@ -610,49 +617,4 @@ function M.apply_buffer_mappings(bufnr, kind)
   end
 end
 
-M.win_opts = {}
-
-function M.restore_win_opts()
-  -- called when leaving an octo buffer to restore the win opts used outside octo
-  local win_id = api.nvim_get_current_win()
-  if vim.tbl_contains(vim.tbl_keys(M.win_opts), win_id) then
-    vim.wo[win_id].number = M.win_opts[win_id].number
-    vim.wo[win_id].relativenumber = M.win_opts[win_id].relativenumber
-    vim.wo[win_id].cursorline = M.win_opts[win_id].cursorline
-    vim.wo[win_id].signcolumn = M.win_opts[win_id].signcolumn
-    vim.wo[win_id].foldcolumn = M.win_opts[win_id].foldcolumn
-    vim.wo[win_id].wrap = M.win_opts[win_id].wrap
-  end
-end
-
-function M.set_octo_win_opts()
-  -- called when entering an octo buffer
-  M.save_win_opts()
-
-  local win_id = api.nvim_get_current_win()
-  if vim.tbl_contains(vim.tbl_keys(M.win_opts), win_id) then
-    vim.wo[win_id].number = false
-    vim.wo[win_id].relativenumber = false
-    vim.wo[win_id].cursorline = false
-    vim.wo[win_id].signcolumn = "yes"
-    vim.wo[win_id].foldcolumn = "1"
-    vim.wo[win_id].wrap = true
-  end
-end
-
-function M.save_win_opts()
-  -- called when entering an octo buffer for the first time in a given window
-  local win_id = api.nvim_get_current_win()
-  if not vim.tbl_contains(vim.tbl_keys(M.win_opts), win_id) then
-    M.win_opts[win_id] = {
-      number = vim.wo[win_id].number,
-      relativenumber = vim.wo[win_id].relativenumber,
-      cursorline = vim.wo[win_id].cursorline,
-      signcolumn = vim.wo[win_id].signcolumn,
-      foldcolumn = vim.wo[win_id].foldcolumn,
-      wrap = vim.wo[win_id].wrap,
-    }
-  end
-end
-
 return M

lua/octo/menu.lua

@@ -368,7 +368,6 @@ function M.changed_files()
   if not repo then
     return
   end
-  -- TODO: graphql
   local url = format("repos/%s/pulls/%d/files", repo, number)
   gh.run(
     {
@@ -586,7 +585,7 @@ function M.review_comments()
       previewer = previewers.review_comment.new({}),
       attach_mappings = function()
 
-        -- TODO: delete comment
+        -- TODO: add action to delete comment
 
         actions.select_default:replace(function(prompt_bufnr)
           local comment = action_state.get_selected_entry(prompt_bufnr).comment

lua/octo/previewers.lua

@@ -172,8 +172,10 @@ M.changed_files =
       define_preview = function(self, entry)
         if self.state.bufname ~= entry.value or api.nvim_buf_line_count(self.state.bufnr) == 1 then
           local diff = entry.change.patch
-          api.nvim_buf_set_lines(self.state.bufnr, 0, -1, false, vim.split(diff, "\n"))
-          api.nvim_buf_set_option(self.state.bufnr, "filetype", "diff")
+          if diff then
+            api.nvim_buf_set_lines(self.state.bufnr, 0, -1, false, vim.split(diff, "\n"))
+            api.nvim_buf_set_option(self.state.bufnr, "filetype", "diff")
+          end
         end
       end
     }