Skip to content

Bugfixes, Etc. #502

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 11 commits into
base: main
Choose a base branch
from
Open

Bugfixes, Etc. #502

wants to merge 11 commits into from

Conversation

harrisoncramer
Copy link
Owner

No description provided.

* fix: prevent "cursor position outside buffer" error

* fix: swap file_name and old_file_name in reviewer data

`old_file_name` is not set to the empty string for un-renamed files anymore, because then we can
remove the empty-line check in `comment_helpers.go` which was used to replace the empty string with
the current file name anyway.

* fix: add old_file_name to discussion root node data

* fix: also consider old_file_name when jumping to the reviewer

This fixes jumping to renamed files, however, may not work for comments that
were created on renamed files with the previous version of `gitlab.nvim` as
that version assigned the `file_name` and `old_file_name` incorrectly.

* refactor: don't shadow variable

* fix: check file_name or old_file_name based on which SHA comment belongs to
@harrisoncramer harrisoncramer requested a review from Copilot June 19, 2025 02:04
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR introduces a toggle for relative/absolute date formats and several enhancements across GitLab integration, including improved error handling, branch utilities, and documentation updates.

  • Add toggle_date_format feature with keybinding, state setting, and UI updates
  • Enhance Git helper functions (get_remote_branch, fetch_remote_branch, get_current_branch, check_current_branch_up_to_date_on_remote) and job error callbacks
  • Refine reviewer file lookup logic, diagnostics handling, and update CI workflow versions

Reviewed Changes

Copilot reviewed 16 out of 16 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
lua/gitlab/state.lua Add toggle_date_format key and default relative_date setting
lua/gitlab/actions/discussions/init.lua Register new keymap and implement toggle_date_format
lua/gitlab/actions/common.lua Update build_note_header to switch between relative/absolute dates
lua/gitlab/git.lua Add get_remote_branch, fetch_remote_branch, get_current_branch
lua/gitlab/job.lua Introduce on_error_callback in run_job
lua/gitlab/reviewer/init.lua Fallback old_file_name handling and swap reviewer data assignments
lua/gitlab/indicators/diagnostics.lua Skip diagnostics for diffview://null buffers
lua/gitlab/colors.lua Trigger color autocmd on ColorScheme as well as VimEnter
lua/gitlab/actions/help.lua Extend help.open signature with HelpPopupOpts
lua/gitlab/actions/draft_notes/init.lua Check branch up-to-date before publishing drafts
lua/gitlab/actions/comment.lua Refactor comment layout file name selection
doc/gitlab.nvim.txt Document new toggle and relative_date option
after/syntax/gitlab.vim Extend date regex to match absolute timestamps
.github/workflows/lua.yaml Bump Lua and Luarocks action versions
cmd/app/comment_helpers.go Provide fallback for renamed file payload in buildCommentPosition
cmd/app/client.go Replace manual retry settings with WithoutRetries option
Comments suppressed due to low confidence (2)

lua/gitlab/job.lua:7

  • The new on_error_callback parameter is not documented. Adding a ---@param on_error_callback function? annotation in the function header will help clarify its expected usage.
M.run_job = function(endpoint, method, body, callback, on_error_callback)

lua/gitlab/actions/discussions/init.lua:658

  • The toggle_date_format_nowait property is referenced here but not defined in the default settings. Consider adding it to state.settings.discussion_tree or removing the nowait option.
      nowait = keymaps.discussion_tree.toggle_date_format_nowait,

@@ -123,7 +126,8 @@ M.jump = function(file_name, old_file_name, line_number, new_buffer)

local files = view.panel:ordered_file_list()
local file = List.new(files):find(function(f)
return new_buffer and f.path == file_name or f.oldpath == old_file_name
local oldpath = f.oldpath ~= nil and f.oldpath or f.path
return new_buffer and f.path == file_name or oldpath == old_file_name
Copy link
Preview

Copilot AI Jun 19, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The compound and/or expression can be ambiguous due to operator precedence. Consider rewriting as an explicit if new_buffer then ... elseif ... end to ensure the intended logic.

Suggested change
return new_buffer and f.path == file_name or oldpath == old_file_name
if new_buffer then
return f.path == file_name
else
return oldpath == old_file_name
end

Copilot uses AI. Check for mistakes.

Comment on lines +156 to +158
local file_name = (M.location.reviewer_data.new_sha_focused or M.location.reviewer_data.old_file_name == "")
and M.location.reviewer_data.file_name
or M.location.reviewer_data.old_file_name
Copy link
Preview

Copilot AI Jun 19, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[nitpick] This inline boolean expression is hard to read and maintain. Consider using a simple if/else block to clearly express which path should be used.

Suggested change
local file_name = (M.location.reviewer_data.new_sha_focused or M.location.reviewer_data.old_file_name == "")
and M.location.reviewer_data.file_name
or M.location.reviewer_data.old_file_name
local file_name
if M.location.reviewer_data.new_sha_focused or M.location.reviewer_data.old_file_name == "" then
file_name = M.location.reviewer_data.file_name
else
file_name = M.location.reviewer_data.old_file_name
end

Copilot uses AI. Check for mistakes.

@duckbrain duckbrain mentioned this pull request Jun 19, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants