diff --git a/lua/gitlab/actions/common.lua b/lua/gitlab/actions/common.lua index 03ca63a0..7dfbd4fd 100644 --- a/lua/gitlab/actions/common.lua +++ b/lua/gitlab/actions/common.lua @@ -248,24 +248,42 @@ M.get_line_number = function(id) return ((is_new_sha and first_note.position.new_line or first_note.position.old_line) or 1), is_new_sha end +---Return the number of lines between the start and end of a range on one side of the diff. +---Line codes are preferred because they hold both line numbers, whereas the range's +---`old_line`/`new_line` are 0 on the side a line doesn't exist on. +---@param start_from_code? integer The start line parsed from the line code +---@param end_from_code? integer The end line parsed from the line code +---@param start_line? integer The start line from the range +---@param end_line? integer The end line from the range +---@return integer +local function get_range_length(start_from_code, end_from_code, start_line, end_line) + if start_from_code and end_from_code then + return end_from_code - start_from_code + end + if (start_line or 0) > 0 and (end_line or 0) > 0 then + return end_line - start_line + end + return 0 +end + ---Return start and end line numbers for the note range, and whether comment is on "NEW SHA". ----The range is calculated from the line codes but the position itself is based on ----either the `new_line` or `old_line`. +---The range is calculated from the line codes, or from the range's line numbers if GitLab +---returns empty line codes, but the position itself is based on either the `new_line` or `old_line`. ---@param old_line? integer The line number in the OLD version ---@param new_line? integer The line number in the NEW version ----@param start_line_code string The line code for the start of the range ----@param end_line_code string The line code for the end of the range +---@param line_range LineRange The range of the note ---@return integer start_line ---@return integer end_line ---@return boolean is_new_sha True if line range refers to NEW SHA -M.get_line_numbers_for_range = function(old_line, new_line, start_line_code, end_line_code) - local old_start_line, new_start_line = indicators_common.parse_line_code(start_line_code) - local old_end_line, new_end_line = indicators_common.parse_line_code(end_line_code) +M.get_line_numbers_for_range = function(old_line, new_line, line_range) + local start, end_ = line_range.start, line_range["end"] + local old_start_line, new_start_line = indicators_common.parse_line_code(start.line_code) + local old_end_line, new_end_line = indicators_common.parse_line_code(end_.line_code) if old_line ~= nil and old_start_line ~= 0 then - local range = old_end_line - old_start_line + local range = get_range_length(old_start_line, old_end_line, start.old_line, end_.old_line) return (old_line - range), old_line, false elseif new_line ~= nil then - local range = new_end_line - new_start_line + local range = get_range_length(new_start_line, new_end_line, start.new_line, end_.new_line) -- Force start_line to be greater than 0 -- TODO: use `math.max(new_line - range, 1)` instead local start_line = (new_line - range > 0) and (new_line - range) or 1 @@ -282,12 +300,8 @@ end ---@return boolean is_new_sha True if line number refers to NEW SHA M.get_line_number_from_node = function(root_node) if root_node.range then - local line_number, _, is_new_sha = M.get_line_numbers_for_range( - root_node.old_line, - root_node.new_line, - root_node.range.start.line_code, - root_node.range["end"].line_code - ) + local line_number, _, is_new_sha = + M.get_line_numbers_for_range(root_node.old_line, root_node.new_line, root_node.range) return line_number, is_new_sha else return M.get_line_number(root_node.id) diff --git a/lua/gitlab/indicators/common.lua b/lua/gitlab/indicators/common.lua index 5324a463..cbe5a11a 100644 --- a/lua/gitlab/indicators/common.lua +++ b/lua/gitlab/indicators/common.lua @@ -53,15 +53,14 @@ M.filter_placeable_discussions = function() end ---Parse old and new line from a line code like "3f454a98e586d1aa0d322e19afd5e67e08f2d3c8_10_44". ----@param line_code string A SHA hash of the file name and line numbers before and after change ----@return integer The line number before the change ----@return integer The line number after the change +---GitLab can return an empty line code for a ranged discussion. +---@param line_code? string A SHA hash of the file name and line numbers before and after change +---@return integer? The line number before the change +---@return integer? The line number after the change 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) - old_line = tonumber(old_line) --[[@as integer]] - new_line = tonumber(new_line) --[[@as integer]] - return old_line, new_line + local old_line, new_line = (line_code or ""):match(line_code_regex) + return tonumber(old_line), tonumber(new_line) end ---Return true if discussion/draft belongs to the old file, otherwise false. diff --git a/lua/gitlab/indicators/diagnostics.lua b/lua/gitlab/indicators/diagnostics.lua index d4d6b810..57c91c3d 100644 --- a/lua/gitlab/indicators/diagnostics.lua +++ b/lua/gitlab/indicators/diagnostics.lua @@ -76,12 +76,8 @@ local create_multiline_diagnostic = function(d_or_n) error("Parsing multi-line comment but note does not contain line range") end - local start_line, end_line, _ = actions_common.get_line_numbers_for_range( - first_note.position.old_line, - first_note.position.new_line, - line_range.start.line_code, - line_range["end"].line_code - ) + local start_line, end_line, _ = + actions_common.get_line_numbers_for_range(first_note.position.old_line, first_note.position.new_line, line_range) return create_diagnostic({ lnum = start_line - 1, diff --git a/tests/spec/actions_common_spec.lua b/tests/spec/actions_common_spec.lua new file mode 100644 index 00000000..16711179 --- /dev/null +++ b/tests/spec/actions_common_spec.lua @@ -0,0 +1,83 @@ +local actions_common = require("gitlab.actions.common") + +local sha = "3f454a98e586d1aa0d322e19afd5e67e08f2d3c8" + +local function line_code(old_line, new_line) + return sha .. "_" .. old_line .. "_" .. new_line +end + +local function position(type, old_line, new_line, code) + return { type = type, old_line = old_line, new_line = new_line, line_code = code or "" } +end + +describe("gitlab/actions/common.lua", function() + describe("get_line_numbers_for_range", function() + describe("with line codes", function() + it("Computes a range on the new SHA", function() + local range = { + start = position("new", 0, 40, line_code(1, 40)), + ["end"] = position("new", 0, 44, line_code(1, 44)), + } + assert.are.same({ 40, 44, true }, { actions_common.get_line_numbers_for_range(nil, 44, range) }) + end) + + it("Computes a range on the old SHA", function() + local range = { + start = position("old", 10, 0, line_code(10, 20)), + ["end"] = position("old", 12, 0, line_code(12, 20)), + } + assert.are.same({ 10, 12, false }, { actions_common.get_line_numbers_for_range(12, nil, range) }) + end) + + it("Computes a range spanning deleted and added lines", function() + local range = { + start = position("old", 5, 0, line_code(5, 38)), + ["end"] = position("new", 0, 44, line_code(7, 44)), + } + assert.are.same({ 38, 44, true }, { actions_common.get_line_numbers_for_range(nil, 44, range) }) + end) + end) + + describe("without line codes", function() + it("Computes a range on the new SHA from the range's new lines", function() + local range = { + start = position("new", 0, 24), + ["end"] = position("new", 0, 26), + } + assert.are.same({ 24, 26, true }, { actions_common.get_line_numbers_for_range(nil, 26, range) }) + end) + + it("Computes a range on the old SHA from the range's old lines", function() + local range = { + start = position("old", 10, 0), + ["end"] = position("old", 12, 0), + } + assert.are.same({ 10, 12, false }, { actions_common.get_line_numbers_for_range(12, nil, range) }) + end) + + it("Computes a range from the range's lines when only one line code is empty", function() + local range = { + start = position("new", 0, 24, line_code(1, 24)), + ["end"] = position("new", 0, 26), + } + assert.are.same({ 24, 26, true }, { actions_common.get_line_numbers_for_range(nil, 26, range) }) + end) + + it("Falls back to a single line for a range spanning deleted and added lines", function() + local range = { + start = position("old", 5, 0), + ["end"] = position("new", 0, 44), + } + assert.are.same({ 44, 44, true }, { actions_common.get_line_numbers_for_range(nil, 44, range) }) + end) + + it("Falls back to a single line when the range's lines are missing", function() + local range = { + start = position("old", 5, nil), + ["end"] = position("new", nil, 44), + } + assert.are.same({ 44, 44, true }, { actions_common.get_line_numbers_for_range(nil, 44, range) }) + end) + end) + end) +end) diff --git a/tests/spec/indicators_common_spec.lua b/tests/spec/indicators_common_spec.lua new file mode 100644 index 00000000..3ed5e3f0 --- /dev/null +++ b/tests/spec/indicators_common_spec.lua @@ -0,0 +1,19 @@ +local indicators_common = require("gitlab.indicators.common") + +local sha = "3f454a98e586d1aa0d322e19afd5e67e08f2d3c8" + +describe("gitlab/indicators/common.lua", function() + describe("parse_line_code", function() + it("Parses both line numbers", function() + assert.are.same({ 10, 44 }, { indicators_common.parse_line_code(sha .. "_10_44") }) + end) + + it("Returns nil for an empty line code", function() + assert.are.same({ nil, nil }, { indicators_common.parse_line_code("") }) + end) + + it("Returns nil for a missing line code", function() + assert.are.same({ nil, nil }, { indicators_common.parse_line_code(nil) }) + end) + end) +end)