Skip to content

Conversation

@harrisoncramer
Copy link
Owner

@harrisoncramer harrisoncramer commented Jan 31, 2025

@jakubbortlik
Copy link
Collaborator

jakubbortlik commented Feb 3, 2025

#446 included the idea to make cycling the reviewed files faster by only updating diagnostics upon entering the files for the first time. This is a brilliant idea, except it doesn't work. Diagnostics are now not displayed when cycling over the files repeatedly.

A simple fix is to add back the refreshing to the set_callback_for_file_changed callback:

diff --git a/lua/gitlab/actions/discussions/init.lua b/lua/gitlab/actions/discussions/init.lua
index 3a32d4e..5b93647 100644
--- a/lua/gitlab/actions/discussions/init.lua
+++ b/lua/gitlab/actions/discussions/init.lua
@@ -74,6 +74,7 @@ M.initialize_discussions = function()
   state.discussion_tree.last_updated = os.time()
   signs.setup_signs()
   reviewer.set_callback_for_file_changed(function(args)
+    M.refresh_diagnostics()
     reviewer.update_winid_for_buffer(args.buf)
   end)
   reviewer.set_callback_for_reviewer_enter(function()

However, that will make the reviewer slow again, because every time the diagnostics are refreshed, the complete list of discussions is filtered several times. I'm working on a fix that will consist of preparing the diagnostics once and then selecting the right diagnostics for the relevant buffer. It will take some time, so if you @harrisoncramer are planning to release #470 soon, I suggest to include the above fix.

I've just opened #473 for that. I've just opened #474 for that, which solves both issues: diagnostic placement and speed of file cycling.

jakubbortlik and others added 7 commits February 16, 2025 12:31
* 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 merged commit 9f898aa into main Mar 1, 2025
6 checks passed
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.

4 participants