Skip to content
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

Feat: Enable always jumping to discussion tree #352

Conversation

jakubbortlik
Copy link
Collaborator

This PR makes it possible to customize the behavior when pressing the move_to_discussion_tree keybinding in the reviewer and there is no diagnostic under the cursor. Users can now set up any combination of:

  • show warning
  • jump to last position in discussion tree

Closes #309.

This PR makes it possible to customize the behavior when pressing the `move_to_discussion_tree`
keybinding and there is no diagnostic under cursor. Users can now do any combination of:
- show warning
- jump to last position in discussion tree
@jakubbortlik
Copy link
Collaborator Author

Hi @harrisoncramer, this PR changes the current behaviour - the new default is to always jump to the discussion tree even if there is no diagnostic under the cursor, as I believe this behaviour will be preferred by the majority of users. If, instead, you'd like the default to be that only a warning is shown (same as the old behaviour), I'll make the necessary changes.

Comment on lines 205 to 212
local warning = "No diagnostics for this line."
if state.settings.reviewer_settings.no_diagnostic_actions.jump then
warning = warning .. " Jumping to last position in discussion tree."
vim.api.nvim_win_set_cursor(M.split.winid, { M.last_row, M.last_column })
vim.api.nvim_set_current_win(M.split.winid)
end
if state.settings.reviewer_settings.no_diagnostic_actions.warn then
u.notify(warning, vim.log.levels.WARN)
Copy link
Owner

Choose a reason for hiding this comment

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

This is a pretty simple feature, and I don't think it merits two new fields in the configuration table. I'd like to keep the API surface as small as possible for users.

Let's have it be a single option called jump_with_no_diagnostics and if it's enabled, do not show any warning and do the jump. Otherwise (it should be the. default case) do not jump and show the warning.

    if state.settings.reviewer_settings.jump_with_no_diagnostics then
      vim.api.nvim_win_set_cursor(M.split.winid, { M.last_row, M.last_column })
      vim.api.nvim_set_current_win(M.split.winid)
    else
      local warning = "No diagnostics for this line."
      u.notify(warning, vim.log.levels.WARN)
    end

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

OK, fixed. Thanks for the review.

@harrisoncramer
Copy link
Owner

Small change, but once you make that I'll merge this in. Sorry this took me so long to get to.

@harrisoncramer harrisoncramer merged commit db91972 into harrisoncramer:develop Sep 5, 2024
6 checks passed
harrisoncramer added a commit that referenced this pull request Sep 5, 2024
feat: Enable always jumping to discussion tree (#352)
feat: Enables motions for easier range selection when creating comments/suggestions (e.g. s3j, c3j) (#353)
fix: Makes help popup not editable and close it on BufLeave (#355)

This is a #MINOR release
@jakubbortlik jakubbortlik deleted the feat-enable-jumping-to-discussion-tree-with-no-diagnostic-under-cursor branch September 6, 2024 05:45
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.

Fallback for gitlab.move_to_discussion_tree_from_diagnostic
2 participants