Fix: Discussion + Note Creation Bugs (#151)

This MR is an attempt to resolve some of the issues this plugin is experiencing with Gitlab's API surrounding comments, in particular the specifics around when to send the "old line" versus "new line" and the file hashes.

This is a PATCH release.
This commit is contained in:
Harrison (Harry) Cramer
2024-01-13 10:52:45 -05:00
committed by GitHub
parent ed3a90cf00
commit 519791c81c
3 changed files with 41 additions and 6 deletions

View File

@@ -192,7 +192,7 @@ M.parse_diagnostics_from_discussions = function(discussions)
return {}, {}, string.format("Unsupported line range type found for discussion %s", discussion.id) return {}, {}, string.format("Unsupported line range type found for discussion %s", discussion.id)
end end
else -- Diagnostics for single line discussions. else -- Diagnostics for single line discussions.
if first_note.position.new_line ~= nil then if first_note.position.new_line ~= nil and first_note.position.old_line == nil then
local new_diagnostic = { local new_diagnostic = {
lnum = first_note.position.new_line - 1, lnum = first_note.position.new_line - 1,
} }
@@ -306,7 +306,7 @@ M.parse_signs_from_discussions = function(discussions)
local sign = vim.tbl_deep_extend("force", { local sign = vim.tbl_deep_extend("force", {
id = first_note.id, id = first_note.id,
}, base_sign) }, base_sign)
if first_note.position.new_line ~= nil then 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)) table.insert(new_signs, vim.tbl_deep_extend("force", { lnum = first_note.position.new_line }, sign))
end end
if first_note.position.old_line ~= nil then if first_note.position.old_line ~= nil then

View File

@@ -53,7 +53,9 @@ M.update_winbar = function(discussions, unlinked_discussions, base_title)
local d = require("gitlab.actions.discussions") local d = require("gitlab.actions.discussions")
local winId = d.split.winid local winId = d.split.winid
local c = content(discussions, unlinked_discussions, base_title) local c = content(discussions, unlinked_discussions, base_title)
if vim.wo[winId] then
vim.wo[winId].winbar = c vim.wo[winId].winbar = c
end
end end
return M return M

View File

@@ -75,7 +75,12 @@ M.jump = function(file_name, new_line, old_line, opts)
end end
async.await(view:set_file(file)) async.await(view:set_file(file))
-- TODO: Ranged comments on unchanged lines will have both a -- TODO: Ranged comments on unchanged lines will have both a
-- new line and a old line. We need to distinguish them somehow from -- 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 -- range comments (which also have this) so that we can know
-- which buffer to jump to. Right now, we jump to the wrong -- which buffer to jump to. Right now, we jump to the wrong
-- buffer for ranged comments on unchanged lines. -- buffer for ranged comments on unchanged lines.
@@ -100,8 +105,11 @@ M.get_location = function(range)
u.notify("Diffview reviewer must be initialized first", vim.log.levels.ERROR) u.notify("Diffview reviewer must be initialized first", vim.log.levels.ERROR)
return return
end end
local bufnr = vim.api.nvim_get_current_buf() local bufnr = vim.api.nvim_get_current_buf()
local current_line = vim.api.nvim_win_get_cursor(0)[1]
-- 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 -- check if we are in the diffview tab
local tabnr = vim.api.nvim_get_current_tabpage() local tabnr = vim.api.nvim_get_current_tabpage()
@@ -120,7 +128,11 @@ M.get_location = function(range)
local result = {} local result = {}
local type local type
local is_new local is_new
if layout.a.file.bufnr == bufnr then
if
layout.a.file.bufnr == bufnr
or (M.lines_are_same(view.cur_layout) and layout.b.file.bufnr == bufnr and range == nil)
then
result.file_name = layout.a.file.path result.file_name = layout.a.file.path
result.old_line = current_line result.old_line = current_line
type = "old" type = "old"
@@ -156,6 +168,16 @@ M.get_location = function(range)
result.new_line = current_line_info.new_line result.new_line = current_line_info.new_line
end end
-- If users leave single-line comments in the new buffer that should be in the old buffer, we can
-- tell because the line will not have changed. Send the correct payload.
if M.lines_are_same(view.cur_layout) and layout.b.file.bufnr == bufnr and range == nil then
local a_win = u.get_win_from_buf(layout.a.file.bufnr)
local a_cursor = vim.api.nvim_win_get_cursor(a_win)[1]
result.old_line = a_cursor
result.new_line = a_cursor
type = "old"
end
if range == nil then if range == nil then
return result return result
end end
@@ -194,6 +216,17 @@ M.get_lines = function(start_line, end_line)
return vim.api.nvim_buf_get_lines(0, start_line - 1, end_line, false) return vim.api.nvim_buf_get_lines(0, start_line - 1, end_line, false)
end end
---@return boolean
M.lines_are_same = function(layout)
local a_win = u.get_win_from_buf(layout.a.file.bufnr)
local b_win = u.get_win_from_buf(layout.b.file.bufnr)
local a_cursor = vim.api.nvim_win_get_cursor(a_win)[1]
local b_cursor = vim.api.nvim_win_get_cursor(b_win)[1]
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 ---Get currently shown file
M.get_current_file = function() M.get_current_file = function()
local view = diffview_lib.get_current_view() local view = diffview_lib.get_current_view()