From e3d7dfe16753a06a0e2945d4daedbfd12b542a79 Mon Sep 17 00:00:00 2001 From: Harrison Cramer Date: Mon, 26 Feb 2024 08:00:01 -0500 Subject: [PATCH 01/16] trim trailing slash from custom URLs --- lua/gitlab/state.lua | 2 +- lua/gitlab/utils/init.lua | 7 +++++++ 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/lua/gitlab/state.lua b/lua/gitlab/state.lua index 0cd3f23d..feab6643 100644 --- a/lua/gitlab/state.lua +++ b/lua/gitlab/state.lua @@ -242,7 +242,7 @@ M.setPluginConfiguration = function() end M.settings.auth_token = file_properties.auth_token or os.getenv("GITLAB_TOKEN") - M.settings.gitlab_url = file_properties.gitlab_url or os.getenv("GITLAB_URL") or "https://gitlab.com" + M.settings.gitlab_url = u.trim_slash(file_properties.gitlab_url or os.getenv("GITLAB_URL") or "https://gitlab.com") if M.settings.auth_token == nil then vim.notify( diff --git a/lua/gitlab/utils/init.lua b/lua/gitlab/utils/init.lua index d181501d..63886ff7 100644 --- a/lua/gitlab/utils/init.lua +++ b/lua/gitlab/utils/init.lua @@ -753,4 +753,11 @@ M.open_in_browser = function(url) end end +---Trims the trailing slash from a URL +---@param s string +---@return string +M.trim_slash = function(s) + return (s:gsub("/+$", "")) +end + return M From 438d126883bc72bdf9301b62d95b4dd43afd6455 Mon Sep 17 00:00:00 2001 From: Harrison Cramer Date: Mon, 26 Feb 2024 08:07:02 -0500 Subject: [PATCH 02/16] Updated .github/CONTRIBUTING.md --- .github/CONTRIBUTING.md | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/.github/CONTRIBUTING.md b/.github/CONTRIBUTING.md index b8403069..eab6cb31 100644 --- a/.github/CONTRIBUTING.md +++ b/.github/CONTRIBUTING.md @@ -45,8 +45,10 @@ $ stylua . $ luacheck --globals vim busted --no-max-line-length -- . ``` -4. Make the merge request to the `main` branch of `.gitlab.nvim` +4. Make the merge request to the `develop` branch of `.gitlab.nvim` Please provide a description of the feature, and links to any relevant issues. -That's it! I'll try to respond to any incoming merge request in a few days. Once we've reviewed it and it's been merged into main, the pipeline will detect whether we're merging in a patch, minor, or major change, and create a new tag (e.g. 1.0.12) and release. +That's it! I'll try to respond to any incoming merge request in a few days. Once we've reviewed it, it will be merged into the develop branc, it will be merged into the develop branch. + +After some time, if the develop branch is found to be stable, that branch will be merged into `main` and released. When merged into `main` the pipeline will detect whether we're merging in a patch, minor, or major change, and create a new tag (e.g. 1.0.12) and release. From d29bd0b367982c1806c4c00259d34825602a6934 Mon Sep 17 00:00:00 2001 From: Harrison Cramer Date: Mon, 26 Feb 2024 08:09:59 -0500 Subject: [PATCH 03/16] Updated .github/ISSUE_TEMPLATE/bug_report.md --- .github/ISSUE_TEMPLATE/bug_report.md | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/.github/ISSUE_TEMPLATE/bug_report.md b/.github/ISSUE_TEMPLATE/bug_report.md index 04a0e136..6f173b45 100644 --- a/.github/ISSUE_TEMPLATE/bug_report.md +++ b/.github/ISSUE_TEMPLATE/bug_report.md @@ -3,15 +3,17 @@ name: Bug report about: Create a report to help improve gitlab.nvim! title: '' labels: '' -assignees: '' - --- ## Prerequsities -- [ ] The "Troubleshooting" section of the README did not help -- [ ] I've installed the required dependencies - [ ] I'm on the latest version of the plugin +- [ ] I've installed the required dependencies +- [ ] I've run `:h gitlab.nvim.troubleshooting` and followed the steps there + +## Setup Configuration and Environment + +Please post here the options you're passing to configure `gitlab.nvim` and specify any environment variables you're relying on. ## Bug Description @@ -26,5 +28,3 @@ A clear and concise description of what the bug is. ## Screenshots If applicable, add screenshots to help explain your problem. - -## Other Details From a6401d9e6820d76cfbba855d719de1441cad71d9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20F=2E=20Bortl=C3=ADk?= Date: Tue, 27 Feb 2024 14:39:07 +0100 Subject: [PATCH 04/16] Feat: Improve discussion tree toggling (#192) * Fix: Toggle modified notes (#188) * Fix: Toggle discussion nodes correctly * Feat: Show Help keymap in discussion tree winbar * Fix: Enable toggling nodes from the note body Previously, the cursor had to be placed on the first line of the note (the one with the author name) for toggling to work. * Fix: Enable toggling resolved status from child nodes Previously, the cursor had to be placed on the root node of the discussion for toggling resolved status to work. * Fix: Only try to show emoji popup on note nodes Without this fix, there was an error whenever the cursor moved to a file_name or path node in the "by_file_name" discussion tree. * Feat: Add keymap for toggling tree type Previously, tree type could only be set in the plugin configuration. Now it can be toggled within a session. The keymap "i" is chosen based on a similar functionality in DiffView. * Fix: Disable tree type toggling in Notes Tree type toggling only makes sense in the (linked) Discussions and making it possible in the Notes could confuse users. --- README.md | 1 + doc/gitlab.nvim.txt | 1 + .../actions/discussions/annotations.lua | 1 + lua/gitlab/actions/discussions/init.lua | 73 ++++++++++++++++--- lua/gitlab/actions/discussions/winbar.lua | 3 +- lua/gitlab/emoji.lua | 2 +- lua/gitlab/state.lua | 14 +++- 7 files changed, 83 insertions(+), 12 deletions(-) diff --git a/README.md b/README.md index c4ffbd40..efafc835 100644 --- a/README.md +++ b/README.md @@ -150,6 +150,7 @@ require("gitlab").setup({ resolved = '✓', -- Symbol to show next to resolved discussions unresolved = '-', -- Symbol to show next to unresolved discussions tree_type = "simple", -- Type of discussion tree - "simple" means just list of discussions, "by_file_name" means file tree with discussions under file + toggle_tree_type = "i", -- Toggle type of discussion tree - "simple", or "by_file_name" winbar = nil -- Custom function to return winbar title, should return a string. Provided with WinbarTable (defined in annotations.lua) -- If using lualine, please add "gitlab" to disabled file types, otherwise you will not see the winbar. }, diff --git a/doc/gitlab.nvim.txt b/doc/gitlab.nvim.txt index 24a9652a..65e6e10d 100644 --- a/doc/gitlab.nvim.txt +++ b/doc/gitlab.nvim.txt @@ -182,6 +182,7 @@ you call this function with no values the defaults will be used: resolved = '✓', -- Symbol to show next to resolved discussions unresolved = '-', -- Symbol to show next to unresolved discussions tree_type = "simple", -- Type of discussion tree - "simple" means just list of discussions, "by_file_name" means file tree with discussions under file + toggle_tree_type = "i", -- Toggle type of discussion tree - "simple", or "by_file_name" winbar = nil -- Custom function to return winbar title, should return a string. Provided with WinbarTable (defined in annotations.lua) -- If using lualine, please add "gitlab" to disabled file types, otherwise you will not see the winbar. }, diff --git a/lua/gitlab/actions/discussions/annotations.lua b/lua/gitlab/actions/discussions/annotations.lua index 341a57c9..72c6db7b 100644 --- a/lua/gitlab/actions/discussions/annotations.lua +++ b/lua/gitlab/actions/discussions/annotations.lua @@ -84,6 +84,7 @@ ---@field resolved_discussions number ---@field resolvable_notes number ---@field resolved_notes number +---@field help_keymap string --- ---@class SignTable ---@field name string diff --git a/lua/gitlab/actions/discussions/init.lua b/lua/gitlab/actions/discussions/init.lua index 92b3c1d0..682971aa 100644 --- a/lua/gitlab/actions/discussions/init.lua +++ b/lua/gitlab/actions/discussions/init.lua @@ -75,6 +75,21 @@ M.refresh_discussion_data = function() end) end +---Toggle Discussions tree type between "simple" and "by_file_name" +---@param unlinked boolean True if selected view type is Notes (unlinked discussions) +M.toggle_tree_type = function(unlinked) + if unlinked then + u.notify("Toggling tree type is only possible in Discussions", vim.log.levels.INFO) + return + end + if state.settings.discussion_tree.tree_type == "simple" then + state.settings.discussion_tree.tree_type = "by_file_name" + else + state.settings.discussion_tree.tree_type = "simple" + end + M.rebuild_discussion_tree() +end + ---Opens the discussion tree, sets the keybindings. It also ---creates the tree for notes (which are not linked to specific lines of code) ---@param callback function? @@ -327,7 +342,15 @@ end -- This function (settings.discussion_tree.toggle_discussion_resolved) will toggle the resolved status of the current discussion and send the change to the Go server M.toggle_discussion_resolved = function(tree) local note = tree:get_node() - if not note or not note.resolvable then + if note == nil then + return + end + + -- Switch to the root node to enable toggling from child nodes and note bodies + if not note.resolvable and M.is_node_note(note) then + note = M.get_root_node(tree, note) + end + if note == nil then return end @@ -370,6 +393,15 @@ M.toggle_node = function(tree) if node == nil then return end + + -- Switch to the "note" node from "note_body" nodes to enable toggling discussions inside comments + if node.type == "note_body" then + node = tree:get_node(node:get_parent_id()) + end + if node == nil then + return + end + local children = node:get_child_ids() if node == nil then return @@ -401,7 +433,7 @@ end ---This function (settings.discussion_tree.toggle_nodes) expands/collapses all nodes and their children according to the opts. ---@param tree NuiTree ---@param opts ToggleNodesOptions -M.toggle_nodes = function(tree, opts) +M.toggle_nodes = function(tree, unlinked, opts) local current_node = tree:get_node() if current_node == nil then return @@ -409,25 +441,41 @@ M.toggle_nodes = function(tree, opts) local root_node = M.get_root_node(tree, current_node) for _, node in ipairs(tree:get_nodes()) do if opts.toggle_resolved then - if state.resolved_expanded then + if + (unlinked and state.unlinked_discussion_tree.resolved_expanded) + or (not unlinked and state.discussion_tree.resolved_expanded) + then M.collapse_recursively(tree, node, root_node, opts.keep_current_open, true) else M.expand_recursively(tree, node, true) end end if opts.toggle_unresolved then - if state.unresolved_expanded then + if + (unlinked and state.unlinked_discussion_tree.unresolved_expanded) + or (not unlinked and state.discussion_tree.unresolved_expanded) + then M.collapse_recursively(tree, node, root_node, opts.keep_current_open, false) else M.expand_recursively(tree, node, false) end end end + -- Reset states of resolved discussions after toggling if opts.toggle_resolved then - state.resolved_expanded = not state.resolved_expanded + if unlinked then + state.unlinked_discussion_tree.resolved_expanded = not state.unlinked_discussion_tree.resolved_expanded + else + state.discussion_tree.resolved_expanded = not state.discussion_tree.resolved_expanded + end end + -- Reset states of unresolved discussions after toggling if opts.toggle_unresolved then - state.unresolved_expanded = not state.unresolved_expanded + if unlinked then + state.unlinked_discussion_tree.unresolved_expanded = not state.unlinked_discussion_tree.unresolved_expanded + else + state.discussion_tree.unresolved_expanded = not state.discussion_tree.unresolved_expanded + end end tree:render() M.restore_cursor_position(tree, current_node, root_node) @@ -543,6 +591,8 @@ M.rebuild_discussion_tree = function() M.discussion_tree = discussion_tree M.switch_can_edit_bufs(false) vim.api.nvim_set_option_value("filetype", "gitlab", { buf = M.linked_bufnr }) + state.discussion_tree.resolved_expanded = false + state.discussion_tree.unresolved_expanded = false end M.rebuild_unlinked_discussion_tree = function() @@ -561,6 +611,8 @@ M.rebuild_unlinked_discussion_tree = function() M.set_tree_keymaps(unlinked_discussion_tree, M.unlinked_bufnr, true) M.unlinked_discussion_tree = unlinked_discussion_tree M.switch_can_edit_bufs(false) + state.unlinked_discussion_tree.resolved_expanded = false + state.unlinked_discussion_tree.unresolved_expanded = false end M.switch_can_edit_bufs = function(bool) @@ -643,6 +695,9 @@ M.is_current_node_note = function(tree) end M.set_tree_keymaps = function(tree, bufnr, unlinked) + vim.keymap.set("n", state.settings.discussion_tree.toggle_tree_type, function() + M.toggle_tree_type(unlinked) + end, { buffer = bufnr, desc = "Toggle tree type between `simple` and `by_file_name`" }) vim.keymap.set("n", state.settings.discussion_tree.edit_comment, function() if M.is_current_node_note(tree) then M.edit_comment(tree, unlinked) @@ -662,21 +717,21 @@ M.set_tree_keymaps = function(tree, bufnr, unlinked) M.toggle_node(tree) end, { buffer = bufnr, desc = "Toggle node" }) vim.keymap.set("n", state.settings.discussion_tree.toggle_all_discussions, function() - M.toggle_nodes(tree, { + M.toggle_nodes(tree, unlinked, { toggle_resolved = true, toggle_unresolved = true, keep_current_open = state.settings.discussion_tree.keep_current_open, }) end, { buffer = bufnr, desc = "Toggle all nodes" }) vim.keymap.set("n", state.settings.discussion_tree.toggle_resolved_discussions, function() - M.toggle_nodes(tree, { + M.toggle_nodes(tree, unlinked, { toggle_resolved = true, toggle_unresolved = false, keep_current_open = state.settings.discussion_tree.keep_current_open, }) end, { buffer = bufnr, desc = "Toggle resolved nodes" }) vim.keymap.set("n", state.settings.discussion_tree.toggle_unresolved_discussions, function() - M.toggle_nodes(tree, { + M.toggle_nodes(tree, unlinked, { toggle_resolved = false, toggle_unresolved = true, keep_current_open = state.settings.discussion_tree.keep_current_open, diff --git a/lua/gitlab/actions/discussions/winbar.lua b/lua/gitlab/actions/discussions/winbar.lua index c7291b50..da325eb5 100644 --- a/lua/gitlab/actions/discussions/winbar.lua +++ b/lua/gitlab/actions/discussions/winbar.lua @@ -40,12 +40,13 @@ local function content(discussions, unlinked_discussions, file_name) resolved_discussions = resolved_discussions, resolvable_notes = resolvable_notes, resolved_notes = resolved_notes, + help_keymap = state.settings.help, } return state.settings.discussion_tree.winbar(t) end ----This function sends the edited comment to the Go server +---This function updates the winbar ---@param discussions Discussion[] ---@param unlinked_discussions UnlinkedDiscussion[] ---@param base_title string diff --git a/lua/gitlab/emoji.lua b/lua/gitlab/emoji.lua index 617ffabf..1d61dafd 100644 --- a/lua/gitlab/emoji.lua +++ b/lua/gitlab/emoji.lua @@ -70,7 +70,7 @@ M.init_popup = function(tree, bufnr) vim.api.nvim_create_autocmd({ "CursorHold" }, { callback = function() local node = tree:get_node() - if node == nil then + if node == nil or not require("gitlab.actions.discussions").is_node_note(node) then return end diff --git a/lua/gitlab/state.lua b/lua/gitlab/state.lua index feab6643..992e66ba 100644 --- a/lua/gitlab/state.lua +++ b/lua/gitlab/state.lua @@ -64,6 +64,7 @@ M.settings = { resolved = "✓", unresolved = "-", tree_type = "simple", + toggle_tree_type = "i", ---@param t WinbarTable winbar = function(t) local discussions_content = t.resolvable_discussions ~= 0 @@ -79,7 +80,8 @@ M.settings = { discussions_content = "%#Comment#" .. discussions_content notes_content = "%#Text#" .. notes_content end - return " " .. discussions_content .. " %#Comment#| " .. notes_content + local help = "%#Comment#%=Help: " .. t.help_keymap:gsub(" ", "") .. " " + return " " .. discussions_content .. " %#Comment#| " .. notes_content .. help end, }, merge = { @@ -169,6 +171,16 @@ M.settings = { }, } +-- These are the initial states of the discussion trees +M.discussion_tree = { + resolved_expanded = false, + unresolved_expanded = false, +} +M.unlinked_discussion_tree = { + resolved_expanded = false, + unresolved_expanded = false, +} + -- Merges user settings into the default settings, overriding them M.merge_settings = function(args) M.settings = u.merge(M.settings, args) From d985d71110532bebafd7d83bae1b0b307bad17bf Mon Sep 17 00:00:00 2001 From: "Harrison (Harry) Cramer" <32515581+harrisoncramer@users.noreply.github.com> Date: Tue, 27 Feb 2024 18:39:52 -0500 Subject: [PATCH 05/16] Fix Multi Line Issues (Large Refactor) (#197) Fix: Multi-line discussions. The calculation of a range for a multiline comment has been consolidated and moved into the location.lua file. This does not attempt to fix diagnostics. Refactor: It refactors the discussions code to split hunk parsing and management into a separate module Fix: Don't allow comments on modified buffers #194 by preventing comments on the reviewer when using --imply-local and when the working tree is dirty entirely. Refactor: It introduces a new List class for data aggregation, filtering, etc. Fix: It removes redundant API calls and refreshes from the discussion pane --- .github/CONTRIBUTING.md | 2 +- .github/workflows/go.yaml | 1 + .github/workflows/lua.yaml | 1 + .../actions/assignees_and_reviewers.lua | 9 +- lua/gitlab/actions/comment.lua | 60 +-- lua/gitlab/actions/create_mr.lua | 3 +- .../actions/discussions/annotations.lua | 19 + lua/gitlab/actions/discussions/init.lua | 115 +++-- lua/gitlab/actions/discussions/winbar.lua | 31 +- lua/gitlab/actions/help.lua | 9 +- lua/gitlab/actions/labels.lua | 17 +- lua/gitlab/actions/summary.lua | 10 +- lua/gitlab/git.lua | 11 + lua/gitlab/hunks/init.lua | 281 ++++++++++++ lua/gitlab/init.lua | 1 + lua/gitlab/reviewer/diffview.lua | 432 ------------------ lua/gitlab/reviewer/init.lua | 344 +++++++++++--- lua/gitlab/reviewer/location.lua | 206 +++++++++ lua/gitlab/server.lua | 9 +- lua/gitlab/utils/init.lua | 192 ++------ lua/gitlab/utils/list.lua | 66 +++ 21 files changed, 1046 insertions(+), 773 deletions(-) create mode 100644 lua/gitlab/git.lua create mode 100644 lua/gitlab/hunks/init.lua delete mode 100755 lua/gitlab/reviewer/diffview.lua create mode 100755 lua/gitlab/reviewer/location.lua create mode 100644 lua/gitlab/utils/list.lua diff --git a/.github/CONTRIBUTING.md b/.github/CONTRIBUTING.md index eab6cb31..0a7679fd 100644 --- a/.github/CONTRIBUTING.md +++ b/.github/CONTRIBUTING.md @@ -49,6 +49,6 @@ $ luacheck --globals vim busted --no-max-line-length -- . Please provide a description of the feature, and links to any relevant issues. -That's it! I'll try to respond to any incoming merge request in a few days. Once we've reviewed it, it will be merged into the develop branc, it will be merged into the develop branch. +That's it! I'll try to respond to any incoming merge request in a few days. Once we've reviewed it, it will be merged into the develop branch. After some time, if the develop branch is found to be stable, that branch will be merged into `main` and released. When merged into `main` the pipeline will detect whether we're merging in a patch, minor, or major change, and create a new tag (e.g. 1.0.12) and release. diff --git a/.github/workflows/go.yaml b/.github/workflows/go.yaml index e1c997d7..ce68a779 100644 --- a/.github/workflows/go.yaml +++ b/.github/workflows/go.yaml @@ -3,6 +3,7 @@ on: pull_request: branches: - main + - develop jobs: go_lint: name: Lint Go 💅 diff --git a/.github/workflows/lua.yaml b/.github/workflows/lua.yaml index 12d5b785..71598d94 100644 --- a/.github/workflows/lua.yaml +++ b/.github/workflows/lua.yaml @@ -3,6 +3,7 @@ on: pull_request: branches: - main + - develop jobs: lua_lint: name: Lint Lua 💅 diff --git a/lua/gitlab/actions/assignees_and_reviewers.lua b/lua/gitlab/actions/assignees_and_reviewers.lua index 8c99cbca..329f3825 100644 --- a/lua/gitlab/actions/assignees_and_reviewers.lua +++ b/lua/gitlab/actions/assignees_and_reviewers.lua @@ -2,6 +2,7 @@ -- and assignees in Gitlab, those who must review an MR. local u = require("gitlab.utils") local job = require("gitlab.job") +local List = require("gitlab.utils.list") local state = require("gitlab.state") local M = {} @@ -67,13 +68,11 @@ end M.filter_eligible = function(current, to_remove) local ids = u.extract(to_remove, "id") - local res = {} - for _, member in ipairs(current) do + return List.new(current):filter(function(member) if not u.contains(ids, member.id) then - table.insert(res, member) + return true end - end - return res + end) end return M diff --git a/lua/gitlab/actions/comment.lua b/lua/gitlab/actions/comment.lua index 580fb00e..e81b4639 100644 --- a/lua/gitlab/actions/comment.lua +++ b/lua/gitlab/actions/comment.lua @@ -5,9 +5,11 @@ local Popup = require("nui.popup") local state = require("gitlab.state") local job = require("gitlab.job") local u = require("gitlab.utils") +local git = require("gitlab.git") local discussions = require("gitlab.actions.discussions") local miscellaneous = require("gitlab.actions.miscellaneous") local reviewer = require("gitlab.reviewer") +local Location = require("gitlab.reviewer.location") local M = {} -- Popup creation is wrapped in a function so that it is performed *after* user @@ -20,6 +22,14 @@ end -- This function will open a comment popup in order to create a comment on the changed/updated -- line in the current MR M.create_comment = function() + local has_clean_tree = git.has_clean_tree() + if state.settings.reviewer_settings.diffview.imply_local and not has_clean_tree then + u.notify( + "Cannot leave comments on a dirty tree. \n Please stash all local changes or push them up.", + vim.log.levels.WARN + ) + return + end local comment_popup = create_comment_popup() comment_popup:mount() state.set_popup_keymaps(comment_popup, function(text) @@ -95,30 +105,11 @@ M.create_note = function() end, miscellaneous.attach_file) end ----@class LineRange ----@field start_line integer ----@field end_line integer - ----@class ReviewerLineInfo ----@field old_line integer ----@field new_line integer ----@field type string either "new" or "old" - ----@class ReviewerRangeInfo ----@field start ReviewerLineInfo ----@field end ReviewerLineInfo - ----@class ReviewerInfo ----@field file_name string ----@field old_line integer | nil ----@field new_line integer | nil ----@field range_info ReviewerRangeInfo|nil - ---This function (settings.popup.perform_action) will send the comment to the Go server ---@param text string comment text ----@param range LineRange | nil range of visuel selection or nil +---@param visual_range LineRange | nil range of visual selection or nil ---@param unlinked boolean | nil if true, the comment is not linked to a line -M.confirm_create_comment = function(text, range, unlinked) +M.confirm_create_comment = function(text, visual_range, unlinked) if text == nil then u.notify("Reviewer did not provide text of change", vim.log.levels.ERROR) return @@ -129,33 +120,42 @@ M.confirm_create_comment = function(text, range, unlinked) job.run_job("/mr/comment", "POST", body, function(data) u.notify("Note created!", vim.log.levels.INFO) discussions.add_discussion({ data = data, unlinked = true }) - discussions.refresh_discussion_data() + discussions.refresh() end) return end - local reviewer_info = reviewer.get_location(range) - if not reviewer_info then + local reviewer_data = reviewer.get_reviewer_data() + if reviewer_data == nil then + u.notify("Error getting reviewer data", vim.log.levels.ERROR) + return + end + + local location = Location:new(reviewer_data, visual_range) + location:build_location_data() + local location_data = location.location_data + if location_data == nil then + u.notify("Error getting location information", vim.log.levels.ERROR) return end local revision = state.MR_REVISIONS[1] local body = { + type = "text", comment = text, - file_name = reviewer_info.file_name, - old_line = reviewer_info.old_line, - new_line = reviewer_info.new_line, + file_name = reviewer_data.file_name, base_commit_sha = revision.base_commit_sha, start_commit_sha = revision.start_commit_sha, head_commit_sha = revision.head_commit_sha, - type = "text", - line_range = reviewer_info.range_info, + old_line = location_data.old_line, + new_line = location_data.new_line, + line_range = location_data.line_range, } job.run_job("/mr/comment", "POST", body, function(data) u.notify("Comment created!", vim.log.levels.INFO) discussions.add_discussion({ data = data, unlinked = false }) - discussions.refresh_discussion_data() + discussions.refresh() end) end diff --git a/lua/gitlab/actions/create_mr.lua b/lua/gitlab/actions/create_mr.lua index 423b8c8e..9e51dc74 100644 --- a/lua/gitlab/actions/create_mr.lua +++ b/lua/gitlab/actions/create_mr.lua @@ -5,6 +5,7 @@ local Input = require("nui.input") local Popup = require("nui.popup") local job = require("gitlab.job") local u = require("gitlab.utils") +local git = require("gitlab.git") local state = require("gitlab.state") local miscellaneous = require("gitlab.actions.miscellaneous") @@ -124,7 +125,7 @@ M.pick_target = function(args) end local function make_template_path(t) - local base_dir = vim.fn.trim(vim.fn.system({ "git", "rev-parse", "--show-toplevel" })) + local base_dir = git.base_dir() return base_dir .. state.settings.file_separator .. ".gitlab" diff --git a/lua/gitlab/actions/discussions/annotations.lua b/lua/gitlab/actions/discussions/annotations.lua index 72c6db7b..92bbf4e5 100644 --- a/lua/gitlab/actions/discussions/annotations.lua +++ b/lua/gitlab/actions/discussions/annotations.lua @@ -101,3 +101,22 @@ ---@field user_data table ---@field source string ---@field code string? + +---@class LineRange +---@field start_line integer +---@field end_line integer + +---@class DiffviewInfo +---@field modification_type string +---@field file_name string +---@field current_bufnr integer +---@field new_sha_win_id integer +---@field old_sha_win_id integer +---@field opposite_bufnr integer +---@field new_line_from_buf integer +---@field old_line_from_buf integer + +---@class LocationData +---@field old_line integer | nil +---@field new_line integer | nil +---@field line_range ReviewerRangeInfo|nil diff --git a/lua/gitlab/actions/discussions/init.lua b/lua/gitlab/actions/discussions/init.lua index 682971aa..17df478f 100644 --- a/lua/gitlab/actions/discussions/init.lua +++ b/lua/gitlab/actions/discussions/init.lua @@ -9,8 +9,10 @@ local job = require("gitlab.job") local u = require("gitlab.utils") local state = require("gitlab.state") local reviewer = require("gitlab.reviewer") +local List = require("gitlab.utils.list") local miscellaneous = require("gitlab.actions.miscellaneous") local discussions_tree = require("gitlab.actions.discussions.tree") +local diffview_lib = require("diffview.lib") local signs = require("gitlab.actions.discussions.signs") local winbar = require("gitlab.actions.discussions.winbar") local help = require("gitlab.actions.help") @@ -53,28 +55,55 @@ end ---Initialize everything for discussions like setup of signs, callbacks for reviewer, etc. M.initialize_discussions = function() signs.setup_signs() - -- Setup callback to refresh discussion data, discussion signs and diagnostics whenever the reviewed file changes. - reviewer.set_callback_for_file_changed(M.refresh_discussion_data) - -- Setup callback to clear signs and diagnostics whenever reviewer is left. - reviewer.set_callback_for_reviewer_leave(signs.clear_signs_and_diagnostics) + reviewer.set_callback_for_file_changed(function() + M.refresh_view() + M.modifiable(false) + end) + reviewer.set_callback_for_reviewer_enter(function() + M.modifiable(false) + end) + reviewer.set_callback_for_reviewer_leave(function() + signs.clear_signs_and_diagnostics() + M.modifiable(true) + end) +end + +--- Ensures that the both buffers in the reviewer are/not modifiable. Relevant if the user is using +--- the --imply-local setting +M.modifiable = function(bool) + local view = diffview_lib.get_current_view() + local a = view.cur_layout.a.file.bufnr + local b = view.cur_layout.b.file.bufnr + if vim.api.nvim_buf_is_loaded(a) then + vim.api.nvim_buf_set_option(a, "modifiable", bool) + end + if vim.api.nvim_buf_is_loaded(b) then + vim.api.nvim_buf_set_option(b, "modifiable", bool) + end end ---Refresh discussion data, signs, diagnostics, and winbar with new data from API -M.refresh_discussion_data = function() +--- and rebuild the entire view +M.refresh = function() M.load_discussions(function() - if state.settings.discussion_sign.enabled then - signs.refresh_signs(M.discussions) - end - if state.settings.discussion_diagnostic.enabled then - signs.refresh_diagnostics(M.discussions) - end - if M.split_visible then - local linked_is_focused = M.linked_bufnr == M.focused_bufnr - winbar.update_winbar(M.discussions, M.unlinked_discussions, linked_is_focused and "Discussions" or "Notes") - end + M.refresh_view() end) end +--- Take existing data and refresh the diagnostics, the winbar, and the signs +M.refresh_view = function() + if state.settings.discussion_sign.enabled then + signs.refresh_signs(M.discussions) + end + if state.settings.discussion_diagnostic.enabled then + signs.refresh_diagnostics(M.discussions) + end + if M.split_visible then + local linked_is_focused = M.linked_bufnr == M.focused_bufnr + winbar.update_winbar(M.discussions, M.unlinked_discussions, linked_is_focused and "Discussions" or "Notes") + end +end + ---Toggle Discussions tree type between "simple" and "by_file_name" ---@param unlinked boolean True if selected view type is Notes (unlinked discussions) M.toggle_tree_type = function(unlinked) @@ -148,6 +177,7 @@ M.toggle = function(callback) end) end +-- Change between views in the discussion panel, either notes or discussions local switch_view_type = function() local change_to_unlinked = M.linked_bufnr == M.focused_bufnr local new_bufnr = change_to_unlinked and M.unlinked_bufnr or M.linked_bufnr @@ -254,36 +284,23 @@ end -- This function will actually send the deletion to Gitlab -- when you make a selection, and re-render the tree -M.send_deletion = function(tree, unlinked) +M.send_deletion = function(tree) local current_node = tree:get_node() local note_node = M.get_note_node(tree, current_node) local root_node = M.get_root_node(tree, current_node) local note_id = note_node.is_root and root_node.root_note_id or note_node.id - local body = { discussion_id = root_node.id, note_id = tonumber(note_id) } - job.run_job("/mr/comment", "DELETE", body, function(data) u.notify(data.message, vim.log.levels.INFO) - if not note_node.is_root then - tree:remove_node("-" .. note_id) -- Note is not a discussion root, safe to remove - tree:render() + if note_node.is_root then + -- Replace root node w/ current node's contents... + tree:remove_node("-" .. root_node.id) else - if unlinked then - M.unlinked_discussions = u.remove_first_value(M.unlinked_discussions) - M.rebuild_unlinked_discussion_tree() - else - M.discussions = u.remove_first_value(M.discussions) - M.rebuild_discussion_tree() - end - M.add_empty_titles({ - { M.linked_bufnr, M.discussions, "No Discussions for this MR" }, - { M.unlinked_bufnr, M.unlinked_discussions, "No Notes (Unlinked Discussions) for this MR" }, - }) - M.switch_can_edit_bufs(false) + tree:remove_node("-" .. note_id) end - - M.refresh_discussion_data() + tree:render() + M.refresh() end) end @@ -293,18 +310,22 @@ M.edit_comment = function(tree, unlinked) local current_node = tree:get_node() local note_node = M.get_note_node(tree, current_node) local root_node = M.get_root_node(tree, current_node) + if note_node == nil or root_node == nil then + u.notify("Could not get root or note node", vim.log.levels.ERROR) + return + end edit_popup:mount() - local lines = {} -- Gather all lines from immediate children that aren't note nodes - local children_ids = note_node:get_child_ids() - for _, child_id in ipairs(children_ids) do + -- Gather all lines from immediate children that aren't note nodes + local lines = List.new(note_node:get_child_ids()):reduce(function(agg, child_id) local child_node = tree:get_node(child_id) if not child_node:has_children() then local line = tree:get_node(child_id).text - table.insert(lines, line) + table.insert(agg, line) end - end + return agg + end, {}) local currentBuffer = vim.api.nvim_get_current_buf() vim.api.nvim_buf_set_lines(currentBuffer, 0, -1, false, lines) @@ -362,7 +383,7 @@ M.toggle_discussion_resolved = function(tree) job.run_job("/mr/discussions/resolve", "PUT", body, function(data) u.notify(data.message, vim.log.levels.INFO) M.redraw_resolved_status(tree, note, not note.resolved) - M.refresh_discussion_data() + M.refresh() end) end @@ -373,7 +394,17 @@ M.jump_to_reviewer = function(tree) u.notify(error, vim.log.levels.ERROR) return end - reviewer.jump(file_name, new_line, old_line, { is_undefined_type = is_undefined_type }) + + local new_line_int = tonumber(new_line) + local old_line_int = tonumber(old_line) + + if new_line_int == nil and old_line_int == nil then + u.notify("Could not get new or old line", vim.log.levels.ERROR) + return + end + + reviewer.jump(file_name, new_line_int, old_line_int, { is_undefined_type = is_undefined_type }) + M.refresh_view() end -- This function (settings.discussion_tree.jump_to_file) will jump to the file changed in a new tab diff --git a/lua/gitlab/actions/discussions/winbar.lua b/lua/gitlab/actions/discussions/winbar.lua index da325eb5..ada616c9 100644 --- a/lua/gitlab/actions/discussions/winbar.lua +++ b/lua/gitlab/actions/discussions/winbar.lua @@ -1,28 +1,31 @@ local M = {} local state = require("gitlab.state") +local List = require("gitlab.utils.list") ---@param nodes Discussion[]|UnlinkedDiscussion[]|nil +---@return number, number local get_data = function(nodes) - if nodes == nil then - return 0, 0 - end local total_resolvable = 0 local total_resolved = 0 - if nodes == vim.NIL then - return "" + if nodes == nil or nodes == vim.NIL then + return total_resolvable, total_resolved end - for _, d in ipairs(nodes) do + total_resolvable = List.new(nodes):reduce(function(agg, d) local first_child = d.notes[1] - if first_child ~= nil then - if first_child.resolvable then - total_resolvable = total_resolvable + 1 - end - if first_child.resolved then - total_resolved = total_resolved + 1 - end + if first_child and first_child.resolvable then + agg = agg + 1 end - end + return agg + end, 0) + + total_resolved = List.new(nodes):reduce(function(agg, d) + local first_child = d.notes[1] + if first_child and first_child.resolved then + agg = agg + 1 + end + return agg + end, 0) return total_resolvable, total_resolved end diff --git a/lua/gitlab/actions/help.lua b/lua/gitlab/actions/help.lua index d588ba28..1db04489 100644 --- a/lua/gitlab/actions/help.lua +++ b/lua/gitlab/actions/help.lua @@ -2,18 +2,19 @@ local M = {} local u = require("gitlab.utils") local state = require("gitlab.state") +local List = require("gitlab.utils.list") local Popup = require("nui.popup") M.open = function() local bufnr = vim.api.nvim_get_current_buf() local keymaps = vim.api.nvim_buf_get_keymap(bufnr, "n") - local help_content_lines = {} - for _, keymap in ipairs(keymaps) do + local help_content_lines = List.new(keymaps):reduce(function(agg, keymap) if keymap.desc ~= nil then local new_line = string.format("%s: %s", keymap.lhs:gsub(" ", ""), keymap.desc) - table.insert(help_content_lines, new_line) + table.insert(agg, new_line) end - end + return agg + end, {}) local longest_line = u.get_longest_string(help_content_lines) local help_popup = Popup(u.create_popup_state("Help", state.settings.popup.help, longest_line + 3, #help_content_lines + 3, 60)) diff --git a/lua/gitlab/actions/labels.lua b/lua/gitlab/actions/labels.lua index 0ef1c26b..3b25016d 100644 --- a/lua/gitlab/actions/labels.lua +++ b/lua/gitlab/actions/labels.lua @@ -3,6 +3,7 @@ local u = require("gitlab.utils") local job = require("gitlab.job") local state = require("gitlab.state") +local List = require("gitlab.utils.list") local M = {} M.add_label = function() @@ -14,11 +15,9 @@ M.delete_label = function() end local refresh_label_state = function(labels) - local new_labels = "" - for _, label in ipairs(labels) do - new_labels = new_labels .. "," .. label - end - state.INFO.labels = new_labels + state.INFO.labels = List.new(labels):reduce(function(agg, label) + return agg .. "," .. label + end, "") end local get_current_labels = function() @@ -31,11 +30,9 @@ local get_current_labels = function() end local get_all_labels = function() - local labels = {} - for _, label in ipairs(state.LABELS) do -- How can we use the colors?? - table.insert(labels, label.Name) - end - return labels + return List.new(state.LABELS):map(function(label) + return label.Name + end) end M.add_popup = function(type) diff --git a/lua/gitlab/actions/summary.lua b/lua/gitlab/actions/summary.lua index 55cb87ad..ac10ce7e 100644 --- a/lua/gitlab/actions/summary.lua +++ b/lua/gitlab/actions/summary.lua @@ -5,6 +5,7 @@ local Layout = require("nui.layout") local Popup = require("nui.popup") local job = require("gitlab.job") local u = require("gitlab.utils") +local List = require("gitlab.utils.list") local state = require("gitlab.state") local miscellaneous = require("gitlab.actions.miscellaneous") local pipeline = require("gitlab.actions.pipeline") @@ -159,8 +160,7 @@ M.build_info_lines = function() return string.rep(" ", offset + 3) end - local lines = {} - for _, v in ipairs(state.settings.info.fields) do + return List.new(state.settings.info.fields):map(function(v) if v == "merge_status" then v = "detailed_merge_status" end @@ -174,10 +174,8 @@ M.build_info_lines = function() else line = line .. row.content end - table.insert(lines, line) - end - - return lines + return line + end) end -- This function will PUT the new description to the Go server diff --git a/lua/gitlab/git.lua b/lua/gitlab/git.lua new file mode 100644 index 00000000..8d48fb45 --- /dev/null +++ b/lua/gitlab/git.lua @@ -0,0 +1,11 @@ +local M = {} + +M.has_clean_tree = function() + return vim.fn.trim(vim.fn.system({ "git", "status", "--short", "--untracked-files=no" })) == "" +end + +M.base_dir = function() + return vim.fn.trim(vim.fn.system({ "git", "rev-parse", "--show-toplevel" })) +end + +return M diff --git a/lua/gitlab/hunks/init.lua b/lua/gitlab/hunks/init.lua new file mode 100644 index 00000000..90af06eb --- /dev/null +++ b/lua/gitlab/hunks/init.lua @@ -0,0 +1,281 @@ +local List = require("gitlab.utils.list") +local u = require("gitlab.utils") +local state = require("gitlab.state") +local M = {} + +---@class Hunk +---@field old_line integer +---@field old_range integer +---@field new_line integer +---@field new_range integer + +---@class HunksAndDiff +---@field hunks Hunk[] list of hunks +---@field all_diff_output table The data from the git diff command + +---Turn hunk line into Lua table +---@param line table +---@return Hunk|nil +M.parse_possible_hunk_headers = function(line) + if line:sub(1, 2) == "@@" then + -- match: + -- @@ -23 +23 @@ ... + -- @@ -23,0 +23 @@ ... + -- @@ -41,0 +42,4 @@ ... + local old_start, old_range, new_start, new_range = line:match("@@+ %-(%d+),?(%d*) %+(%d+),?(%d*) @@+") + + return { + old_line = tonumber(old_start), + old_range = tonumber(old_range) or 0, + new_line = tonumber(new_start), + new_range = tonumber(new_range) or 0, + } + end +end +---@param linnr number +---@param hunk Hunk +---@param all_diff_output table +local line_was_removed = function(linnr, hunk, all_diff_output) + for matching_line_index, line in ipairs(all_diff_output) do + local found_hunk = M.parse_possible_hunk_headers(line) + if found_hunk ~= nil and vim.deep_equal(found_hunk, hunk) then + -- We found a matching hunk, now we need to iterate over the lines from the raw diff output + -- at that hunk until we reach the line we are looking for. When the indexes match we check + -- to see if that line is deleted or not. + for hunk_line_index = found_hunk.old_line, hunk.old_line + hunk.old_range - 1, 1 do + local line_content = all_diff_output[matching_line_index + 1] + if hunk_line_index == linnr then + if string.match(line_content, "^%-") then + return "deleted" + end + end + end + end + end +end + +---@param linnr number +---@param hunk Hunk +---@param all_diff_output table +local line_was_added = function(linnr, hunk, all_diff_output) + for matching_line_index, line in ipairs(all_diff_output) do + local found_hunk = M.parse_possible_hunk_headers(line) + if found_hunk ~= nil and vim.deep_equal(found_hunk, hunk) then + -- For added lines, we only want to iterate over the part of the diff that has has new lines, + -- so we skip over the old range. We then keep track of the increment to the original new line index, + -- and iterate until we reach the end of the total range of this hunk. If we arrive at the matching + -- index for the line number, we check to see if the line was added. + local i = 0 + local old_range = (found_hunk.old_range == 0 and found_hunk.old_line ~= 0) and 1 or found_hunk.old_range + for hunk_line_index = matching_line_index + old_range + 1, matching_line_index + old_range + found_hunk.new_range, 1 do + local line_content = all_diff_output[hunk_line_index] + if (found_hunk.new_line + i) == linnr then + if string.match(line_content, "^%+") then + return "added" + end + end + i = i + 1 + end + end + end +end + +---Parse git diff hunks. +---@param file_path string Path to file. +---@param base_branch string Git base branch of merge request. +---@return HunksAndDiff +local parse_hunks_and_diff = function(file_path, base_branch) + local hunks = {} + local all_diff_output = {} + + local Job = require("plenary.job") + + local diff_job = Job:new({ + command = "git", + args = { "diff", "--minimal", "--unified=0", "--no-color", base_branch, "--", file_path }, + on_exit = function(j, return_code) + if return_code == 0 then + all_diff_output = j:result() + for _, line in ipairs(all_diff_output) do + local hunk = M.parse_possible_hunk_headers(line) + if hunk ~= nil then + table.insert(hunks, hunk) + end + end + else + M.notify("Failed to get git diff: " .. j:stderr(), vim.log.levels.WARN) + end + end, + }) + + diff_job:sync() + + return { hunks = hunks, all_diff_output = all_diff_output } +end + +-- Parses the lines from a diff and returns the +-- index of the next hunk, when provided an initial index +---@param lines table +---@param i integer +---@return integer|nil +local next_hunk_index = function(lines, i) + for j, line in ipairs(lines) do + local hunk = M.parse_possible_hunk_headers(line) + if hunk ~= nil and j > i then + return j + end + end + return nil +end + +--- Processes the number of changes until the target is reached. This returns +--- a negative or positive number indicating the number of lines in the hunk +--that have been added or removed prior to the target line +---comment +---@param line_number number +---@param hunk Hunk +---@param lines table +---@return integer +local net_changed_in_hunk_before_line = function(line_number, hunk, lines) + local net_lines = 0 + local current_line_old = hunk.old_line + + for _, line in ipairs(lines) do + if line:sub(1, 1) == "-" then + if current_line_old < line_number then + net_lines = net_lines - 1 + end + current_line_old = current_line_old + 1 + elseif line:sub(1, 1) == "+" then + if current_line_old < line_number then + net_lines = net_lines + 1 + end + else + current_line_old = current_line_old + 1 + end + end + + return net_lines +end + +---Counts the total number of changes in a set of lines, can be positive if added lines or negative if removed lines +---@param lines table +---@return integer +local count_changes = function(lines) + local total = 0 + for _, line in ipairs(lines) do + if line:match("^%+") then + total = total + 1 + else + total = total - 1 + end + end + return total +end + +---Returns whether the comment is on a deleted line, added line, or unmodified line. +---This is in order to build the payload for Gitlab correctly by setting the old line and new line. +---@param old_line number|nil +---@param new_line number|nil +---@param current_file string +---@return string|nil +function M.get_modification_type(old_line, new_line, current_file) + local hunk_and_diff_data = parse_hunks_and_diff(current_file, state.INFO.target_branch) + if hunk_and_diff_data.hunks == nil then + return + end + + local hunks = hunk_and_diff_data.hunks + local all_diff_output = hunk_and_diff_data.all_diff_output + + local is_current_sha = require("gitlab.reviewer").is_current_sha() + + for _, hunk in ipairs(hunks) do + local old_line_end = hunk.old_line + hunk.old_range + local new_line_end = hunk.new_line + hunk.new_range + + if is_current_sha then + -- If it is a single line change and neither hunk has a range, then it's added + if new_line >= hunk.new_line and new_line <= new_line_end then + if hunk.new_range == 0 and hunk.old_range == 0 then + return "added" + end + -- If leaving a comment on the new window, we may be commenting on an added line + -- or on an unmodified line. To tell, we have to check whether the line itself is + -- prefixed with "+" and only return "added" if it is. + if line_was_added(new_line, hunk, all_diff_output) then + return "added" + end + end + else + -- It's a deletion if it's in the range of the hunks and the new + -- range is zero, since that is only a deletion hunk, or if we find + -- a match in another hunk with a range, and the corresponding line is prefixed + -- with a "-" only. If it is, then it's a deletion. + if old_line >= hunk.old_line and old_line <= old_line_end and hunk.old_range == 0 then + return "deleted" + end + if + (old_line >= hunk.old_line and old_line <= old_line_end) + or (old_line >= hunk.new_line and new_line <= new_line_end) + then + if line_was_removed(old_line, hunk, all_diff_output) then + return "deleted" + end + end + end + end + + -- If we can't find the line, this means the user is either trying to leave + -- a comment on an unchanged line in the new or old file SHA. This is only + -- allowed in the old file + return is_current_sha and "bad_file_unmodified" or "unmodified" +end + +---Returns the matching line number of a line in the new/old version of the file compared to the current SHA. +---@param old_sha string +---@param new_sha string +---@param file_path string +---@param line_number number +---@return number|nil +M.calculate_matching_line_new = function(old_sha, new_sha, file_path, line_number) + local net_change = 0 + local diff_cmd = string.format("git diff --minimal --unified=0 --no-color %s %s -- %s", old_sha, new_sha, file_path) + local handle = io.popen(diff_cmd) + if handle == nil then + u.notify(string.format("Error running git diff command for %s", file_path), vim.log.levels.ERROR) + return nil + end + + local all_lines = List.new({}) + for line in handle:lines() do + table.insert(all_lines, line) + end + + for i, line in ipairs(all_lines) do + local hunk = M.parse_possible_hunk_headers(line) + if hunk ~= nil then + if line_number <= hunk.old_line then + -- We have reached a hunk which starts after our target, return the changed total lines + return line_number + net_change + end + + local n = next_hunk_index(all_lines, i) or #all_lines + local diff_lines = all_lines:slice(i + 1, n - 1) + + -- If the line is IN the hunk, process the hunk and return the change until that line + if line_number >= hunk.old_line and line_number < hunk.old_line + hunk.old_range then + net_change = line_number + net_change + net_changed_in_hunk_before_line(line_number, hunk, diff_lines) + return net_change + end + + -- If it's not it's after this hunk, just add all the changes and keep iterating + net_change = net_change + count_changes(diff_lines) + end + end + + -- TODO: Possibly handle lines that are out of range in the new files + return line_number +end + +return M diff --git a/lua/gitlab/init.lua b/lua/gitlab/init.lua index ab91d9dc..4a2ee718 100644 --- a/lua/gitlab/init.lua +++ b/lua/gitlab/init.lua @@ -1,3 +1,4 @@ +require("gitlab.utils.list") local u = require("gitlab.utils") local async = require("gitlab.async") local server = require("gitlab.server") diff --git a/lua/gitlab/reviewer/diffview.lua b/lua/gitlab/reviewer/diffview.lua deleted file mode 100755 index fe62e4e5..00000000 --- a/lua/gitlab/reviewer/diffview.lua +++ /dev/null @@ -1,432 +0,0 @@ --- This Module contains all of the reviewer code for diffview -local u = require("gitlab.utils") -local state = require("gitlab.state") -local async_ok, async = pcall(require, "diffview.async") -local diffview_lib = require("diffview.lib") - -local M = { - bufnr = nil, - tabnr = nil, -} - -local all_git_manged_files_unmodified = function() - -- check local managed files are unmodified, matching the state in the MR - -- TODO: ensure correct CWD? - return vim.fn.trim(vim.fn.system({ "git", "status", "--short", "--untracked-files=no" })) == "" -end - -M.open = function() - local diff_refs = state.INFO.diff_refs - if diff_refs == nil then - u.notify("Gitlab did not provide diff refs required to review this MR", vim.log.levels.ERROR) - return - end - - if diff_refs.base_sha == "" or diff_refs.head_sha == "" then - u.notify("Merge request contains no changes", vim.log.levels.ERROR) - return - end - - local diffview_open_command = "DiffviewOpen" - local diffview_feature_imply_local = { - user_requested = state.settings.reviewer_settings.diffview.imply_local, - usable = all_git_manged_files_unmodified(), - } - if diffview_feature_imply_local.user_requested and diffview_feature_imply_local.usable then - diffview_open_command = diffview_open_command .. " --imply-local" - end - - vim.api.nvim_command(string.format("%s %s..%s", diffview_open_command, diff_refs.base_sha, diff_refs.head_sha)) - M.tabnr = vim.api.nvim_get_current_tabpage() - - if diffview_feature_imply_local.user_requested and not diffview_feature_imply_local.usable then - u.notify( - "There are uncommited changes in the working tree, cannot use 'imply_local' setting for gitlab reviews. Stash or commit all changes to use.", - vim.log.levels.WARN - ) - end - - if state.INFO.has_conflicts then - u.notify("This merge request has conflicts!", vim.log.levels.WARN) - end - - -- Register Diffview hook for close event to set tab page # to nil - local on_diffview_closed = function(view) - if view.tabpage == M.tabnr then - M.tabnr = nil - end - end - require("diffview.config").user_emitter:on("view_closed", function(_, ...) - on_diffview_closed(...) - end) - - if state.settings.discussion_tree.auto_open then - local discussions = require("gitlab.actions.discussions") - discussions.close() - discussions.toggle() - end -end - -M.close = function() - vim.cmd("DiffviewClose") - local discussions = require("gitlab.actions.discussions") - discussions.close() -end - -M.jump = function(file_name, new_line, old_line, opts) - if M.tabnr == nil then - u.notify("Can't jump to Diffvew. Is it open?", vim.log.levels.ERROR) - return - end - vim.api.nvim_set_current_tabpage(M.tabnr) - vim.cmd("DiffviewFocusFiles") - local view = diffview_lib.get_current_view() - if view == nil then - u.notify("Could not find Diffview view", vim.log.levels.ERROR) - return - end - local files = view.panel:ordered_file_list() - local layout = view.cur_layout - for _, file in ipairs(files) do - if file.path == file_name then - if not async_ok then - u.notify("Could not load Diffview async", vim.log.levels.ERROR) - return - end - async.await(view:set_file(file)) - -- TODO: Ranged comments on unchanged lines will have both a - -- new line and a old line. - -- - -- The same is true when the user leaves a single-line comment - -- on an unchanged line in the "b" buffer. - -- - -- We need to distinguish them somehow from - -- range comments (which also have this) so that we can know - -- which buffer to jump to. Right now, we jump to the wrong - -- buffer for ranged comments on unchanged lines. - if new_line ~= nil and not opts.is_undefined_type then - layout.b:focus() - vim.api.nvim_win_set_cursor(0, { tonumber(new_line), 0 }) - elseif old_line ~= nil then - layout.a:focus() - vim.api.nvim_win_set_cursor(0, { tonumber(old_line), 0 }) - end - break - end - end -end - ----Get the location of a line within the diffview. If range is specified, then also the location ----of the lines in range. ----@param range LineRange | nil Line range to get location for ----@return ReviewerInfo | nil nil is returned only if error was encountered -M.get_location = function(range) - if M.tabnr == nil then - u.notify("Diffview reviewer must be initialized first", vim.log.levels.ERROR) - return - end - - -- If there's a range, use the start of the visual selection, not the current line - local current_line = range and range.start_line or vim.api.nvim_win_get_cursor(0)[1] - - -- Check if we are in the diffview tab - local tabnr = vim.api.nvim_get_current_tabpage() - if tabnr ~= M.tabnr then - u.notify("Line location can only be determined within reviewer window", vim.log.levels.ERROR) - return - end - - -- Check if we are in the diffview buffer - local view = diffview_lib.get_current_view() - if view == nil then - u.notify("Could not find Diffview view", vim.log.levels.ERROR) - return - end - - local layout = view.cur_layout - - ---@type ReviewerInfo - local reviewer_info = { - file_name = layout.a.file.path, - new_line = nil, - old_line = nil, - range_info = nil, - } - - local a_win = u.get_window_id_by_buffer_id(layout.a.file.bufnr) - local b_win = u.get_window_id_by_buffer_id(layout.b.file.bufnr) - local current_win = vim.fn.win_getid() - local is_current_sha = current_win == b_win - - if a_win == nil or b_win == nil then - u.notify("Error retrieving window IDs for current files", vim.log.levels.ERROR) - return - end - - local current_file = M.get_current_file() - if current_file == nil then - u.notify("Error retrieving current file from Diffview", vim.log.levels.ERROR) - return - end - - local a_linenr = vim.api.nvim_win_get_cursor(a_win)[1] - local b_linenr = vim.api.nvim_win_get_cursor(b_win)[1] - - local data = u.parse_hunk_headers(current_file, state.INFO.target_branch) - - if data.hunks == nil then - u.notify("Could not parse hunks", vim.log.levels.ERROR) - return - end - - -- Will be different depending on focused window. - local modification_type = - M.get_modification_type(a_linenr, b_linenr, is_current_sha, data.hunks, data.all_diff_output) - - if modification_type == "bad_file_unmodified" then - u.notify("Comments on unmodified lines will be placed in the old file", vim.log.levels.WARN) - end - - -- Comment on new line: Include only new_line in payload. - if modification_type == "added" then - reviewer_info.old_line = nil - reviewer_info.new_line = b_linenr - -- Comment on deleted line: Include only new_line in payload. - elseif modification_type == "deleted" then - reviewer_info.old_line = a_linenr - reviewer_info.new_line = nil - -- The line was not found in any hunks, only send the old line number - elseif modification_type == "unmodified" or modification_type == "bad_file_unmodified" then - reviewer_info.old_line = a_linenr - reviewer_info.new_line = b_linenr - end - - if range == nil then - return reviewer_info - end - - -- If leaving a multi-line comment, we want to also add range_info to the payload. - local is_new = reviewer_info.new_line ~= nil - local current_line_info = is_new and u.get_lines_from_hunks(data.hunks, reviewer_info.new_line, is_new) - or u.get_lines_from_hunks(data.hunks, reviewer_info.old_line, is_new) - local type = is_new and "new" or "old" - - ---@type ReviewerRangeInfo - local range_info = { start = {}, ["end"] = {} } - - if current_line == range.start_line then - range_info.start.old_line = current_line_info.old_line - range_info.start.new_line = current_line_info.new_line - range_info.start.type = type - else - local start_line_info = u.get_lines_from_hunks(data.hunks, range.start_line, is_new) - range_info.start.old_line = start_line_info.old_line - range_info.start.new_line = start_line_info.new_line - range_info.start.type = type - end - if current_line == range.end_line then - range_info["end"].old_line = current_line_info.old_line - range_info["end"].new_line = current_line_info.new_line - range_info["end"].type = type - else - local end_line_info = u.get_lines_from_hunks(data.hunks, range.end_line, is_new) - range_info["end"].old_line = end_line_info.old_line - range_info["end"].new_line = end_line_info.new_line - range_info["end"].type = type - end - - reviewer_info.range_info = range_info - return reviewer_info -end - ----Return content between start_line and end_line ----@param start_line integer ----@param end_line integer ----@return string[] -M.get_lines = function(start_line, end_line) - return vim.api.nvim_buf_get_lines(0, start_line - 1, end_line, false) -end - ----Checks whether the lines in the two buffers are the same ----@return boolean -M.lines_are_same = function(layout, a_cursor, b_cursor) - local line_a = u.get_line_content(layout.a.file.bufnr, a_cursor) - local line_b = u.get_line_content(layout.b.file.bufnr, b_cursor) - return line_a == line_b -end - ----Get currently shown file -M.get_current_file = function() - local view = diffview_lib.get_current_view() - if not view then - return - end - return view.panel.cur_file.path -end - ----Place a sign in currently reviewed file. Use new line for identifing lines after changes, old ----line for identifing lines before changes and both if line was not changed. ----@param signs SignTable[] table of signs. See :h sign_placelist ----@param type string "new" if diagnostic should be in file after changes else "old" -M.place_sign = function(signs, type) - local view = diffview_lib.get_current_view() - if not view then - return - end - if type == "new" then - for _, sign in ipairs(signs) do - sign.buffer = view.cur_layout.b.file.bufnr - end - elseif type == "old" then - for _, sign in ipairs(signs) do - sign.buffer = view.cur_layout.a.file.bufnr - end - end - vim.fn.sign_placelist(signs) -end - ----Set diagnostics in currently reviewed file. ----@param namespace integer namespace for diagnostics ----@param diagnostics table see :h vim.diagnostic.set ----@param type string "new" if diagnostic should be in file after changes else "old" ----@param opts table? see :h vim.diagnostic.set -M.set_diagnostics = function(namespace, diagnostics, type, opts) - local view = diffview_lib.get_current_view() - if not view then - return - end - if type == "new" and view.cur_layout.b.file.bufnr then - vim.diagnostic.set(namespace, view.cur_layout.b.file.bufnr, diagnostics, opts) - elseif type == "old" and view.cur_layout.a.file.bufnr then - vim.diagnostic.set(namespace, view.cur_layout.a.file.bufnr, diagnostics, opts) - end -end - ----Diffview exposes events which can be used to setup autocommands. ----@param callback fun(opts: table) - for more information about opts see callback in :h nvim_create_autocmd -M.set_callback_for_file_changed = function(callback) - local group = vim.api.nvim_create_augroup("gitlab.diffview.autocommand.file_changed", {}) - vim.api.nvim_create_autocmd("User", { - pattern = { "DiffviewDiffBufWinEnter", "DiffviewViewEnter" }, - group = group, - callback = function(...) - if M.tabnr == vim.api.nvim_get_current_tabpage() then - callback(...) - end - end, - }) -end - ----Diffview exposes events which can be used to setup autocommands. ----@param callback fun(opts: table) - for more information about opts see callback in :h nvim_create_autocmd -M.set_callback_for_reviewer_leave = function(callback) - local group = vim.api.nvim_create_augroup("gitlab.diffview.autocommand.leave", {}) - vim.api.nvim_create_autocmd("User", { - pattern = { "DiffviewViewLeave", "DiffviewViewClosed" }, - group = group, - callback = function(...) - if M.tabnr == vim.api.nvim_get_current_tabpage() then - callback(...) - end - end, - }) -end - ----Returns whether the comment is on a deleted line, added line, or unmodified line. ----This is in order to build the payload for Gitlab correctly by setting the old line and new line. ----@param a_linenr number ----@param b_linenr number ----@param is_current_sha boolean ----@param hunks Hunk[] A list of hunks ----@param all_diff_output table The raw diff output -function M.get_modification_type(a_linenr, b_linenr, is_current_sha, hunks, all_diff_output) - for _, hunk in ipairs(hunks) do - local old_line_end = hunk.old_line + hunk.old_range - local new_line_end = hunk.new_line + hunk.new_range - - if is_current_sha then - -- If it is a single line change and neither hunk has a range, then it's added - if b_linenr >= hunk.new_line and b_linenr <= new_line_end then - if hunk.new_range == 0 and hunk.old_range == 0 then - return "added" - end - -- If leaving a comment on the new window, we may be commenting on an added line - -- or on an unmodified line. To tell, we have to check whether the line itself is - -- prefixed with "+" and only return "added" if it is. - if M.line_was_added(b_linenr, hunk, all_diff_output) then - return "added" - end - end - else - -- It's a deletion if it's in the range of the hunks and the new - -- range is zero, since that is only a deletion hunk, or if we find - -- a match in another hunk with a range, and the corresponding line is prefixed - -- with a "-" only. If it is, then it's a deletion. - if a_linenr >= hunk.old_line and a_linenr <= old_line_end and hunk.old_range == 0 then - return "deleted" - end - if - (a_linenr >= hunk.old_line and a_linenr <= old_line_end) - or (a_linenr >= hunk.new_line and b_linenr <= new_line_end) - then - if M.line_was_removed(a_linenr, hunk, all_diff_output) then - return "deleted" - end - end - end - end - - -- If we can't find the line, this means the user is either trying to leave - -- a comment on an unchanged line in the new or old file SHA. This is only - -- allowed in the old file - return is_current_sha and "bad_file_unmodified" or "unmodified" -end - ----@param linnr number ----@param hunk Hunk ----@param all_diff_output table -M.line_was_removed = function(linnr, hunk, all_diff_output) - for matching_line_index, line in ipairs(all_diff_output) do - local found_hunk = u.parse_possible_hunk_headers(line) - if found_hunk ~= nil and vim.deep_equal(found_hunk, hunk) then - -- We found a matching hunk, now we need to iterate over the lines from the raw diff output - -- at that hunk until we reach the line we are looking for. When the indexes match we check - -- to see if that line is deleted or not. - for hunk_line_index = found_hunk.old_line, hunk.old_line + hunk.old_range - 1, 1 do - local line_content = all_diff_output[matching_line_index + 1] - if hunk_line_index == linnr then - if string.match(line_content, "^%-") then - return "deleted" - end - end - end - end - end -end - ----@param linnr number ----@param hunk Hunk ----@param all_diff_output table -M.line_was_added = function(linnr, hunk, all_diff_output) - for matching_line_index, line in ipairs(all_diff_output) do - local found_hunk = u.parse_possible_hunk_headers(line) - if found_hunk ~= nil and vim.deep_equal(found_hunk, hunk) then - -- For added lines, we only want to iterate over the part of the diff that has has new lines, - -- so we skip over the old range. We then keep track of the increment to the original new line index, - -- and iterate until we reach the end of the total range of this hunk. If we arrive at the matching - -- index for the line number, we check to see if the line was added. - local i = 0 - local old_range = (found_hunk.old_range == 0 and found_hunk.old_line ~= 0) and 1 or found_hunk.old_range - for hunk_line_index = matching_line_index + old_range + 1, matching_line_index + old_range + found_hunk.new_range, 1 do - local line_content = all_diff_output[hunk_line_index] - if (found_hunk.new_line + i) == linnr then - if string.match(line_content, "^%+") then - return "added" - end - end - i = i + 1 - end - end - end -end -return M diff --git a/lua/gitlab/reviewer/init.lua b/lua/gitlab/reviewer/init.lua index 297ef2f7..3c184908 100644 --- a/lua/gitlab/reviewer/init.lua +++ b/lua/gitlab/reviewer/init.lua @@ -1,74 +1,302 @@ +-- This Module contains all of the reviewer code for diffview +local u = require("gitlab.utils") local state = require("gitlab.state") -local diffview = require("gitlab.reviewer.diffview") +local git = require("gitlab.git") +local hunks = require("gitlab.hunks") +local async_ok, async = pcall(require, "diffview.async") +local diffview_lib = require("diffview.lib") local M = { - reviewer = nil, -} - -local reviewer_map = { - diffview = diffview, + bufnr = nil, + tabnr = nil, } +-- Checks for legacy installations, only Diffview is supported. M.init = function() - local reviewer = reviewer_map[state.settings.reviewer] - if reviewer == nil then + if state.settings.reviewer ~= "diffview" then vim.notify( string.format("gitlab.nvim could not find reviewer %s, only diffview is supported", state.settings.reviewer), vim.log.levels.ERROR ) + end +end + +-- Opens the reviewer window. +M.open = function() + local diff_refs = state.INFO.diff_refs + if diff_refs == nil then + u.notify("Gitlab did not provide diff refs required to review this MR", vim.log.levels.ERROR) + return + end + + if diff_refs.base_sha == "" or diff_refs.head_sha == "" then + u.notify("Merge request contains no changes", vim.log.levels.ERROR) + return + end + + local diffview_open_command = "DiffviewOpen" + local has_clean_tree = git.has_clean_tree() + if state.settings.reviewer_settings.diffview.imply_local and has_clean_tree then + diffview_open_command = diffview_open_command .. " --imply-local" + end + + vim.api.nvim_command(string.format("%s %s..%s", diffview_open_command, diff_refs.base_sha, diff_refs.head_sha)) + M.tabnr = vim.api.nvim_get_current_tabpage() + + if state.settings.reviewer_settings.diffview.imply_local and not has_clean_tree then + u.notify( + "There are uncommited changes in the working tree, cannot use 'imply_local' setting for gitlab reviews.\n Stash or commit all changes to use.", + vim.log.levels.WARN + ) + end + + if state.INFO.has_conflicts then + u.notify("This merge request has conflicts!", vim.log.levels.WARN) + end + + -- Register Diffview hook for close event to set tab page # to nil + local on_diffview_closed = function(view) + if view.tabpage == M.tabnr then + M.tabnr = nil + end + end + require("diffview.config").user_emitter:on("view_closed", function(_, ...) + on_diffview_closed(...) + end) + + if state.settings.discussion_tree.auto_open then + local discussions = require("gitlab.actions.discussions") + discussions.close() + discussions.toggle() + end +end + +-- Closes the reviewer and cleans up +M.close = function() + vim.cmd("DiffviewClose") + local discussions = require("gitlab.actions.discussions") + discussions.close() +end + +-- Jumps to the location provided in the reviewer window +---@param file_name string +---@param new_line number|nil +---@param old_line number|nil +---@param opts table +M.jump = function(file_name, new_line, old_line, opts) + if M.tabnr == nil then + u.notify("Can't jump to Diffvew. Is it open?", vim.log.levels.ERROR) + return + end + vim.api.nvim_set_current_tabpage(M.tabnr) + vim.cmd("DiffviewFocusFiles") + local view = diffview_lib.get_current_view() + if view == nil then + u.notify("Could not find Diffview view", vim.log.levels.ERROR) + return + end + local files = view.panel:ordered_file_list() + local layout = view.cur_layout + for _, file in ipairs(files) do + if file.path == file_name then + if not async_ok then + u.notify("Could not load Diffview async", vim.log.levels.ERROR) + return + end + async.await(view:set_file(file)) + -- TODO: Ranged comments on unchanged lines will have both a + -- new line and a old line. + -- + -- The same is true when the user leaves a single-line comment + -- on an unchanged line in the "b" buffer. + -- + -- We need to distinguish them somehow from + -- range comments (which also have this) so that we can know + -- which buffer to jump to. Right now, we jump to the wrong + -- buffer for ranged comments on unchanged lines. + if new_line ~= nil and not opts.is_undefined_type then + layout.b:focus() + vim.api.nvim_win_set_cursor(0, { new_line, 0 }) + elseif old_line ~= nil then + layout.a:focus() + vim.api.nvim_win_set_cursor(0, { old_line, 0 }) + end + break + end + end +end + +---Get the data from diffview, such as line information and file name. May be used by +---other modules such as the comment module to create line codes or set diagnostics +---@return DiffviewInfo | nil +M.get_reviewer_data = function() + if M.tabnr == nil then + u.notify("Diffview reviewer must be initialized first", vim.log.levels.ERROR) + return + end + + -- Check if we are in the diffview tab + local tabnr = vim.api.nvim_get_current_tabpage() + if tabnr ~= M.tabnr then + u.notify("Line location can only be determined within reviewer window", vim.log.levels.ERROR) + return + end + + -- Check if we are in the diffview buffer + local view = diffview_lib.get_current_view() + if view == nil then + u.notify("Could not find Diffview view", vim.log.levels.ERROR) + return + end + + local layout = view.cur_layout + local old_win = u.get_window_id_by_buffer_id(layout.a.file.bufnr) + local new_win = u.get_window_id_by_buffer_id(layout.b.file.bufnr) + + if old_win == nil or new_win == nil then + u.notify("Error getting window IDs for current files", vim.log.levels.ERROR) + return + end + + local current_file = M.get_current_file() + if current_file == nil then + u.notify("Error getting current file from Diffview", vim.log.levels.ERROR) + return + end + + local new_line = vim.api.nvim_win_get_cursor(new_win)[1] + local old_line = vim.api.nvim_win_get_cursor(old_win)[1] + local modification_type = hunks.get_modification_type(old_line, new_line, current_file) + if modification_type == nil then + u.notify("Error parsing hunks for modification type", vim.log.levels.ERROR) + return + end + + if modification_type == "bad_file_unmodified" then + u.notify("Comments on unmodified lines will be placed in the old file", vim.log.levels.WARN) + end + + local current_bufnr = M.is_current_sha() and layout.b.file.bufnr or layout.a.file.bufnr + local opposite_bufnr = M.is_current_sha() and layout.a.file.bufnr or layout.b.file.bufnr + local old_sha_win_id = u.get_window_id_by_buffer_id(layout.a.file.bufnr) + local new_sha_win_id = u.get_window_id_by_buffer_id(layout.b.file.bufnr) + + return { + file_name = layout.a.file.path, + old_line_from_buf = old_line, + new_line_from_buf = new_line, + modification_type = modification_type, + new_sha_win_id = new_sha_win_id, + current_bufnr = current_bufnr, + old_sha_win_id = old_sha_win_id, + opposite_bufnr = opposite_bufnr, + } +end + +---Return whether user is focused on the new version of the file +---@return boolean +M.is_current_sha = function() + local view = diffview_lib.get_current_view() + local layout = view.cur_layout + local b_win = u.get_window_id_by_buffer_id(layout.b.file.bufnr) + local current_win = vim.fn.win_getid() + return current_win == b_win +end + +---Checks whether the lines in the two buffers are the same +---@return boolean +M.lines_are_same = function(layout, a_cursor, b_cursor) + local line_a = u.get_line_content(layout.a.file.bufnr, a_cursor) + local line_b = u.get_line_content(layout.b.file.bufnr, b_cursor) + return line_a == line_b +end + +---Get currently shown file +---@return string|nil +M.get_current_file = function() + local view = diffview_lib.get_current_view() + if not view then + return + end + return view.panel.cur_file.path +end + +-- Places a sign on the line for currently reviewed file. +---@param signs SignTable[] table of signs. See :h sign_placelist +---@param type string "new" if diagnostic should be in file after changes else "old" +M.place_sign = function(signs, type) + local view = diffview_lib.get_current_view() + if not view then return end + if type == "new" then + for _, sign in ipairs(signs) do + sign.buffer = view.cur_layout.b.file.bufnr + end + elseif type == "old" then + for _, sign in ipairs(signs) do + sign.buffer = view.cur_layout.a.file.bufnr + end + end + vim.fn.sign_placelist(signs) +end + +---Set diagnostics in currently reviewed file. +---@param namespace integer namespace for diagnostics +---@param diagnostics table see :h vim.diagnostic.set +---@param type string "new" if diagnostic should be in file after changes else "old" +---@param opts table? see :h vim.diagnostic.set +M.set_diagnostics = function(namespace, diagnostics, type, opts) + local view = diffview_lib.get_current_view() + if not view then + return + end + if type == "new" and view.cur_layout.b.file.bufnr then + vim.diagnostic.set(namespace, view.cur_layout.b.file.bufnr, diagnostics, opts) + elseif type == "old" and view.cur_layout.a.file.bufnr then + vim.diagnostic.set(namespace, view.cur_layout.a.file.bufnr, diagnostics, opts) + end +end + +---Diffview exposes events which can be used to setup autocommands. +---@param callback fun(opts: table) - for more information about opts see callback in :h nvim_create_autocmd +M.set_callback_for_file_changed = function(callback) + local group = vim.api.nvim_create_augroup("gitlab.diffview.autocommand.file_changed", {}) + vim.api.nvim_create_autocmd("User", { + pattern = { "DiffviewDiffBufWinEnter" }, + group = group, + callback = function(...) + if M.tabnr == vim.api.nvim_get_current_tabpage() then + callback(...) + end + end, + }) +end + +---Diffview exposes events which can be used to setup autocommands. +---@param callback fun(opts: table) - for more information about opts see callback in :h nvim_create_autocmd +M.set_callback_for_reviewer_leave = function(callback) + local group = vim.api.nvim_create_augroup("gitlab.diffview.autocommand.leave", {}) + vim.api.nvim_create_autocmd("User", { + pattern = { "DiffviewViewLeave", "DiffviewViewClosed" }, + group = group, + callback = function(...) + if M.tabnr == vim.api.nvim_get_current_tabpage() then + callback(...) + end + end, + }) +end - M.open = reviewer.open - -- Opens the reviewer window - - M.close = reviewer.close - -- Closes the reviewer and cleans up - - M.jump = reviewer.jump - -- Jumps to the location provided in the reviewer window - -- Parameters: - -- • {file_name} The name of the file to jump to - -- • {new_line} The new_line of the change - -- • {interval} The old_line of the change - - M.get_location = reviewer.get_location - -- Parameters: - -- • {range} LineRange if function was triggered from visual selection - -- Returns the current location (based on cursor) from the reviewer window as ReviewerInfo class - - M.get_lines = reviewer.get_lines - -- Returns the content of the file in the current location in the reviewer window - - M.get_current_file = reviewer.get_current_file - -- Get currently loaded file - - M.place_sign = reviewer.place_sign - -- Places a sign on the line for currently reviewed file. - -- Parameters: - -- • {id} The sign id - -- • {sign} The sign to place - -- • {group} The sign group to place on - -- • {new_line} The line to place the sign on - -- • {old_line} The buffer number to place the sign on - - M.set_callback_for_file_changed = reviewer.set_callback_for_file_changed - -- Call callback whenever the file changes - -- Parameters: - -- • {callback} The callback to call - - M.set_callback_for_reviewer_leave = reviewer.set_callback_for_reviewer_leave - -- Call callback whenever the reviewer is left - -- Parameters: - -- • {callback} The callback to call - - M.set_diagnostics = reviewer.set_diagnostics - -- Set diagnostics for currently reviewed file - -- Parameters: - -- • {namespace} The namespace for diagnostics - -- • {diagnostics} The diagnostics to set - -- • {type} "new" if diagnostic should be in file after changes else "old" - -- • {opts} see opts in :h vim.diagnostic.set +M.set_callback_for_reviewer_enter = function(callback) + local group = vim.api.nvim_create_augroup("gitlab.diffview.autocommand.enter", {}) + vim.api.nvim_create_autocmd("User", { + pattern = { "DiffviewViewOpened" }, + group = group, + callback = function(...) + callback(...) + end, + }) end return M diff --git a/lua/gitlab/reviewer/location.lua b/lua/gitlab/reviewer/location.lua new file mode 100755 index 00000000..95e85c76 --- /dev/null +++ b/lua/gitlab/reviewer/location.lua @@ -0,0 +1,206 @@ +local u = require("gitlab.utils") +local hunks = require("gitlab.hunks") +local state = require("gitlab.state") + +---@class Location +---@field location_data LocationData +---@field reviewer_data DiffviewInfo +---@field run function +---@field build_location_data function + +---@class ReviewerLineInfo +---@field old_line integer|nil +---@field new_line integer|nil +---@field type "new"|"old" + +---@class ReviewerRangeInfo +---@field start ReviewerLineInfo +---@field end ReviewerLineInfo + +local Location = {} +Location.__index = Location +---@param reviewer_data DiffviewInfo +---@param visual_range LineRange | nil +---@return Location +function Location.new(reviewer_data, visual_range) + local instance = setmetatable({}, Location) + instance.reviewer_data = reviewer_data + instance.visual_range = visual_range + instance.base_sha = state.INFO.diff_refs.base_sha + instance.head_sha = state.INFO.diff_refs.head_sha + return instance +end + +---Takes in information about the current changes, such as the file name, modification type of the diff, and the line numbers +---and builds the appropriate payload when creating a comment. +function Location:build_location_data() + ---@type DiffviewInfo + local reviewer_data = self.reviewer_data + ---@type LineRange | nil + local visual_range = self.visual_range + + ---@type LocationData + local location_data = { + old_line = nil, + new_line = nil, + line_range = nil, + } + + -- Comment on new line: Include only new_line in payload. + -- Comment on deleted line: Include only old_line in payload. + -- The line was not found in any hunks, send both lines. + if reviewer_data.modification_type == "added" then + location_data.old_line = nil + location_data.new_line = reviewer_data.new_line_from_buf + elseif reviewer_data.modification_type == "deleted" then + location_data.old_line = reviewer_data.old_line_from_buf + location_data.new_line = nil + elseif + reviewer_data.modification_type == "unmodified" or reviewer_data.modification_type == "bad_file_unmodified" + then + location_data.old_line = reviewer_data.old_line_from_buf + location_data.new_line = reviewer_data.new_line_from_buf + end + + self.location_data = location_data + if visual_range == nil then + return + else + self.location_data.line_range = {} + end + + self:set_start_range(visual_range) + self:set_end_range(visual_range) + + -- Ranged comments should always use the end of the range. + -- Otherwise they will not highlight the full comment in Gitlab. + self.location_data.old_line = self.location_data.line_range["end"].old_line + self.location_data.new_line = self.location_data.line_range["end"].new_line +end + +-- Helper methods 🤝 + +-- Returns the matching line from the new SHA. +-- For instance, line 12 in the new SHA may be scroll-linked +-- to line 10 in the old SHA. +---@param line number +---@return number|nil +function Location:get_line_number_from_new_sha(line) + local reviewer = require("gitlab.reviewer") + local is_current_sha = reviewer.is_current_sha() + if is_current_sha then + return line + end + -- Otherwise we want to get the matching line in the opposite buffer + return hunks.calculate_matching_line_new(self.base_sha, self.head_sha, self.reviewer_data.file_name, line) +end + +-- Returns the matching line from the old SHA. +-- For instance, line 12 in the new SHA may be scroll-linked +-- to line 10 in the old SHA. +---@param line number +---@return number|nil +function Location:get_line_number_from_old_sha(line) + local reviewer = require("gitlab.reviewer") + local is_current_sha = reviewer.is_current_sha() + if not is_current_sha then + return line + end + + -- Otherwise we want to get the matching line in the opposite buffer + return hunks.calculate_matching_line_new(self.head_sha, self.base_sha, self.reviewer_data.file_name, line) +end + +-- Returns the current line number from whatever SHA (new or old) +-- the reviewer is focused in. +---@return number|nil +function Location:get_current_line() + local reviewer = require("gitlab.reviewer") + local win_id = reviewer.is_current_sha() and self.reviewer_data.new_sha_win_id or self.reviewer_data.old_sha_win_id + if win_id == nil then + return + end + + local current_line = vim.api.nvim_win_get_cursor(win_id)[1] + return current_line +end + +-- Given a new_line and old_line from the start of a ranged comment, returns the start +-- range information for the Gitlab payload +---@param visual_range LineRange +---@return ReviewerLineInfo|nil +function Location:set_start_range(visual_range) + local current_file = require("gitlab.reviewer").get_current_file() + if current_file == nil then + u.notify("Error getting current file from Diffview", vim.log.levels.ERROR) + return + end + + local reviewer = require("gitlab.reviewer") + local win_id = reviewer.is_current_sha() and self.reviewer_data.new_sha_win_id or self.reviewer_data.old_sha_win_id + if win_id == nil then + u.notify("Error getting window number of SHA for start range", vim.log.levels.ERROR) + return + end + + local current_line = self:get_current_line() + if current_line == nil then + u.notify("Error getting window number of SHA for start range", vim.log.levels.ERROR) + return + end + + local new_line = self:get_line_number_from_new_sha(visual_range.start_line) + local old_line = self:get_line_number_from_old_sha(visual_range.start_line) + if + (new_line == nil and self.reviewer_data.modification_type ~= "deleted") + or (old_line == nil and self.reviewer_data.modification_type ~= "added") + then + u.notify("Error getting new or old line for start range", vim.log.levels.ERROR) + return + end + + local modification_type = hunks.get_modification_type(old_line, new_line, current_file) + + self.location_data.line_range.start = { + new_line = modification_type ~= "deleted" and new_line or nil, + old_line = modification_type ~= "added" and old_line or nil, + type = modification_type == "added" and "new" or "old", + } +end + +-- Given a modification type, a range, and the hunk data, returns the end range information +-- for the Gitlab payload +---@param visual_range LineRange +function Location:set_end_range(visual_range) + local current_file = require("gitlab.reviewer").get_current_file() + if current_file == nil then + u.notify("Error getting current file from Diffview", vim.log.levels.ERROR) + return + end + + local current_line = self:get_current_line() + if current_line == nil then + u.notify("Error getting window number of SHA for start range", vim.log.levels.ERROR) + return + end + + local new_line = self:get_line_number_from_new_sha(visual_range.end_line) + local old_line = self:get_line_number_from_old_sha(visual_range.end_line) + + if + (new_line == nil and self.reviewer_data.modification_type ~= "deleted") + or (old_line == nil and self.reviewer_data.modification_type ~= "added") + then + u.notify("Error getting new or old line for end range", vim.log.levels.ERROR) + return + end + + local modification_type = hunks.get_modification_type(old_line, new_line, current_file) + self.location_data.line_range["end"] = { + new_line = modification_type ~= "deleted" and new_line or nil, + old_line = modification_type ~= "added" and old_line or nil, + type = modification_type == "added" and "new" or "old", + } +end + +return Location diff --git a/lua/gitlab/server.lua b/lua/gitlab/server.lua index 53f9ac02..def61af9 100644 --- a/lua/gitlab/server.lua +++ b/lua/gitlab/server.lua @@ -1,6 +1,7 @@ -- This module contains the logic responsible for building and starting -- the Golang server. The Go server is responsible for making API calls -- to Gitlab and returning the data +local List = require("gitlab.utils.list") local state = require("gitlab.state") local u = require("gitlab.utils") local job = require("gitlab.job") @@ -49,12 +50,12 @@ M.start = function(callback) end end, on_stderr = function(_, errors) - local err_msg = "" - for _, err in ipairs(errors) do + local err_msg = List.new(errors):reduce(function(agg, err) if err ~= "" and err ~= nil then - err_msg = err_msg .. err .. "\n" + agg = agg .. err .. "\n" end - end + return agg + end, "") if err_msg ~= "" then u.notify(err_msg, vim.log.levels.ERROR) diff --git a/lua/gitlab/utils/init.lua b/lua/gitlab/utils/init.lua index 63886ff7..fe05ccbc 100644 --- a/lua/gitlab/utils/init.lua +++ b/lua/gitlab/utils/init.lua @@ -1,3 +1,4 @@ +local List = require("gitlab.utils.list") local has_devicons, devicons = pcall(require, "nvim-web-devicons") local M = {} @@ -484,14 +485,10 @@ M.get_window_id_by_buffer_id = function(buffer_id) local tabpage = vim.api.nvim_get_current_tabpage() local windows = vim.api.nvim_tabpage_list_wins(tabpage) - for _, win_id in ipairs(windows) do + return List.new(windows):find(function(win_id) local buf_id = vim.api.nvim_win_get_buf(win_id) - if buf_id == buffer_id then - return win_id - end - end - - return nil -- Buffer ID not found in any window + return buf_id == buffer_id + end) end M.list_files_in_folder = function(folder_path) @@ -507,166 +504,21 @@ M.list_files_in_folder = function(folder_path) local files = {} if folder ~= nil then - for _, file in ipairs(folder) do - local file_path = folder_path .. M.path_separator .. file - local timestamp = vim.fn.getftime(file_path) - table.insert(files, { name = file, timestamp = timestamp }) - end + files = List.new(folder) + :map(function(file) + local file_path = folder_path .. M.path_separator .. file + local timestamp = vim.fn.getftime(file_path) + return { name = file, timestamp = timestamp } + end) + :sort(function(a, b) + return a.timestamp > b.timestamp + end) + :map(function(file) + return file.name + end) end - -- Sort the table by timestamp in descending order (newest first) - table.sort(files, function(a, b) - return a.timestamp > b.timestamp - end) - - local result = {} - for _, file in ipairs(files) do - table.insert(result, file.name) - end - - return result -end - ----@class Hunk ----@field old_line integer ----@field old_range integer ----@field new_line integer ----@field new_range integer - ----@class HunksAndDiff ----@field hunks Hunk[] list of hunks ----@field all_diff_output table The data from the git diff command - ----Turn hunk line into Lua table ----@param line table ----@return Hunk|nil -M.parse_possible_hunk_headers = function(line) - if line:sub(1, 2) == "@@" then - -- match: - -- @@ -23 +23 @@ ... - -- @@ -23,0 +23 @@ ... - -- @@ -41,0 +42,4 @@ ... - local old_start, old_range, new_start, new_range = line:match("@@+ %-(%d+),?(%d*) %+(%d+),?(%d*) @@+") - - return { - old_line = tonumber(old_start), - old_range = tonumber(old_range) or 0, - new_line = tonumber(new_start), - new_range = tonumber(new_range) or 0, - } - end -end - ----Parse git diff hunks. ----@param file_path string Path to file. ----@param base_branch string Git base branch of merge request. ----@return HunksAndDiff -M.parse_hunk_headers = function(file_path, base_branch) - local hunks = {} - local all_diff_output = {} - - local Job = require("plenary.job") - - local diff_job = Job:new({ - command = "git", - args = { "diff", "--minimal", "--unified=0", "--no-color", base_branch, "--", file_path }, - on_exit = function(j, return_code) - if return_code == 0 then - all_diff_output = j:result() - for _, line in ipairs(all_diff_output) do - local hunk = M.parse_possible_hunk_headers(line) - if hunk ~= nil then - table.insert(hunks, hunk) - end - end - else - M.notify("Failed to get git diff: " .. j:stderr(), vim.log.levels.WARN) - end - end, - }) - - diff_job:sync() - - return { hunks = hunks, all_diff_output = all_diff_output } -end - ----@class LineDiffInfo ----@field old_line integer ----@field new_line integer ----@field in_hunk boolean - ----Search git diff hunks to find old and new line number corresponding to target line. ----This function does not check if target line is outside of boundaries of file. ----@param hunks Hunk[] git diff parsed hunks. ----@param target_line integer line number to search for - based on is_new paramter the search is ----either in new lines or old lines of hunks. ----@param is_new boolean whether to search for new line or old line ----@return LineDiffInfo -M.get_lines_from_hunks = function(hunks, target_line, is_new) - if #hunks == 0 then - -- If there are zero hunks, return target_line for both old and new lines - return { old_line = target_line, new_line = target_line, in_hunk = false } - end - local current_new_line = 0 - local current_old_line = 0 - if is_new then - for _, hunk in ipairs(hunks) do - -- target line is before current hunk - if target_line < hunk.new_line then - return { - old_line = current_old_line + (target_line - current_new_line), - new_line = target_line, - in_hunk = false, - } - -- target line is within the current hunk - elseif hunk.new_line <= target_line and target_line <= (hunk.new_line + hunk.new_range) then - -- this is interesting magic of gitlab calculation - return { - old_line = hunk.old_line + hunk.old_range + 1, - new_line = target_line, - in_hunk = true, - } - -- target line is after the current hunk - else - current_new_line = hunk.new_line + hunk.new_range - current_old_line = hunk.old_line + hunk.old_range - end - end - -- target line is after last hunk - return { - old_line = current_old_line + (target_line - current_new_line), - new_line = target_line, - in_hunk = false, - } - else - for _, hunk in ipairs(hunks) do - -- target line is before current hunk - if target_line < hunk.old_line then - return { - old_line = target_line, - new_line = current_new_line + (target_line - current_old_line), - in_hunk = false, - } - -- target line is within the current hunk - elseif hunk.old_line <= target_line and target_line <= (hunk.old_line + hunk.old_range) then - return { - old_line = target_line, - new_line = hunk.new_line, - in_hunk = true, - } - -- target line is after the current hunk - else - current_new_line = hunk.new_line + hunk.new_range - current_old_line = hunk.old_line + hunk.old_range - end - end - -- target line is after last hunk - return { - old_line = current_old_line + (target_line - current_new_line), - new_line = target_line, - in_hunk = false, - } - end + return files end ---Check if current mode is visual mode @@ -707,6 +559,14 @@ M.get_icon = function(filename) end end +---Return content between start_line and end_line +---@param start_line integer +---@param end_line integer +---@return string[] +M.get_lines = function(start_line, end_line) + return vim.api.nvim_buf_get_lines(0, start_line - 1, end_line, false) +end + M.make_comma_separated_readable = function(str) return string.gsub(str, ",", ", ") end @@ -731,7 +591,7 @@ M.get_all_git_branches = function(remote) end handle:close() else - print("Error running 'git branch' command.") + M.notify("Error running 'git branch' command.", vim.log.levels.ERROR) end return branches diff --git a/lua/gitlab/utils/list.lua b/lua/gitlab/utils/list.lua new file mode 100644 index 00000000..27180388 --- /dev/null +++ b/lua/gitlab/utils/list.lua @@ -0,0 +1,66 @@ +local List = {} +List.__index = List + +function List.new(t) + local list = t or {} + setmetatable(list, List) + return list +end + +---Mutates a given list +---@generic T +---@param func fun(v: T):T +---@return List @Returns a new list of elements mutated by func +function List:map(func) + local result = List.new() + for i, v in ipairs(self) do + result[i] = func(v) + end + return result +end + +---Filters a given list +---@generic T +---@param func fun(v: T):boolean +---@return List @Returns a new list of elements for which func returns true +function List:filter(func) + local result = List.new() + for i, v in ipairs(self) do + if func(v) == true then + result[i] = v + end + end + return result +end + +function List:reduce(func, agg) + for i, v in ipairs(self) do + agg = func(agg, v, i) + end + return agg +end + +function List:sort(func) + local result = List.new(self) + table.sort(result, func) + return result +end + +function List:find(func) + for _, v in ipairs(self) do + if func(v) == true then + return v + end + end + return nil +end + +function List:slice(first, last, step) + local sliced = {} + for i = first or 1, last or #self, step or 1 do + sliced[#sliced + 1] = self[i] + end + return sliced +end + +return List From 8859c47bbce352c15b341978a2294fb11134ae72 Mon Sep 17 00:00:00 2001 From: "Harrison (Harry) Cramer" <32515581+harrisoncramer@users.noreply.github.com> Date: Tue, 27 Feb 2024 19:07:13 -0500 Subject: [PATCH 06/16] Fix: location provider (#198) --- lua/gitlab/actions/comment.lua | 2 +- lua/gitlab/reviewer/location.lua | 12 ++++++++---- 2 files changed, 9 insertions(+), 5 deletions(-) diff --git a/lua/gitlab/actions/comment.lua b/lua/gitlab/actions/comment.lua index e81b4639..23d4d259 100644 --- a/lua/gitlab/actions/comment.lua +++ b/lua/gitlab/actions/comment.lua @@ -131,7 +131,7 @@ M.confirm_create_comment = function(text, visual_range, unlinked) return end - local location = Location:new(reviewer_data, visual_range) + local location = Location.new(reviewer_data, visual_range) location:build_location_data() local location_data = location.location_data if location_data == nil then diff --git a/lua/gitlab/reviewer/location.lua b/lua/gitlab/reviewer/location.lua index 95e85c76..782a68e8 100755 --- a/lua/gitlab/reviewer/location.lua +++ b/lua/gitlab/reviewer/location.lua @@ -23,7 +23,8 @@ Location.__index = Location ---@param visual_range LineRange | nil ---@return Location function Location.new(reviewer_data, visual_range) - local instance = setmetatable({}, Location) + local location = {} + local instance = setmetatable(location, Location) instance.reviewer_data = reviewer_data instance.visual_range = visual_range instance.base_sha = state.INFO.diff_refs.base_sha @@ -66,7 +67,10 @@ function Location:build_location_data() if visual_range == nil then return else - self.location_data.line_range = {} + self.location_data.line_range = { + start = {}, + ["end"] = {}, + } end self:set_start_range(visual_range) @@ -145,7 +149,7 @@ function Location:set_start_range(visual_range) local current_line = self:get_current_line() if current_line == nil then - u.notify("Error getting window number of SHA for start range", vim.log.levels.ERROR) + u.notify("Error getting current line for start range", vim.log.levels.ERROR) return end @@ -180,7 +184,7 @@ function Location:set_end_range(visual_range) local current_line = self:get_current_line() if current_line == nil then - u.notify("Error getting window number of SHA for start range", vim.log.levels.ERROR) + u.notify("Error getting current line for end range", vim.log.levels.ERROR) return end From 00e3e24be4646cde672775cd9e6720a684e7654f Mon Sep 17 00:00:00 2001 From: "Harrison (Harry) Cramer" <32515581+harrisoncramer@users.noreply.github.com> Date: Wed, 28 Feb 2024 09:13:09 -0500 Subject: [PATCH 07/16] fix: add nil check for diffview performance issue (#199) --- lua/gitlab/actions/discussions/init.lua | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lua/gitlab/actions/discussions/init.lua b/lua/gitlab/actions/discussions/init.lua index 17df478f..6d614c61 100644 --- a/lua/gitlab/actions/discussions/init.lua +++ b/lua/gitlab/actions/discussions/init.lua @@ -74,10 +74,10 @@ M.modifiable = function(bool) local view = diffview_lib.get_current_view() local a = view.cur_layout.a.file.bufnr local b = view.cur_layout.b.file.bufnr - if vim.api.nvim_buf_is_loaded(a) then + if a ~= nil and vim.api.nvim_buf_is_loaded(a) then vim.api.nvim_buf_set_option(a, "modifiable", bool) end - if vim.api.nvim_buf_is_loaded(b) then + if b ~= nil and vim.api.nvim_buf_is_loaded(b) then vim.api.nvim_buf_set_option(b, "modifiable", bool) end end From 8937783040f91fbe036b1655faac31c20ac72ffd Mon Sep 17 00:00:00 2001 From: "Harrison (Harry) Cramer" <32515581+harrisoncramer@users.noreply.github.com> Date: Wed, 28 Feb 2024 13:44:28 -0500 Subject: [PATCH 08/16] Fix: Switch Tabs During Comment Creation (#200) Fix: Allow users to switch tabs while creating comments --- lua/gitlab/reviewer/init.lua | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/lua/gitlab/reviewer/init.lua b/lua/gitlab/reviewer/init.lua index 3c184908..a04727fb 100644 --- a/lua/gitlab/reviewer/init.lua +++ b/lua/gitlab/reviewer/init.lua @@ -9,6 +9,7 @@ local diffview_lib = require("diffview.lib") local M = { bufnr = nil, tabnr = nil, + stored_win = nil, } -- Checks for legacy installations, only Diffview is supported. @@ -199,7 +200,14 @@ M.is_current_sha = function() local view = diffview_lib.get_current_view() local layout = view.cur_layout local b_win = u.get_window_id_by_buffer_id(layout.b.file.bufnr) + local a_win = u.get_window_id_by_buffer_id(layout.a.file.bufnr) local current_win = vim.fn.win_getid() + + -- Handle cases where user navigates tabs in the middle of making a comment + if a_win ~= current_win and b_win ~= current_win then + current_win = M.stored_win + M.stored_win = nil + end return current_win == b_win end @@ -266,6 +274,7 @@ M.set_callback_for_file_changed = function(callback) pattern = { "DiffviewDiffBufWinEnter" }, group = group, callback = function(...) + M.stored_win = vim.api.nvim_get_current_win() if M.tabnr == vim.api.nvim_get_current_tabpage() then callback(...) end From 7a3e761aa3deddddbc80214e04007a27db49d20d Mon Sep 17 00:00:00 2001 From: "Harrison (Harry) Cramer" <32515581+harrisoncramer@users.noreply.github.com> Date: Wed, 28 Feb 2024 13:55:04 -0500 Subject: [PATCH 09/16] Fix: Check if file is modified (#201) --- lua/gitlab/actions/comment.lua | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/lua/gitlab/actions/comment.lua b/lua/gitlab/actions/comment.lua index 23d4d259..cee4094d 100644 --- a/lua/gitlab/actions/comment.lua +++ b/lua/gitlab/actions/comment.lua @@ -23,9 +23,10 @@ end -- line in the current MR M.create_comment = function() local has_clean_tree = git.has_clean_tree() - if state.settings.reviewer_settings.diffview.imply_local and not has_clean_tree then + local is_modified = vim.api.nvim_buf_get_option(0, "modified") + if state.settings.reviewer_settings.diffview.imply_local and (is_modified or not has_clean_tree) then u.notify( - "Cannot leave comments on a dirty tree. \n Please stash all local changes or push them up.", + "Cannot leave comments on changed files. \n Please stash all local changes or push them to the feature branch.", vim.log.levels.WARN ) return From 4c4f4b5230e030557df62e015c6fef3d07785f99 Mon Sep 17 00:00:00 2001 From: "Harrison (Harry) Cramer" <32515581+harrisoncramer@users.noreply.github.com> Date: Wed, 28 Feb 2024 16:29:49 -0500 Subject: [PATCH 10/16] Fix: Off by 1 Error in Old Sha (#202) Fixes an issue in the old sha where the index when iterating through the lines of the hunk in the old SHA was off by one --- lua/gitlab/hunks/init.lua | 93 ++++++++++++++++---------------- lua/gitlab/reviewer/init.lua | 10 ++-- lua/gitlab/reviewer/location.lua | 18 +++++-- 3 files changed, 67 insertions(+), 54 deletions(-) diff --git a/lua/gitlab/hunks/init.lua b/lua/gitlab/hunks/init.lua index 90af06eb..1f17f5b4 100644 --- a/lua/gitlab/hunks/init.lua +++ b/lua/gitlab/hunks/init.lua @@ -35,6 +35,7 @@ end ---@param linnr number ---@param hunk Hunk ---@param all_diff_output table +---@return boolean local line_was_removed = function(linnr, hunk, all_diff_output) for matching_line_index, line in ipairs(all_diff_output) do local found_hunk = M.parse_possible_hunk_headers(line) @@ -42,21 +43,23 @@ local line_was_removed = function(linnr, hunk, all_diff_output) -- We found a matching hunk, now we need to iterate over the lines from the raw diff output -- at that hunk until we reach the line we are looking for. When the indexes match we check -- to see if that line is deleted or not. - for hunk_line_index = found_hunk.old_line, hunk.old_line + hunk.old_range - 1, 1 do + for hunk_line_index = found_hunk.old_line, hunk.old_line + hunk.old_range, 1 do local line_content = all_diff_output[matching_line_index + 1] if hunk_line_index == linnr then if string.match(line_content, "^%-") then - return "deleted" + return true end end end end end + return false end ---@param linnr number ---@param hunk Hunk ---@param all_diff_output table +---@return boolean local line_was_added = function(linnr, hunk, all_diff_output) for matching_line_index, line in ipairs(all_diff_output) do local found_hunk = M.parse_possible_hunk_headers(line) @@ -71,13 +74,14 @@ local line_was_added = function(linnr, hunk, all_diff_output) local line_content = all_diff_output[hunk_line_index] if (found_hunk.new_line + i) == linnr then if string.match(line_content, "^%+") then - return "added" + return true end end i = i + 1 end end end + return false end ---Parse git diff hunks. @@ -173,13 +177,49 @@ local count_changes = function(lines) return total end +---@param new_line number|nil +---@param hunks Hunk[] +---@param all_diff_output table +---@return string|nil +local function get_modification_type_from_new_sha(new_line, hunks, all_diff_output) + if new_line == nil then + return nil + end + return List.new(hunks):find(function(hunk) + local new_line_end = hunk.new_line + hunk.new_range + local in_new_range = new_line >= hunk.new_line and new_line <= new_line_end + local is_range_zero = hunk.new_range == 0 and hunk.old_range == 0 + return in_new_range and (is_range_zero or line_was_added(new_line, hunk, all_diff_output)) + end) and "added" or "bad_file_unmodified" +end + +---@param old_line number|nil +---@param new_line number|nil +---@param hunks Hunk[] +---@param all_diff_output table +---@return string|nil +local function get_modification_type_from_old_sha(old_line, new_line, hunks, all_diff_output) + if old_line == nil then + return nil + end + + return List.new(hunks):find(function(hunk) + local old_line_end = hunk.old_line + hunk.old_range + local new_line_end = hunk.new_line + hunk.new_range + local in_old_range = old_line >= hunk.old_line and old_line <= old_line_end + local in_new_range = old_line >= hunk.new_line and new_line <= new_line_end + return (in_old_range or in_new_range) and line_was_removed(old_line, hunk, all_diff_output) + end) and "deleted" or "unmodified" +end + ---Returns whether the comment is on a deleted line, added line, or unmodified line. ---This is in order to build the payload for Gitlab correctly by setting the old line and new line. ---@param old_line number|nil ---@param new_line number|nil ---@param current_file string +---@param is_current_sha boolean ---@return string|nil -function M.get_modification_type(old_line, new_line, current_file) +function M.get_modification_type(old_line, new_line, current_file, is_current_sha) local hunk_and_diff_data = parse_hunks_and_diff(current_file, state.INFO.target_branch) if hunk_and_diff_data.hunks == nil then return @@ -187,49 +227,8 @@ function M.get_modification_type(old_line, new_line, current_file) local hunks = hunk_and_diff_data.hunks local all_diff_output = hunk_and_diff_data.all_diff_output - - local is_current_sha = require("gitlab.reviewer").is_current_sha() - - for _, hunk in ipairs(hunks) do - local old_line_end = hunk.old_line + hunk.old_range - local new_line_end = hunk.new_line + hunk.new_range - - if is_current_sha then - -- If it is a single line change and neither hunk has a range, then it's added - if new_line >= hunk.new_line and new_line <= new_line_end then - if hunk.new_range == 0 and hunk.old_range == 0 then - return "added" - end - -- If leaving a comment on the new window, we may be commenting on an added line - -- or on an unmodified line. To tell, we have to check whether the line itself is - -- prefixed with "+" and only return "added" if it is. - if line_was_added(new_line, hunk, all_diff_output) then - return "added" - end - end - else - -- It's a deletion if it's in the range of the hunks and the new - -- range is zero, since that is only a deletion hunk, or if we find - -- a match in another hunk with a range, and the corresponding line is prefixed - -- with a "-" only. If it is, then it's a deletion. - if old_line >= hunk.old_line and old_line <= old_line_end and hunk.old_range == 0 then - return "deleted" - end - if - (old_line >= hunk.old_line and old_line <= old_line_end) - or (old_line >= hunk.new_line and new_line <= new_line_end) - then - if line_was_removed(old_line, hunk, all_diff_output) then - return "deleted" - end - end - end - end - - -- If we can't find the line, this means the user is either trying to leave - -- a comment on an unchanged line in the new or old file SHA. This is only - -- allowed in the old file - return is_current_sha and "bad_file_unmodified" or "unmodified" + return is_current_sha and get_modification_type_from_new_sha(new_line, hunks, all_diff_output) + or get_modification_type_from_old_sha(old_line, new_line, hunks, all_diff_output) end ---Returns the matching line number of a line in the new/old version of the file compared to the current SHA. diff --git a/lua/gitlab/reviewer/init.lua b/lua/gitlab/reviewer/init.lua index a04727fb..1745d121 100644 --- a/lua/gitlab/reviewer/init.lua +++ b/lua/gitlab/reviewer/init.lua @@ -167,9 +167,11 @@ M.get_reviewer_data = function() local new_line = vim.api.nvim_win_get_cursor(new_win)[1] local old_line = vim.api.nvim_win_get_cursor(old_win)[1] - local modification_type = hunks.get_modification_type(old_line, new_line, current_file) + + local is_current_sha = M.is_current_sha() + local modification_type = hunks.get_modification_type(old_line, new_line, current_file, is_current_sha) if modification_type == nil then - u.notify("Error parsing hunks for modification type", vim.log.levels.ERROR) + u.notify("Error getting modification type", vim.log.levels.ERROR) return end @@ -177,8 +179,8 @@ M.get_reviewer_data = function() u.notify("Comments on unmodified lines will be placed in the old file", vim.log.levels.WARN) end - local current_bufnr = M.is_current_sha() and layout.b.file.bufnr or layout.a.file.bufnr - local opposite_bufnr = M.is_current_sha() and layout.a.file.bufnr or layout.b.file.bufnr + local current_bufnr = is_current_sha and layout.b.file.bufnr or layout.a.file.bufnr + local opposite_bufnr = is_current_sha and layout.a.file.bufnr or layout.b.file.bufnr local old_sha_win_id = u.get_window_id_by_buffer_id(layout.a.file.bufnr) local new_sha_win_id = u.get_window_id_by_buffer_id(layout.b.file.bufnr) diff --git a/lua/gitlab/reviewer/location.lua b/lua/gitlab/reviewer/location.lua index 782a68e8..d84eeacb 100755 --- a/lua/gitlab/reviewer/location.lua +++ b/lua/gitlab/reviewer/location.lua @@ -141,7 +141,8 @@ function Location:set_start_range(visual_range) end local reviewer = require("gitlab.reviewer") - local win_id = reviewer.is_current_sha() and self.reviewer_data.new_sha_win_id or self.reviewer_data.old_sha_win_id + local is_current_sha = reviewer.is_current_sha() + local win_id = is_current_sha and self.reviewer_data.new_sha_win_id or self.reviewer_data.old_sha_win_id if win_id == nil then u.notify("Error getting window number of SHA for start range", vim.log.levels.ERROR) return @@ -163,7 +164,11 @@ function Location:set_start_range(visual_range) return end - local modification_type = hunks.get_modification_type(old_line, new_line, current_file) + local modification_type = hunks.get_modification_type(old_line, new_line, current_file, is_current_sha) + if modification_type == nil then + u.notify("Error getting modification type for start of range", vim.log.levels.ERROR) + return + end self.location_data.line_range.start = { new_line = modification_type ~= "deleted" and new_line or nil, @@ -199,7 +204,14 @@ function Location:set_end_range(visual_range) return end - local modification_type = hunks.get_modification_type(old_line, new_line, current_file) + local reviewer = require("gitlab.reviewer") + local is_current_sha = reviewer.is_current_sha() + local modification_type = hunks.get_modification_type(old_line, new_line, current_file, is_current_sha) + if modification_type == nil then + u.notify("Error getting modification type for end of range", vim.log.levels.ERROR) + return + end + self.location_data.line_range["end"] = { new_line = modification_type ~= "deleted" and new_line or nil, old_line = modification_type ~= "added" and old_line or nil, From 2010c247a327cf9f16c9d1ee1465b17a2597a124 Mon Sep 17 00:00:00 2001 From: "Harrison (Harry) Cramer" <32515581+harrisoncramer@users.noreply.github.com> Date: Sat, 2 Mar 2024 15:43:16 -0500 Subject: [PATCH 11/16] Fix: Rebuild Diagnostics + Signs (#203) * Updated lua/gitlab/actions/discussions/signs.lua * Initial commit of changes * Updated lua/gitlab/actions/discussions/signs.lua * Updated lua/gitlab/actions/discussions/signs.lua * Updated lua/gitlab/actions/discussions/init.lua * Updated lua/gitlab/actions/discussions/signs.lua * Split up signs and diagnostics * Moved common to common file * Continued modularization * added formatting * Moved signs/diagnostic lines into appropriate files * Updated lua/gitlab/reviewer/init.lua * Renamed is_current_sha => is_current_sha_focused * Updated lua/gitlab/reviewer/init.lua * Moved actions.indicators => indicators * Split discussions/signs apart into separate modules * Fixed filter function, fixed discussions * Updated lua/gitlab/indicators/diagnostics.lua * Setting diagnostics, handling and catching errors * Greatly simplified diagnostics setup * Re-split multi-line * Updated lua/gitlab/indicators/diagnostics.lua * Setting up signs * Updated lua/gitlab/indicators/signs.lua * Updated signs + display options * Updated lua/gitlab/state.lua * Updated doc/gitlab.nvim.txt * Updated readme * Added deprecation warning * formatting --- README.md | 37 +-- doc/gitlab.nvim.txt | 82 ++---- lua/gitlab/actions/discussions/init.lua | 43 ++- lua/gitlab/actions/discussions/signs.lua | 336 ----------------------- lua/gitlab/actions/discussions/tree.lua | 4 +- lua/gitlab/hunks/init.lua | 6 +- lua/gitlab/indicators/common.lua | 73 +++++ lua/gitlab/indicators/diagnostics.lua | 152 ++++++++++ lua/gitlab/indicators/signs.lua | 86 ++++++ lua/gitlab/reviewer/init.lua | 114 +++----- lua/gitlab/reviewer/location.lua | 21 +- lua/gitlab/state.lua | 35 +-- lua/gitlab/utils/init.lua | 15 + lua/gitlab/utils/list.lua | 18 +- script.lua | 12 + 15 files changed, 455 insertions(+), 579 deletions(-) delete mode 100644 lua/gitlab/actions/discussions/signs.lua create mode 100644 lua/gitlab/indicators/common.lua create mode 100644 lua/gitlab/indicators/diagnostics.lua create mode 100644 lua/gitlab/indicators/signs.lua create mode 100644 script.lua diff --git a/README.md b/README.md index efafc835..c80326df 100644 --- a/README.md +++ b/README.md @@ -170,35 +170,16 @@ require("gitlab").setup({ "pipeline", }, }, - discussion_sign_and_diagnostic = { + discussion_signs = { + enabled = true, -- Show diagnostics for gitlab comments in the reviewer skip_resolved_discussion = false, - skip_old_revision_discussion = true, - }, - discussion_sign = { - -- See :h sign_define for details about sign configuration. - enabled = true, - text = "💬", - linehl = nil, - texthl = nil, - culhl = nil, - numhl = nil, - priority = 20, -- Priority of sign, the lower the number the higher the priority - helper_signs = { - -- For multiline comments the helper signs are used to indicate the whole context - -- Priority of helper signs is lower than the main sign (-1). - enabled = true, - start = "↑", - mid = "|", - ["end"] = "↓", - }, - }, - discussion_diagnostic = { - -- If you want to customize diagnostics for discussions you can make special config - -- for namespace `gitlab_discussion`. See :h vim.diagnostic.config - enabled = true, severity = vim.diagnostic.severity.INFO, - code = nil, -- see :h diagnostic-structure - display_opts = {}, -- see opts in vim.diagnostic.set + virtual_text = false, + icons = { + comment = "→|", + range = " |", + }, + skip_old_revision_discussion = false, }, pipeline = { created = "", @@ -231,7 +212,7 @@ require("gitlab").setup({ directory = "Directory", directory_icon = "DiffviewFolderSign", file_name = "Normal", - } + } } }) ``` diff --git a/doc/gitlab.nvim.txt b/doc/gitlab.nvim.txt index 65e6e10d..817f080e 100644 --- a/doc/gitlab.nvim.txt +++ b/doc/gitlab.nvim.txt @@ -204,35 +204,16 @@ you call this function with no values the defaults will be used: "labels", }, }, - discussion_sign_and_diagnostic = { - skip_resolved_discussion = false, - skip_old_revision_discussion = true, - }, - discussion_sign = { - -- See :h sign_define for details about sign configuration. - enabled = true, - text = "💬", - linehl = nil, - texthl = nil, - culhl = nil, - numhl = nil, - priority = 20, -- Priority of sign, the lower the number the higher the priority - helper_signs = { - -- For multiline comments the helper signs are used to indicate the whole context - -- Priority of helper signs is lower than the main sign (-1). - enabled = true, - start = "↑", - mid = "|", - ["end"] = "↓", - }, - }, - discussion_diagnostic = { - -- If you want to customize diagnostics for discussions you can make special config - -- for namespace `gitlab_discussion`. See :h vim.diagnostic.config - enabled = true, - severity = vim.diagnostic.severity.INFO, - code = nil, -- see :h diagnostic-structure - display_opts = {}, -- see opts in vim.diagnostic.set + discussion_signs = { + enabled = true, + skip_resolved_discussion = false, + skip_old_revision_discussion = false, + severity = vim.diagnostic.severity.INFO, + virtual_text = false, + icons = { + comment = "→|", + range = " |", + }, }, pipeline = { created = "", @@ -386,46 +367,17 @@ These labels will be visible in the summary panel, as long as you provide the SIGNS AND DIAGNOSTICS *gitlab.nvim.signs-and-diagnostics* -By default when reviewing files you will see signs and diagnostics (if enabled -in configuration). When cursor is on diagnostic line you can view discussion -thread by using `vim.diagnostic.show`. You can also jump to discussion tree -where you can reply, edit or delete discussion. +By default when reviewing files you will see diagnostics in the reviewer +for comments that have been added to a review. When the cursor is on +diagnostic line you can view discussion thread by using `vim.diagnostic.show`. +You can also jump to discussion tree for the given comment: >lua require("gitlab").move_to_discussion_tree_from_diagnostic() -< -The `discussion_sign` configuration controls the display of signs for -discussions in the reviewer pane. This allows users to jump to comments in the -current buffer in the reviewer pane directly. Keep in mind that the highlights -provided here can be overridden by other highlights (for example from -`diffview.nvim`). - -These diagnostics are configurable in the same way that diagnostics are -typically configurable in Neovim. For instance, the `severity` key sets the -diagnostic severity level and should be set to one of -`vim.diagnostic.severity.ERROR`, `vim.diagnostic.severity.WARN`, -`vim.diagnostic.severity.INFO`, or `vim.diagnostic.severity.HINT`. The -`display_opts` option configures the diagnostic display options (this is -directly used as opts in vim.diagnostic.set). Here you can configure values -like: - -- `virtual_text` - Show virtual text for diagnostics. -- `underline` - Underline text for diagnostics. - -Diagnostics for discussions use the `gitlab_discussion` namespace. See -|vim.diagnostic.config| and |diagnostic-structure| for more details. Signs and -diagnostics have common settings in `discussion_sign_and_diagnostic`. This -allows customizing if discussions that are resolved or no longer relevant -should still display visual indicators in the editor. The -`skip_resolved_discussion` Boolean will control visibility of resolved -discussions, and `skip_old_revision_discussion` whether to show signs and -diagnostics for discussions on outdated diff revisions. - -When interacting with multiline comments, the cursor must be on the "main" line -of diagnostic, where the `discussion_sign.text` is shown, otherwise -`vim.diagnostic.show` and `move_to_discussion_tree_from_diagnostic` will not -work. +You may skip resolved discussions by toggling `discussion_signs.skip_resolved_discussion` +in your setup function to true. By default, discussions from this plugin +are shown at the INFO severity level (see :h vim.diagnostic.severity). EMOJIS *gitlab.nvim.emojis* diff --git a/lua/gitlab/actions/discussions/init.lua b/lua/gitlab/actions/discussions/init.lua index 6d614c61..dde14832 100644 --- a/lua/gitlab/actions/discussions/init.lua +++ b/lua/gitlab/actions/discussions/init.lua @@ -13,7 +13,8 @@ local List = require("gitlab.utils.list") local miscellaneous = require("gitlab.actions.miscellaneous") local discussions_tree = require("gitlab.actions.discussions.tree") local diffview_lib = require("diffview.lib") -local signs = require("gitlab.actions.discussions.signs") +local signs = require("gitlab.indicators.signs") +local diagnostics = require("gitlab.indicators.diagnostics") local winbar = require("gitlab.actions.discussions.winbar") local help = require("gitlab.actions.help") local emoji = require("gitlab.emoji") @@ -63,7 +64,8 @@ M.initialize_discussions = function() M.modifiable(false) end) reviewer.set_callback_for_reviewer_leave(function() - signs.clear_signs_and_diagnostics() + signs.clear_signs() + diagnostics.clear_diagnostics() M.modifiable(true) end) end @@ -92,11 +94,8 @@ end --- Take existing data and refresh the diagnostics, the winbar, and the signs M.refresh_view = function() - if state.settings.discussion_sign.enabled then - signs.refresh_signs(M.discussions) - end - if state.settings.discussion_diagnostic.enabled then - signs.refresh_diagnostics(M.discussions) + if state.settings.discussion_signs.enabled then + diagnostics.refresh_diagnostics(M.discussions) end if M.split_visible then local linked_is_focused = M.linked_bufnr == M.focused_bufnr @@ -168,7 +167,7 @@ M.toggle = function(callback) M.focused_bufnr = default_buffer M.switch_can_edit_bufs(false) - winbar.update_winbar(M.discussions, M.unlinked_discussions, default_discussions and "Discussions" or "Notes") + M.refresh_view() vim.api.nvim_set_current_win(current_window) if type(callback) == "function" then @@ -198,7 +197,7 @@ end ---Move to the discussion tree at the discussion from diagnostic on current line. M.move_to_discussion_tree = function() local current_line = vim.api.nvim_win_get_cursor(0)[1] - local diagnostics = vim.diagnostic.get(0, { namespace = signs.diagnostics_namespace, lnum = current_line - 1 }) + local d = vim.diagnostic.get(0, { namespace = diagnostics.diagnostics_namespace, lnum = current_line - 1 }) ---Function used to jump to the discussion tree after the menu selection. local jump_after_menu_selection = function(diagnostic) @@ -229,11 +228,11 @@ M.move_to_discussion_tree = function() end end - if #diagnostics == 0 then + if #d == 0 then u.notify("No diagnostics for this line", vim.log.levels.WARN) return - elseif #diagnostics > 1 then - vim.ui.select(diagnostics, { + elseif #d > 1 then + vim.ui.select(d, { prompt = "Choose discussion to jump to", format_item = function(diagnostic) return diagnostic.message @@ -245,7 +244,7 @@ M.move_to_discussion_tree = function() jump_after_menu_selection(diagnostic) end) else - jump_after_menu_selection(diagnostics[1]) + jump_after_menu_selection(d[1]) end end @@ -389,7 +388,7 @@ end -- This function (settings.discussion_tree.jump_to_reviewer) will jump the cursor to the reviewer's location associated with the note. The implementation depends on the reviewer M.jump_to_reviewer = function(tree) - local file_name, new_line, old_line, is_undefined_type, error = M.get_note_location(tree) + local file_name, new_line, old_line, error = M.get_note_location(tree) if error ~= nil then u.notify(error, vim.log.levels.ERROR) return @@ -403,13 +402,13 @@ M.jump_to_reviewer = function(tree) return end - reviewer.jump(file_name, new_line_int, old_line_int, { is_undefined_type = is_undefined_type }) + reviewer.jump(file_name, new_line_int, old_line_int) M.refresh_view() end -- This function (settings.discussion_tree.jump_to_file) will jump to the file changed in a new tab M.jump_to_file = function(tree) - local file_name, new_line, old_line, _, error = M.get_note_location(tree) + local file_name, new_line, old_line, error = M.get_note_location(tree) if error ~= nil then u.notify(error, vim.log.levels.ERROR) return @@ -911,21 +910,17 @@ end ---Get note location ---@param tree NuiTree ----@return string, string, string, boolean, string? +---@return string, string, string, string? M.get_note_location = function(tree) local node = tree:get_node() if node == nil then - return "", "", "", false, "Could not get node" + return "", "", "", "Could not get node" end local discussion_node = M.get_root_node(tree, node) if discussion_node == nil then - return "", "", "", false, "Could not get discussion node" + return "", "", "", "Could not get discussion node" end - return discussion_node.file_name, - discussion_node.new_line, - discussion_node.old_line, - discussion_node.undefined_type or false, - nil + return discussion_node.file_name, discussion_node.new_line, discussion_node.old_line, nil end ---@param tree NuiTree diff --git a/lua/gitlab/actions/discussions/signs.lua b/lua/gitlab/actions/discussions/signs.lua deleted file mode 100644 index e1c5747b..00000000 --- a/lua/gitlab/actions/discussions/signs.lua +++ /dev/null @@ -1,336 +0,0 @@ -local state = require("gitlab.state") -local u = require("gitlab.utils") -local reviewer = require("gitlab.reviewer") -local discussion_sign_name = "gitlab_discussion" -local discussion_helper_sign_start = "gitlab_discussion_helper_start" -local discussion_helper_sign_mid = "gitlab_discussion_helper_mid" -local discussion_helper_sign_end = "gitlab_discussion_helper_end" -local diagnostics_namespace = vim.api.nvim_create_namespace(discussion_sign_name) - -local M = {} -M.diagnostics_namespace = diagnostics_namespace - ----Clear all signs and diagnostics -M.clear_signs_and_diagnostics = function() - vim.fn.sign_unplace(discussion_sign_name) - vim.diagnostic.reset(diagnostics_namespace) -end - ----Refresh the discussion signs for currently loaded file in reviewer For convinience we use same ----string for sign name and sign group ( currently there is only one sign needed) ----@param discussions Discussion[] -M.refresh_signs = function(discussions) - local filtered_discussions = M.filter_discussions_for_signs_and_diagnostics(discussions) - if filtered_discussions == nil then - vim.diagnostic.reset(diagnostics_namespace) - return - end - - local new_signs, old_signs, error = M.parse_signs_from_discussions(filtered_discussions) - if error ~= nil then - vim.notify(error, vim.log.levels.ERROR) - return - end - - vim.fn.sign_unplace(discussion_sign_name) - reviewer.place_sign(old_signs, "old") - reviewer.place_sign(new_signs, "new") -end - ----Refresh the diagnostics for the currently reviewed file ----@param discussions Discussion[] -M.refresh_diagnostics = function(discussions) - -- Keep in mind that diagnostic line numbers use 0-based indexing while line numbers use - -- 1-based indexing - local filtered_discussions = M.filter_discussions_for_signs_and_diagnostics(discussions) - if filtered_discussions == nil then - vim.diagnostic.reset(diagnostics_namespace) - return - end - - local new_diagnostics, old_diagnostics = M.parse_diagnostics_from_discussions(filtered_discussions) - - vim.diagnostic.reset(diagnostics_namespace) - reviewer.set_diagnostics( - diagnostics_namespace, - new_diagnostics, - "new", - state.settings.discussion_diagnostic.display_opts - ) - reviewer.set_diagnostics( - diagnostics_namespace, - old_diagnostics, - "old", - state.settings.discussion_diagnostic.display_opts - ) -end - ----Filter all discussions which are relevant for currently visible signs and diagnostscs. ----@return Discussion[]? -M.filter_discussions_for_signs_and_diagnostics = function(all_discussions) - if type(all_discussions) ~= "table" then - return - end - local file = reviewer.get_current_file() - if not file then - return - end - local discussions = {} - for _, discussion in ipairs(all_discussions) do - local first_note = discussion.notes[1] - if - type(first_note.position) == "table" - and (first_note.position.new_path == file or first_note.position.old_path == file) - then - if - --Skip resolved discussions - not ( - state.settings.discussion_sign_and_diagnostic.skip_resolved_discussion - and first_note.resolvable - and first_note.resolved - ) - --Skip discussions from old revisions - and not ( - state.settings.discussion_sign_and_diagnostic.skip_old_revision_discussion - and u.from_iso_format_date_to_timestamp(first_note.created_at) - <= u.from_iso_format_date_to_timestamp(state.MR_REVISIONS[1].created_at) - ) - then - table.insert(discussions, discussion) - end - end - end - return discussions -end - ----Define signs for discussions if not already defined -M.setup_signs = function() - local discussion_sign = state.settings.discussion_sign - local signs = { - [discussion_sign_name] = discussion_sign.text, - [discussion_helper_sign_start] = discussion_sign.helper_signs.start, - [discussion_helper_sign_mid] = discussion_sign.helper_signs.mid, - [discussion_helper_sign_end] = discussion_sign.helper_signs["end"], - } - for sign_name, sign_text in pairs(signs) do - if #vim.fn.sign_getdefined(sign_name) == 0 then - vim.fn.sign_define(sign_name, { - text = sign_text, - linehl = discussion_sign.linehl, - texthl = discussion_sign.texthl, - culhl = discussion_sign.culhl, - numhl = discussion_sign.numhl, - }) - end - end -end - ----Iterates over each discussion and returns a list of tables with sign ----data, for instance group, priority, line number etc. ----@param discussions Discussion[] ----@return DiagnosticTable[], DiagnosticTable[], string? -M.parse_diagnostics_from_discussions = function(discussions) - local new_diagnostics = {} - local old_diagnostics = {} - for _, discussion in ipairs(discussions) do - local first_note = discussion.notes[1] - local message = "" - for _, note in ipairs(discussion.notes) do - message = message .. M.build_note_header(note) .. "\n" .. note.body .. "\n" - end - - local diagnostic = { - message = message, - col = 0, - severity = state.settings.discussion_diagnostic.severity, - user_data = { discussion_id = discussion.id, header = M.build_note_header(discussion.notes[1]) }, - source = "gitlab", - code = state.settings.discussion_diagnostic.code, - } - - -- Diagnostics for line range discussions are tricky - you need to set lnum to be the - -- line number equal to note.position.new_line or note.position.old_line because that is the - -- only line where you can trigger the diagnostic to show. This also needs to be in sync - -- with the sign placement. - local line_range = first_note.position.line_range - if line_range ~= nil then - local start_old_line, start_new_line = M.parse_line_code(line_range.start.line_code) - local end_old_line, end_new_line = M.parse_line_code(line_range["end"].line_code) - - local start_type = line_range.start.type - if start_type == "new" then - local new_diagnostic - if first_note.position.new_line == start_new_line then - new_diagnostic = { - lnum = start_new_line - 1, - end_lnum = end_new_line - 1, - } - else - new_diagnostic = { - lnum = end_new_line - 1, - end_lnum = start_new_line - 1, - } - end - new_diagnostic = vim.tbl_deep_extend("force", new_diagnostic, diagnostic) - table.insert(new_diagnostics, new_diagnostic) - elseif start_type == "old" or start_type == "expanded" or start_type == "" then - local old_diagnostic - if first_note.position.old_line == start_old_line then - old_diagnostic = { - lnum = start_old_line - 1, - end_lnum = end_old_line - 1, - } - else - old_diagnostic = { - lnum = end_old_line - 1, - end_lnum = start_old_line - 1, - } - end - old_diagnostic = vim.tbl_deep_extend("force", old_diagnostic, diagnostic) - table.insert(old_diagnostics, old_diagnostic) - else -- Comments on expanded, non-changed lines - return {}, {}, string.format("Unsupported line range type found for discussion %s", discussion.id) - end - else -- Diagnostics for single line discussions. - if first_note.position.new_line ~= nil and first_note.position.old_line == nil then - local new_diagnostic = { - lnum = first_note.position.new_line - 1, - } - new_diagnostic = vim.tbl_deep_extend("force", new_diagnostic, diagnostic) - table.insert(new_diagnostics, new_diagnostic) - end - if first_note.position.old_line ~= nil then - local old_diagnostic = { - lnum = first_note.position.old_line - 1, - } - old_diagnostic = vim.tbl_deep_extend("force", old_diagnostic, diagnostic) - table.insert(old_diagnostics, old_diagnostic) - end - end - end - - return new_diagnostics, old_diagnostics -end - -local base_sign = { - name = discussion_sign_name, - group = discussion_sign_name, - priority = state.settings.discussion_sign.priority, - buffer = nil, -} -local base_helper_sign = { - name = discussion_sign_name, - group = discussion_sign_name, - priority = state.settings.discussion_sign.priority - 1, - buffer = nil, -} - ----Iterates over each discussion and returns a list of tables with sign ----data, for instance group, priority, line number etc. ----@param discussions Discussion[] ----@return SignTable[], SignTable[], string? -M.parse_signs_from_discussions = function(discussions) - local new_signs = {} - local old_signs = {} - for _, discussion in ipairs(discussions) do - local first_note = discussion.notes[1] - local line_range = first_note.position.line_range - - -- We have a line range which means we either have a multi-line comment or a comment - -- on a line in an "expanded" part of a file - if line_range ~= nil then - local start_old_line, start_new_line = M.parse_line_code(line_range.start.line_code) - local end_old_line, end_new_line = M.parse_line_code(line_range["end"].line_code) - local discussion_line, start_line, end_line - - local start_type = line_range.start.type - if start_type == "new" then - table.insert( - new_signs, - vim.tbl_deep_extend("force", { - id = first_note.id, - lnum = first_note.position.new_line, - }, base_sign) - ) - discussion_line = first_note.position.new_line - start_line = start_new_line - end_line = end_new_line - elseif start_type == "old" or start_type == "expanded" or start_type == "" then - table.insert( - old_signs, - vim.tbl_deep_extend("force", { - id = first_note.id, - lnum = first_note.position.old_line, - }, base_sign) - ) - discussion_line = first_note.position.old_line - start_line = start_old_line - end_line = end_old_line - else - return {}, {}, string.format("Unsupported line range type found for discussion %s", discussion.id) - end - - -- Helper signs does not have specific ids currently. - if state.settings.discussion_sign.helper_signs.enabled then - local helper_signs = {} - if start_line > end_line then - start_line, end_line = end_line, start_line - end - for i = start_line, end_line do - if i ~= discussion_line then - local sign_name - if i == start_line then - sign_name = discussion_helper_sign_start - elseif i == end_line then - sign_name = discussion_helper_sign_end - else - sign_name = discussion_helper_sign_mid - end - table.insert( - helper_signs, - vim.tbl_deep_extend("keep", { - name = sign_name, - lnum = i, - }, base_helper_sign) - ) - end - end - if start_type == "new" then - vim.list_extend(new_signs, helper_signs) - elseif start_type == "old" or start_type == "expanded" or start_type == "" then - vim.list_extend(old_signs, helper_signs) - end - end - else -- The note is a normal comment, not a range comment - local sign = vim.tbl_deep_extend("force", { - id = first_note.id, - }, base_sign) - if first_note.position.new_line ~= nil and first_note.position.old_line == nil then - table.insert(new_signs, vim.tbl_deep_extend("force", { lnum = first_note.position.new_line }, sign)) - end - if first_note.position.old_line ~= nil then - table.insert(old_signs, vim.tbl_deep_extend("force", { lnum = first_note.position.old_line }, sign)) - end - end - end - - return new_signs, old_signs, nil -end - ----Parse line code and return old and new line numbers ----@param line_code string gitlab line code -> 588440f66559714280628a4f9799f0c4eb880a4a_10_10 ----@return number? -M.parse_line_code = function(line_code) - local line_code_regex = "%w+_(%d+)_(%d+)" - local old_line, new_line = line_code:match(line_code_regex) - return tonumber(old_line), tonumber(new_line) -end - ----Build note header from note. ----@param note Note ----@return string -M.build_note_header = function(note) - return "@" .. note.author.username .. " " .. u.time_since(note.created_at) -end - -return M diff --git a/lua/gitlab/actions/discussions/tree.lua b/lua/gitlab/actions/discussions/tree.lua index e2ded22a..d52ed1a0 100644 --- a/lua/gitlab/actions/discussions/tree.lua +++ b/lua/gitlab/actions/discussions/tree.lua @@ -81,7 +81,7 @@ end ---Build note header from note. ---@param note Note ---@return string -local function build_note_header(note) +M.build_note_header = function(note) return "@" .. note.author.username .. " " .. u.time_since(note.created_at) end @@ -112,7 +112,7 @@ local function build_note_body(note, resolve_info) or state.settings.discussion_tree.unresolved end - local noteHeader = build_note_header(note) .. " " .. resolve_symbol + local noteHeader = M.build_note_header(note) .. " " .. resolve_symbol return noteHeader, text_nodes end diff --git a/lua/gitlab/hunks/init.lua b/lua/gitlab/hunks/init.lua index 1f17f5b4..06e9fbcf 100644 --- a/lua/gitlab/hunks/init.lua +++ b/lua/gitlab/hunks/init.lua @@ -217,9 +217,9 @@ end ---@param old_line number|nil ---@param new_line number|nil ---@param current_file string ----@param is_current_sha boolean +---@param is_current_sha_focused boolean ---@return string|nil -function M.get_modification_type(old_line, new_line, current_file, is_current_sha) +function M.get_modification_type(old_line, new_line, current_file, is_current_sha_focused) local hunk_and_diff_data = parse_hunks_and_diff(current_file, state.INFO.target_branch) if hunk_and_diff_data.hunks == nil then return @@ -227,7 +227,7 @@ function M.get_modification_type(old_line, new_line, current_file, is_current_sh local hunks = hunk_and_diff_data.hunks local all_diff_output = hunk_and_diff_data.all_diff_output - return is_current_sha and get_modification_type_from_new_sha(new_line, hunks, all_diff_output) + return is_current_sha_focused and get_modification_type_from_new_sha(new_line, hunks, all_diff_output) or get_modification_type_from_old_sha(old_line, new_line, hunks, all_diff_output) end diff --git a/lua/gitlab/indicators/common.lua b/lua/gitlab/indicators/common.lua new file mode 100644 index 00000000..b09a66bb --- /dev/null +++ b/lua/gitlab/indicators/common.lua @@ -0,0 +1,73 @@ +local u = require("gitlab.utils") +local state = require("gitlab.state") +local reviewer = require("gitlab.reviewer") +local List = require("gitlab.utils.list") + +local M = {} + +---Filter all discussions which are relevant for currently visible signs and diagnostics. +---@return Discussion[] +M.filter_placeable_discussions = function(all_discussions) + if type(all_discussions) ~= "table" then + return {} + end + local file = reviewer.get_current_file() + if not file then + return {} + end + return List.new(all_discussions):filter(function(discussion) + local first_note = discussion.notes[1] + return type(first_note.position) == "table" + --Do not include unlinked notes + and (first_note.position.new_path == file or first_note.position.old_path == file) + --Skip resolved discussions if user wants to + and not (state.settings.discussion_signs.skip_resolved_discussion and first_note.resolvable and first_note.resolved) + --Skip discussions from old revisions + and not ( + state.settings.discussion_signs.skip_old_revision_discussion + and u.from_iso_format_date_to_timestamp(first_note.created_at) + <= u.from_iso_format_date_to_timestamp(state.MR_REVISIONS[1].created_at) + ) + end) +end + +M.parse_line_code = function(line_code) + local line_code_regex = "%w+_(%d+)_(%d+)" + local old_line, new_line = line_code:match(line_code_regex) + return tonumber(old_line), tonumber(new_line) +end + +---@param discussion Discussion +---@return boolean +M.is_old_sha = function(discussion) + local first_note = discussion.notes[1] + return first_note.position.old_line ~= nil +end + +---@param discussion Discussion +---@return boolean +M.is_new_sha = function(discussion) + return not M.is_old_sha(discussion) +end + +---@param discussion Discussion +---@return boolean +M.is_single_line = function(discussion) + local first_note = discussion.notes[1] + local line_range = first_note.position.line_range + return line_range == nil +end + +---@param discussion Discussion +---@return boolean +M.is_multi_line = function(discussion) + return not M.is_single_line(discussion) +end + +---@param discussion Discussion +---@return Note +M.get_first_note = function(discussion) + return discussion.notes[1] +end + +return M diff --git a/lua/gitlab/indicators/diagnostics.lua b/lua/gitlab/indicators/diagnostics.lua new file mode 100644 index 00000000..13eeba14 --- /dev/null +++ b/lua/gitlab/indicators/diagnostics.lua @@ -0,0 +1,152 @@ +local u = require("gitlab.utils") +local diffview_lib = require("diffview.lib") +local discussion_tree = require("gitlab.actions.discussions.tree") +local common = require("gitlab.indicators.common") +local List = require("gitlab.utils.list") +local state = require("gitlab.state") +local discussion_sign_name = "gitlab_discussion" + +local M = {} +local diagnostics_namespace = vim.api.nvim_create_namespace(discussion_sign_name) +M.diagnostics_namespace = diagnostics_namespace +M.discussion_sign_name = discussion_sign_name +M.clear_diagnostics = function() + vim.diagnostic.reset(diagnostics_namespace) +end + +-- Display options for the diagnostic +local display_opts = { + virtual_text = state.settings.discussion_signs.virtual_text, + severity_sort = true, + underline = false, +} + +---Takes some range information and data about a discussion +---and creates a diagnostic to be placed in the reviewer +---@param range_info table +---@param discussion Discussion +---@return Diagnostic +local function create_diagnostic(range_info, discussion) + local message = "" + for _, note in ipairs(discussion.notes) do + message = message .. discussion_tree.build_note_header(note) .. "\n" .. note.body .. "\n" + end + + local diagnostic = { + message = message, + col = 0, + severity = state.settings.discussion_signs.severity, + user_data = { discussion_id = discussion.id, header = discussion_tree.build_note_header(discussion.notes[1]) }, + source = "gitlab", + code = "gitlab.nvim", + } + return vim.tbl_deep_extend("force", diagnostic, range_info) +end + +---Creates a single line diagnostic +---@param discussion Discussion +---@return Diagnostic +local create_single_line_diagnostic = function(discussion) + local first_note = discussion.notes[1] + return create_diagnostic({ + lnum = first_note.position.new_line - 1, + }, discussion) +end + +---Creates a mutli-line line diagnostic +---@param discussion Discussion +---@return Diagnostic +local create_multiline_diagnostic = function(discussion) + local first_note = discussion.notes[1] + local line_range = first_note.position.line_range + if line_range == nil then + error("Parsing multi-line comment but note does not contain line range") + end + + local start_old_line, start_new_line = common.parse_line_code(line_range.start.line_code) + + if common.is_new_sha(discussion) then + return create_diagnostic({ + lnum = start_new_line - 1, + end_lnum = first_note.position.new_line - 1, + }, discussion) + else + return create_diagnostic({ + lnum = start_old_line - 1, + end_lnum = first_note.position.old_line - 1, + }, discussion) + end +end + +---Set diagnostics in currently new SHA. +---@param namespace number namespace for diagnostics +---@param diagnostics table see :h vim.diagnostic.set +---@param opts table? see :h vim.diagnostic.set +local set_diagnostics_in_new_sha = function(namespace, diagnostics, opts) + local view = diffview_lib.get_current_view() + if not view then + return + end + vim.diagnostic.set(namespace, view.cur_layout.b.file.bufnr, diagnostics, opts) + require("gitlab.indicators.signs").set_signs(diagnostics, view.cur_layout.b.file.bufnr) +end + +---Set diagnostics in old SHA. +---@param namespace number namespace for diagnostics +---@param diagnostics table see :h vim.diagnostic.set +---@param opts table? see :h vim.diagnostic.set +local set_diagnostics_in_old_sha = function(namespace, diagnostics, opts) + local view = diffview_lib.get_current_view() + if not view then + return + end + vim.diagnostic.set(namespace, view.cur_layout.a.file.bufnr, diagnostics, opts) + require("gitlab.indicators.signs").set_signs(diagnostics, view.cur_layout.a.file.bufnr) +end + +---Refresh the diagnostics for the currently reviewed file +---@param discussions Discussion[] +M.refresh_diagnostics = function(discussions) + local ok, err = pcall(function() + require("gitlab.indicators.signs").clear_signs() + M.clear_diagnostics() + local filtered_discussions = common.filter_placeable_discussions(discussions) + if filtered_discussions == nil then + return + end + + local new_diagnostics = M.parse_new_diagnostics(filtered_discussions) + set_diagnostics_in_new_sha(diagnostics_namespace, new_diagnostics, display_opts) + + local old_diagnostics = M.parse_old_diagnostics(filtered_discussions) + set_diagnostics_in_old_sha(diagnostics_namespace, old_diagnostics, display_opts) + end) + + if not ok then + u.notify(string.format("Error setting diagnostics: %s", err), vim.log.levels.ERROR) + end +end + +---Iterates over each discussion and returns a list of tables with sign +---data, for instance group, priority, line number etc for the new SHA +---@param discussions Discussion[] +---@return DiagnosticTable[] +M.parse_new_diagnostics = function(discussions) + local new_diagnostics = List.new(discussions):filter(common.is_new_sha) + local single_line = new_diagnostics:filter(common.is_single_line):map(create_single_line_diagnostic) + local multi_line = new_diagnostics:filter(common.is_multi_line):map(create_multiline_diagnostic) + return u.combine(single_line, multi_line) +end + +---Iterates over each discussion and returns a list of tables with sign +---data, for instance group, priority, line number etc for the old SHA +---@param discussions Discussion[] +---@return DiagnosticTable[] +M.parse_old_diagnostics = function(discussions) + local old_diagnostics = List.new(discussions):filter(common.is_old_sha) + local single_line = old_diagnostics:filter(common.is_single_line):map(create_single_line_diagnostic) + local multi_line = old_diagnostics:filter(common.is_multi_line):map(create_multiline_diagnostic) + return u.combine(single_line, multi_line) +end + +return M diff --git a/lua/gitlab/indicators/signs.lua b/lua/gitlab/indicators/signs.lua new file mode 100644 index 00000000..9b94a4e4 --- /dev/null +++ b/lua/gitlab/indicators/signs.lua @@ -0,0 +1,86 @@ +local state = require("gitlab.state") +local discussion_sign_name = require("gitlab.indicators.diagnostics").discussion_sign_name +local namespace = require("gitlab.indicators.diagnostics").diagnostics_namespace + +local M = {} +M.clear_signs = function() + vim.fn.sign_unplace(discussion_sign_name) +end + +local gitlab_comment = "GitlabComment" +local gitlab_range = "GitlabRange" + +local severity_map = { + "Error", + "Warn", + "Info", + "Hint", +} + +---Refresh the discussion signs for currently loaded file in reviewer For convinience we use same +---string for sign name and sign group ( currently there is only one sign needed) +---@param diagnostics Diagnostic[] +---@param bufnr number +M.set_signs = function(diagnostics, bufnr) + if not state.settings.discussion_sign.enabled then + return + end + + -- Filter diagnostics from the 'gitlab' source and apply custom signs + for _, diagnostic in ipairs(diagnostics) do + local sign_id = string.format("%s__%d", namespace, diagnostic.lnum) + + if diagnostic.end_lnum then + local linenr = diagnostic.lnum + 1 + while linenr < diagnostic.end_lnum do + linenr = linenr + 1 + vim.fn.sign_place( + sign_id, + discussion_sign_name, + "DiagnosticSign" .. M.severity .. gitlab_range, + bufnr, + { lnum = linenr, priority = 999998 } + ) + end + end + + vim.fn.sign_place( + sign_id, + discussion_sign_name, + "DiagnosticSign" .. M.severity .. gitlab_comment, + bufnr, + { lnum = diagnostic.lnum + 1, priority = 999999 } + ) + + -- TODO: Detect whether diagnostic is ranged and set helper signs + end +end + +---Define signs for discussions +M.setup_signs = function() + local discussion_sign_settings = state.settings.discussion_signs + local comment_icon = discussion_sign_settings.icons.comment + local range_icon = discussion_sign_settings.icons.range + M.severity = severity_map[state.settings.discussion_signs.severity] + local signs = { "Error", "Warn", "Hint", "Info" } + for _, type in ipairs(signs) do + -- Define comment highlight group + local hl = "DiagnosticSign" .. type + local comment_hl = hl .. gitlab_comment + vim.fn.sign_define(comment_hl, { + text = comment_icon, + texthl = comment_hl, + }) + vim.cmd(string.format("highlight link %s %s", comment_hl, hl)) + + -- Define range highlight group + local range_hl = hl .. gitlab_range + vim.fn.sign_define(range_hl, { + text = range_icon, + texthl = range_hl, + }) + vim.cmd(string.format("highlight link %s %s", range_hl, hl)) + end +end + +return M diff --git a/lua/gitlab/reviewer/init.lua b/lua/gitlab/reviewer/init.lua index 1745d121..4e060989 100644 --- a/lua/gitlab/reviewer/init.lua +++ b/lua/gitlab/reviewer/init.lua @@ -1,4 +1,9 @@ --- This Module contains all of the reviewer code for diffview +-- This Module contains all of the reviewer code. This is the code +-- that parses or interacts with diffview directly, such as opening +-- and closing, getting metadata about the current view, and registering +-- callbacks for open/close actions. + +local List = require("gitlab.utils.list") local u = require("gitlab.utils") local state = require("gitlab.state") local git = require("gitlab.git") @@ -55,6 +60,13 @@ M.open = function() u.notify("This merge request has conflicts!", vim.log.levels.WARN) end + if state.settings.discussion_diagnostic ~= nil or state.settings.discussion_sign ~= nil then + u.notify( + "Diagnostics are now configured settings.discussion_signs, see :h gitlab.signs_and_diagnostics", + vim.log.levels.WARN + ) + end + -- Register Diffview hook for close event to set tab page # to nil local on_diffview_closed = function(view) if view.tabpage == M.tabnr then @@ -83,8 +95,7 @@ end ---@param file_name string ---@param new_line number|nil ---@param old_line number|nil ----@param opts table -M.jump = function(file_name, new_line, old_line, opts) +M.jump = function(file_name, new_line, old_line) if M.tabnr == nil then u.notify("Can't jump to Diffvew. Is it open?", vim.log.levels.ERROR) return @@ -96,34 +107,24 @@ M.jump = function(file_name, new_line, old_line, opts) u.notify("Could not find Diffview view", vim.log.levels.ERROR) return end + if not async_ok then + u.notify("Could not load Diffview async", vim.log.levels.ERROR) + return + end + local files = view.panel:ordered_file_list() + local file = List.new(files):find(function(file) + return file.path == file_name + end) + async.await(view:set_file(file)) + local layout = view.cur_layout - for _, file in ipairs(files) do - if file.path == file_name then - if not async_ok then - u.notify("Could not load Diffview async", vim.log.levels.ERROR) - return - end - async.await(view:set_file(file)) - -- TODO: Ranged comments on unchanged lines will have both a - -- new line and a old line. - -- - -- The same is true when the user leaves a single-line comment - -- on an unchanged line in the "b" buffer. - -- - -- We need to distinguish them somehow from - -- range comments (which also have this) so that we can know - -- which buffer to jump to. Right now, we jump to the wrong - -- buffer for ranged comments on unchanged lines. - if new_line ~= nil and not opts.is_undefined_type then - layout.b:focus() - vim.api.nvim_win_set_cursor(0, { new_line, 0 }) - elseif old_line ~= nil then - layout.a:focus() - vim.api.nvim_win_set_cursor(0, { old_line, 0 }) - end - break - end + if old_line == nil then + layout.b:focus() + vim.api.nvim_win_set_cursor(0, { new_line, 0 }) + else + layout.a:focus() + vim.api.nvim_win_set_cursor(0, { old_line, 0 }) end end @@ -168,8 +169,8 @@ M.get_reviewer_data = function() local new_line = vim.api.nvim_win_get_cursor(new_win)[1] local old_line = vim.api.nvim_win_get_cursor(old_win)[1] - local is_current_sha = M.is_current_sha() - local modification_type = hunks.get_modification_type(old_line, new_line, current_file, is_current_sha) + local is_current_sha_focused = M.is_current_sha_focused() + local modification_type = hunks.get_modification_type(old_line, new_line, current_file, is_current_sha_focused) if modification_type == nil then u.notify("Error getting modification type", vim.log.levels.ERROR) return @@ -179,8 +180,8 @@ M.get_reviewer_data = function() u.notify("Comments on unmodified lines will be placed in the old file", vim.log.levels.WARN) end - local current_bufnr = is_current_sha and layout.b.file.bufnr or layout.a.file.bufnr - local opposite_bufnr = is_current_sha and layout.a.file.bufnr or layout.b.file.bufnr + local current_bufnr = is_current_sha_focused and layout.b.file.bufnr or layout.a.file.bufnr + local opposite_bufnr = is_current_sha_focused and layout.a.file.bufnr or layout.b.file.bufnr local old_sha_win_id = u.get_window_id_by_buffer_id(layout.a.file.bufnr) local new_sha_win_id = u.get_window_id_by_buffer_id(layout.b.file.bufnr) @@ -198,7 +199,7 @@ end ---Return whether user is focused on the new version of the file ---@return boolean -M.is_current_sha = function() +M.is_current_sha_focused = function() local view = diffview_lib.get_current_view() local layout = view.cur_layout local b_win = u.get_window_id_by_buffer_id(layout.b.file.bufnr) @@ -213,14 +214,6 @@ M.is_current_sha = function() return current_win == b_win end ----Checks whether the lines in the two buffers are the same ----@return boolean -M.lines_are_same = function(layout, a_cursor, b_cursor) - local line_a = u.get_line_content(layout.a.file.bufnr, a_cursor) - local line_b = u.get_line_content(layout.b.file.bufnr, b_cursor) - return line_a == line_b -end - ---Get currently shown file ---@return string|nil M.get_current_file = function() @@ -231,43 +224,6 @@ M.get_current_file = function() return view.panel.cur_file.path end --- Places a sign on the line for currently reviewed file. ----@param signs SignTable[] table of signs. See :h sign_placelist ----@param type string "new" if diagnostic should be in file after changes else "old" -M.place_sign = function(signs, type) - local view = diffview_lib.get_current_view() - if not view then - return - end - if type == "new" then - for _, sign in ipairs(signs) do - sign.buffer = view.cur_layout.b.file.bufnr - end - elseif type == "old" then - for _, sign in ipairs(signs) do - sign.buffer = view.cur_layout.a.file.bufnr - end - end - vim.fn.sign_placelist(signs) -end - ----Set diagnostics in currently reviewed file. ----@param namespace integer namespace for diagnostics ----@param diagnostics table see :h vim.diagnostic.set ----@param type string "new" if diagnostic should be in file after changes else "old" ----@param opts table? see :h vim.diagnostic.set -M.set_diagnostics = function(namespace, diagnostics, type, opts) - local view = diffview_lib.get_current_view() - if not view then - return - end - if type == "new" and view.cur_layout.b.file.bufnr then - vim.diagnostic.set(namespace, view.cur_layout.b.file.bufnr, diagnostics, opts) - elseif type == "old" and view.cur_layout.a.file.bufnr then - vim.diagnostic.set(namespace, view.cur_layout.a.file.bufnr, diagnostics, opts) - end -end - ---Diffview exposes events which can be used to setup autocommands. ---@param callback fun(opts: table) - for more information about opts see callback in :h nvim_create_autocmd M.set_callback_for_file_changed = function(callback) diff --git a/lua/gitlab/reviewer/location.lua b/lua/gitlab/reviewer/location.lua index d84eeacb..b0299be9 100755 --- a/lua/gitlab/reviewer/location.lua +++ b/lua/gitlab/reviewer/location.lua @@ -91,8 +91,8 @@ end ---@return number|nil function Location:get_line_number_from_new_sha(line) local reviewer = require("gitlab.reviewer") - local is_current_sha = reviewer.is_current_sha() - if is_current_sha then + local is_current_sha_focused = reviewer.is_current_sha_focused() + if is_current_sha_focused then return line end -- Otherwise we want to get the matching line in the opposite buffer @@ -106,8 +106,8 @@ end ---@return number|nil function Location:get_line_number_from_old_sha(line) local reviewer = require("gitlab.reviewer") - local is_current_sha = reviewer.is_current_sha() - if not is_current_sha then + local is_current_sha_focused = reviewer.is_current_sha_focused() + if not is_current_sha_focused then return line end @@ -120,7 +120,8 @@ end ---@return number|nil function Location:get_current_line() local reviewer = require("gitlab.reviewer") - local win_id = reviewer.is_current_sha() and self.reviewer_data.new_sha_win_id or self.reviewer_data.old_sha_win_id + local win_id = reviewer.is_current_sha_focused() and self.reviewer_data.new_sha_win_id + or self.reviewer_data.old_sha_win_id if win_id == nil then return end @@ -141,8 +142,8 @@ function Location:set_start_range(visual_range) end local reviewer = require("gitlab.reviewer") - local is_current_sha = reviewer.is_current_sha() - local win_id = is_current_sha and self.reviewer_data.new_sha_win_id or self.reviewer_data.old_sha_win_id + local is_current_sha_focused = reviewer.is_current_sha_focused() + local win_id = is_current_sha_focused and self.reviewer_data.new_sha_win_id or self.reviewer_data.old_sha_win_id if win_id == nil then u.notify("Error getting window number of SHA for start range", vim.log.levels.ERROR) return @@ -164,7 +165,7 @@ function Location:set_start_range(visual_range) return end - local modification_type = hunks.get_modification_type(old_line, new_line, current_file, is_current_sha) + local modification_type = hunks.get_modification_type(old_line, new_line, current_file, is_current_sha_focused) if modification_type == nil then u.notify("Error getting modification type for start of range", vim.log.levels.ERROR) return @@ -205,8 +206,8 @@ function Location:set_end_range(visual_range) end local reviewer = require("gitlab.reviewer") - local is_current_sha = reviewer.is_current_sha() - local modification_type = hunks.get_modification_type(old_line, new_line, current_file, is_current_sha) + local is_current_sha_focused = reviewer.is_current_sha_focused() + local modification_type = hunks.get_modification_type(old_line, new_line, current_file, is_current_sha_focused) if modification_type == nil then u.notify("Error getting modification type for end of range", vim.log.levels.ERROR) return diff --git a/lua/gitlab/state.lua b/lua/gitlab/state.lua index 992e66ba..8d6acb04 100644 --- a/lua/gitlab/state.lua +++ b/lua/gitlab/state.lua @@ -114,35 +114,16 @@ M.settings = { "labels", }, }, - discussion_sign_and_diagnostic = { - skip_resolved_discussion = false, - skip_old_revision_discussion = false, - }, - discussion_sign = { - -- See :h sign_define for details about sign configuration. - enabled = true, - text = "💬", - linehl = nil, - texthl = nil, - culhl = nil, - numhl = nil, - priority = 20, - helper_signs = { - -- For multiline comments the helper signs are used to indicate the whole context - -- Priority of helper signs is lower than the main sign (-1). - enabled = true, - start = "↑", - mid = "|", - ["end"] = "↓", - }, - }, - discussion_diagnostic = { - -- If you want to customize diagnostics for discussions you can make special config - -- for namespace `gitlab_discussion`. See :h vim.diagnostic.config + discussion_signs = { enabled = true, + skip_resolved_discussion = false, severity = vim.diagnostic.severity.INFO, - code = nil, -- see :h diagnostic-structure - display_opts = {}, -- this is dirrectly used as opts in vim.diagnostic.set, see :h vim.diagnostic.config. + virtual_text = false, + icons = { + comment = "→|", + range = " |", + }, + skip_old_revision_discussion = false, }, pipeline = { created = "", diff --git a/lua/gitlab/utils/init.lua b/lua/gitlab/utils/init.lua index fe05ccbc..ec258666 100644 --- a/lua/gitlab/utils/init.lua +++ b/lua/gitlab/utils/init.lua @@ -59,6 +59,21 @@ M.merge = function(defaults, overrides) return vim.tbl_deep_extend("force", defaults, overrides) end +---Combines two list-like (non associative) tables, keeping values from both +---@param t1 table The first table +---@param ... table[] The first table +---@return table +M.combine = function(t1, ...) + local result = t1 + local tables = { ... } + for _, t in ipairs(tables) do + for _, v in ipairs(t) do + table.insert(result, v) + end + end + return result +end + ---Pluralizes the input word, e.g. "3 cows" ---@param num integer The count of the item/word ---@param word string The word to pluralize diff --git a/lua/gitlab/utils/list.lua b/lua/gitlab/utils/list.lua index 27180388..71db0023 100644 --- a/lua/gitlab/utils/list.lua +++ b/lua/gitlab/utils/list.lua @@ -13,8 +13,8 @@ end ---@return List @Returns a new list of elements mutated by func function List:map(func) local result = List.new() - for i, v in ipairs(self) do - result[i] = func(v) + for _, v in ipairs(self) do + table.insert(result, func(v)) end return result end @@ -25,9 +25,9 @@ end ---@return List @Returns a new list of elements for which func returns true function List:filter(func) local result = List.new() - for i, v in ipairs(self) do + for _, v in ipairs(self) do if func(v) == true then - result[i] = v + table.insert(result, v) end end return result @@ -56,11 +56,19 @@ function List:find(func) end function List:slice(first, last, step) - local sliced = {} + local sliced = List.new() for i = first or 1, last or #self, step or 1 do sliced[#sliced + 1] = self[i] end return sliced end +function List:values() + local result = {} + for _, v in ipairs(self) do + table.insert(result, v) + end + return result +end + return List diff --git a/script.lua b/script.lua new file mode 100644 index 00000000..df38c38f --- /dev/null +++ b/script.lua @@ -0,0 +1,12 @@ +local combine = function(t1, ...) + local result = t1 + local tables = { ... } + for _, t in ipairs(tables) do + for _, v in ipairs(t) do + table.insert(result, v) + end + end + return result +end + +vim.print(combine({ 1, 2, 3 }, { 4, 5, 6 }, { 7, 8, 9 })) From f6ada57ba106c2c6021ea55cb0aa4ad792725630 Mon Sep 17 00:00:00 2001 From: "Harrison (Harry) Cramer" <32515581+harrisoncramer@users.noreply.github.com> Date: Sat, 2 Mar 2024 15:58:46 -0500 Subject: [PATCH 12/16] Fixes Off-By-One Issue w/ New SHA (#205) Fixes off-by one in new SHA comment creation, thank you @jakubbortlik for catching this --- lua/gitlab/hunks/init.lua | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lua/gitlab/hunks/init.lua b/lua/gitlab/hunks/init.lua index 06e9fbcf..2df8c044 100644 --- a/lua/gitlab/hunks/init.lua +++ b/lua/gitlab/hunks/init.lua @@ -70,7 +70,7 @@ local line_was_added = function(linnr, hunk, all_diff_output) -- index for the line number, we check to see if the line was added. local i = 0 local old_range = (found_hunk.old_range == 0 and found_hunk.old_line ~= 0) and 1 or found_hunk.old_range - for hunk_line_index = matching_line_index + old_range + 1, matching_line_index + old_range + found_hunk.new_range, 1 do + for hunk_line_index = matching_line_index + old_range, matching_line_index + old_range + found_hunk.new_range, 1 do local line_content = all_diff_output[hunk_line_index] if (found_hunk.new_line + i) == linnr then if string.match(line_content, "^%+") then From 949e0765dc65f2c30c877e8568573921f2ae45de Mon Sep 17 00:00:00 2001 From: "Harrison (Harry) Cramer" <32515581+harrisoncramer@users.noreply.github.com> Date: Sun, 3 Mar 2024 11:42:35 -0500 Subject: [PATCH 13/16] Fix: Reviewer Jump (#206) Fixes issue where jumping to the reviewer would jump to the wrong buffer (old SHA vs new SHA). Also addresses an issue in the refactor where ranged signs were being placed on top of comment signs. --- README.md | 8 +-- lua/gitlab/actions/discussions/init.lua | 95 +++++++++++++++++-------- lua/gitlab/actions/discussions/tree.lua | 16 ++--- lua/gitlab/indicators/signs.lua | 30 +++++--- lua/gitlab/reviewer/init.lua | 6 +- lua/gitlab/state.lua | 1 + lua/gitlab/utils/init.lua | 28 +++----- script.lua | 12 ---- 8 files changed, 102 insertions(+), 94 deletions(-) delete mode 100644 script.lua diff --git a/README.md b/README.md index c80326df..2f6cf8e7 100644 --- a/README.md +++ b/README.md @@ -172,14 +172,14 @@ require("gitlab").setup({ }, discussion_signs = { enabled = true, -- Show diagnostics for gitlab comments in the reviewer - skip_resolved_discussion = false, - severity = vim.diagnostic.severity.INFO, - virtual_text = false, + skip_resolved_discussion = false, -- Show diagnostics for resolved discussions + severity = vim.diagnostic.severity.INFO, -- ERROR, WARN, INFO, or HINT + virtual_text = false, -- Whether to show the comment text inline as floating virtual text + priority = 100, -- Higher will override LSP warnings, etc icons = { comment = "→|", range = " |", }, - skip_old_revision_discussion = false, }, pipeline = { created = "", diff --git a/lua/gitlab/actions/discussions/init.lua b/lua/gitlab/actions/discussions/init.lua index dde14832..eb98661d 100644 --- a/lua/gitlab/actions/discussions/init.lua +++ b/lua/gitlab/actions/discussions/init.lua @@ -13,6 +13,7 @@ local List = require("gitlab.utils.list") local miscellaneous = require("gitlab.actions.miscellaneous") local discussions_tree = require("gitlab.actions.discussions.tree") local diffview_lib = require("diffview.lib") +local common = require("gitlab.indicators.common") local signs = require("gitlab.indicators.signs") local diagnostics = require("gitlab.indicators.diagnostics") local winbar = require("gitlab.actions.discussions.winbar") @@ -386,35 +387,82 @@ M.toggle_discussion_resolved = function(tree) end) end --- This function (settings.discussion_tree.jump_to_reviewer) will jump the cursor to the reviewer's location associated with the note. The implementation depends on the reviewer -M.jump_to_reviewer = function(tree) - local file_name, new_line, old_line, error = M.get_note_location(tree) - if error ~= nil then - u.notify(error, vim.log.levels.ERROR) - return +---Takes a node and returns the line where the note is positioned in the new SHA. If +---the line is not in the new SHA, returns nil +---@param node any +---@return number|nil +local function get_new_line(node) + if node.new_line == nil then + return nil end - local new_line_int = tonumber(new_line) - local old_line_int = tonumber(old_line) + ---@type GitlabLineRange|nil + local range = node.range + if range == nil then + if node.new_line == nil then + return nil + end + return node.new_line + end - if new_line_int == nil and old_line_int == nil then - u.notify("Could not get new or old line", vim.log.levels.ERROR) - return + local start_new_line, _ = common.parse_line_code(range.start.line_code) + return start_new_line +end + +---Takes a node and returns the line where the note is positioned in the old SHA. If +---the line is not in the old SHA, returns nil +---@param node any +---@return number|nil +local function get_old_line(node) + if node.old_line == nil then + return nil + end + + ---@type GitlabLineRange|nil + local range = node.range + if range == nil then + return node.old_line end - reviewer.jump(file_name, new_line_int, old_line_int) + local _, start_old_line = common.parse_line_code(range.start.line_code) + return start_old_line +end + +-- This function (settings.discussion_tree.jump_to_reviewer) will jump the cursor to the reviewer's location associated with the note. The implementation depends on the reviewer +M.jump_to_reviewer = function(tree) + local node = tree:get_node() + local root_node = M.get_root_node(tree, node) + if root_node == nil then + u.notify("Could not get discussion node", vim.log.levels.ERROR) + return + end + reviewer.jump(root_node.file_name, get_new_line(root_node), get_old_line(root_node)) M.refresh_view() end -- This function (settings.discussion_tree.jump_to_file) will jump to the file changed in a new tab M.jump_to_file = function(tree) - local file_name, new_line, old_line, error = M.get_note_location(tree) - if error ~= nil then - u.notify(error, vim.log.levels.ERROR) + local node = tree:get_node() + local root_node = M.get_root_node(tree, node) + if root_node == nil then + u.notify("Could not get discussion node", vim.log.levels.ERROR) return end vim.cmd.tabnew() - u.jump_to_file(file_name, (new_line or old_line)) + local line_number = get_new_line(root_node) or get_old_line(root_node) + if line_number == nil then + line_number = 1 + end + local bufnr = vim.fn.bufnr(root_node.filename) + if bufnr ~= -1 then + vim.cmd("buffer " .. bufnr) + vim.api.nvim_win_set_cursor(0, { line_number, 0 }) + return + end + + -- If buffer is not already open, open it + vim.cmd("edit " .. root_node.filename) + vim.api.nvim_win_set_cursor(0, { line_number, 0 }) end -- This function (settings.discussion_tree.toggle_node) expands/collapses the current node and its children @@ -908,21 +956,6 @@ M.add_reply_to_tree = function(tree, note, discussion_id) tree:render() end ----Get note location ----@param tree NuiTree ----@return string, string, string, string? -M.get_note_location = function(tree) - local node = tree:get_node() - if node == nil then - return "", "", "", "Could not get node" - end - local discussion_node = M.get_root_node(tree, node) - if discussion_node == nil then - return "", "", "", "Could not get discussion node" - end - return discussion_node.file_name, discussion_node.new_line, discussion_node.old_line, nil -end - ---@param tree NuiTree M.open_in_browser = function(tree) local current_node = tree:get_node() diff --git a/lua/gitlab/actions/discussions/tree.lua b/lua/gitlab/actions/discussions/tree.lua index d52ed1a0..50ea6763 100644 --- a/lua/gitlab/actions/discussions/tree.lua +++ b/lua/gitlab/actions/discussions/tree.lua @@ -158,8 +158,9 @@ M.add_discussions_to_table = function(items, unlinked) local root_id local root_text_nodes = {} local resolvable = false + ---@type GitlabLineRange|nil + local range = nil local resolved = false - local undefined_type = false local root_new_line = nil local root_old_line = nil local root_url @@ -175,16 +176,7 @@ M.add_discussions_to_table = function(items, unlinked) resolvable = note.resolvable resolved = note.resolved root_url = state.INFO.web_url .. "#note_" .. note.id - - -- This appears to be a Gitlab 🐛 where the "type" is returned as an empty string in some cases - -- We link these comments to the old file by default - if - type(note.position) == "table" - and note.position.line_range ~= nil - and note.position.line_range.start.type == "" - then - undefined_type = true - end + range = (type(note.position) == "table" and note.position.line_range or nil) else -- Otherwise insert it as a child node... local note_node = M.build_note(note) table.insert(discussion_children, note_node) @@ -194,6 +186,7 @@ M.add_discussions_to_table = function(items, unlinked) -- Creates the first node in the discussion, and attaches children local body = u.spread(root_text_nodes, discussion_children) local root_node = NuiTree.Node({ + range = range, text = root_text, type = "note", is_root = true, @@ -204,7 +197,6 @@ M.add_discussions_to_table = function(items, unlinked) old_line = root_old_line, resolvable = resolvable, resolved = resolved, - undefined_type = undefined_type, url = root_url, }, body) diff --git a/lua/gitlab/indicators/signs.lua b/lua/gitlab/indicators/signs.lua index 9b94a4e4..8631574a 100644 --- a/lua/gitlab/indicators/signs.lua +++ b/lua/gitlab/indicators/signs.lua @@ -1,4 +1,6 @@ +local u = require("gitlab.utils") local state = require("gitlab.state") +local List = require("gitlab.utils.list") local discussion_sign_name = require("gitlab.indicators.diagnostics").discussion_sign_name local namespace = require("gitlab.indicators.diagnostics").diagnostics_namespace @@ -28,19 +30,27 @@ M.set_signs = function(diagnostics, bufnr) -- Filter diagnostics from the 'gitlab' source and apply custom signs for _, diagnostic in ipairs(diagnostics) do - local sign_id = string.format("%s__%d", namespace, diagnostic.lnum) + ---@type SignTable[] + local existing_signs = + vim.fn.sign_getplaced(vim.api.nvim_get_current_buf(), { group = "gitlab_discussion" })[1].signs + local sign_id = string.format("%s__%d", namespace, diagnostic.lnum) if diagnostic.end_lnum then local linenr = diagnostic.lnum + 1 - while linenr < diagnostic.end_lnum do + while linenr <= diagnostic.end_lnum do linenr = linenr + 1 - vim.fn.sign_place( - sign_id, - discussion_sign_name, - "DiagnosticSign" .. M.severity .. gitlab_range, - bufnr, - { lnum = linenr, priority = 999998 } - ) + local conflicting_comment_sign = List.new(existing_signs):find(function(sign) + return u.ends_with(sign.name, gitlab_comment) and sign.lnum == linenr + end) + if conflicting_comment_sign == nil then + vim.fn.sign_place( + sign_id, + discussion_sign_name, + "DiagnosticSign" .. M.severity .. gitlab_range, + bufnr, + { lnum = linenr, priority = state.settings.discussion_signs.priority } + ) + end end end @@ -49,7 +59,7 @@ M.set_signs = function(diagnostics, bufnr) discussion_sign_name, "DiagnosticSign" .. M.severity .. gitlab_comment, bufnr, - { lnum = diagnostic.lnum + 1, priority = 999999 } + { lnum = diagnostic.lnum + 1, priority = state.settings.discussion_signs.priority } ) -- TODO: Detect whether diagnostic is ranged and set helper signs diff --git a/lua/gitlab/reviewer/init.lua b/lua/gitlab/reviewer/init.lua index 4e060989..8a074fc6 100644 --- a/lua/gitlab/reviewer/init.lua +++ b/lua/gitlab/reviewer/init.lua @@ -8,7 +8,7 @@ local u = require("gitlab.utils") local state = require("gitlab.state") local git = require("gitlab.git") local hunks = require("gitlab.hunks") -local async_ok, async = pcall(require, "diffview.async") +local async = require("diffview.async") local diffview_lib = require("diffview.lib") local M = { @@ -107,10 +107,6 @@ M.jump = function(file_name, new_line, old_line) u.notify("Could not find Diffview view", vim.log.levels.ERROR) return end - if not async_ok then - u.notify("Could not load Diffview async", vim.log.levels.ERROR) - return - end local files = view.panel:ordered_file_list() local file = List.new(files):find(function(file) diff --git a/lua/gitlab/state.lua b/lua/gitlab/state.lua index 8d6acb04..41dda027 100644 --- a/lua/gitlab/state.lua +++ b/lua/gitlab/state.lua @@ -124,6 +124,7 @@ M.settings = { range = " |", }, skip_old_revision_discussion = false, + priority = 100, }, pipeline = { created = "", diff --git a/lua/gitlab/utils/init.lua b/lua/gitlab/utils/init.lua index ec258666..f641162d 100644 --- a/lua/gitlab/utils/init.lua +++ b/lua/gitlab/utils/init.lua @@ -29,6 +29,14 @@ M.get_last_word = function(sentence, divider) return words[#words] or "" end +---Returns whether a string ends with a substring +---@param str string +---@param ending string +---@return boolean +M.ends_with = function(str, ending) + return ending == "" or str:sub(-#ending) == ending +end + M.filter = function(input_table, value_to_remove) local resultTable = {} for _, v in ipairs(input_table) do @@ -389,26 +397,6 @@ M.difference = function(a, b) return not_included end -M.jump_to_file = function(filename, line_number) - if line_number == nil then - line_number = 1 - end - local bufnr = vim.fn.bufnr(filename) - if bufnr ~= -1 then - M.jump_to_buffer(bufnr, line_number) - return - end - - -- If buffer is not already open, open it - vim.cmd("edit " .. filename) - vim.api.nvim_win_set_cursor(0, { line_number, 0 }) -end - -M.jump_to_buffer = function(bufnr, line_number) - vim.cmd("buffer " .. bufnr) - vim.api.nvim_win_set_cursor(0, { line_number, 0 }) -end - ---Get the popup view_opts ---@param title string The string to appear on top of the popup ---@param settings table User defined popup settings diff --git a/script.lua b/script.lua deleted file mode 100644 index df38c38f..00000000 --- a/script.lua +++ /dev/null @@ -1,12 +0,0 @@ -local combine = function(t1, ...) - local result = t1 - local tables = { ... } - for _, t in ipairs(tables) do - for _, v in ipairs(t) do - table.insert(result, v) - end - end - return result -end - -vim.print(combine({ 1, 2, 3 }, { 4, 5, 6 }, { 7, 8, 9 })) From e6cc34c42a69995e5ada8cc83b8fb9b8973f2bca Mon Sep 17 00:00:00 2001 From: "Harrison (Harry) Cramer" <32515581+harrisoncramer@users.noreply.github.com> Date: Sun, 3 Mar 2024 12:14:45 -0500 Subject: [PATCH 14/16] Fix Typo In Settings Check (#209) Fixes typo in settings check, this is a #PATCH release --- lua/gitlab/indicators/signs.lua | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lua/gitlab/indicators/signs.lua b/lua/gitlab/indicators/signs.lua index 8631574a..2b3ac19e 100644 --- a/lua/gitlab/indicators/signs.lua +++ b/lua/gitlab/indicators/signs.lua @@ -24,7 +24,7 @@ local severity_map = { ---@param diagnostics Diagnostic[] ---@param bufnr number M.set_signs = function(diagnostics, bufnr) - if not state.settings.discussion_sign.enabled then + if not state.settings.discussion_signs.enabled then return end From d5ad0ea34df53ba1d20c3c3af9f3d86782acde53 Mon Sep 17 00:00:00 2001 From: "Harrison (Harry) Cramer" <32515581+harrisoncramer@users.noreply.github.com> Date: Mon, 4 Mar 2024 09:17:40 -0500 Subject: [PATCH 15/16] fix: Calculate new line in ranged comment after all hunks correctly (#211) fix: Calculate new line in ranged comment after all hunks correctly --- lua/gitlab/hunks/init.lua | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lua/gitlab/hunks/init.lua b/lua/gitlab/hunks/init.lua index 2df8c044..a14d5be4 100644 --- a/lua/gitlab/hunks/init.lua +++ b/lua/gitlab/hunks/init.lua @@ -274,7 +274,7 @@ M.calculate_matching_line_new = function(old_sha, new_sha, file_path, line_numbe end -- TODO: Possibly handle lines that are out of range in the new files - return line_number + return line_number + net_change + 1 end return M From ca35faa52e5f783aace1c2d271f3ba5690a95b0b Mon Sep 17 00:00:00 2001 From: "Harrison (Harry) Cramer" <32515581+harrisoncramer@users.noreply.github.com> Date: Mon, 4 Mar 2024 10:48:12 -0500 Subject: [PATCH 16/16] fix: range in new SHA (#212) Fix: Ranged comments in new SHA --- lua/gitlab/hunks/init.lua | 29 +++++++++++++++++------------ 1 file changed, 17 insertions(+), 12 deletions(-) diff --git a/lua/gitlab/hunks/init.lua b/lua/gitlab/hunks/init.lua index a14d5be4..f90d6f1c 100644 --- a/lua/gitlab/hunks/init.lua +++ b/lua/gitlab/hunks/init.lua @@ -64,20 +64,25 @@ local line_was_added = function(linnr, hunk, all_diff_output) for matching_line_index, line in ipairs(all_diff_output) do local found_hunk = M.parse_possible_hunk_headers(line) if found_hunk ~= nil and vim.deep_equal(found_hunk, hunk) then - -- For added lines, we only want to iterate over the part of the diff that has has new lines, - -- so we skip over the old range. We then keep track of the increment to the original new line index, - -- and iterate until we reach the end of the total range of this hunk. If we arrive at the matching - -- index for the line number, we check to see if the line was added. - local i = 0 - local old_range = (found_hunk.old_range == 0 and found_hunk.old_line ~= 0) and 1 or found_hunk.old_range - for hunk_line_index = matching_line_index + old_range, matching_line_index + old_range + found_hunk.new_range, 1 do - local line_content = all_diff_output[hunk_line_index] - if (found_hunk.new_line + i) == linnr then - if string.match(line_content, "^%+") then - return true - end + -- Parse the lines from the hunk and return only the added lines + local hunk_lines = {} + local i = 1 + local line_content = all_diff_output[matching_line_index + i] + while line_content ~= nil and line_content:sub(1, 2) ~= "@@" do + if string.match(line_content, "^%+") then + table.insert(hunk_lines, line_content) end i = i + 1 + line_content = all_diff_output[matching_line_index + i] + end + + -- We are only looking at added lines in the changed hunk to see if their index + -- matches the index of a line that was added + local starting_index = found_hunk.new_line - 1 -- The "+j" will add one + for j, _ in ipairs(hunk_lines) do + if (starting_index + j) == linnr then + return true + end end end end