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

Fix: Checking whether comment can be created #434

Open
wants to merge 21 commits into
base: develop
Choose a base branch
from

Conversation

jakubbortlik
Copy link
Collaborator

Hi @harrisoncramer, in this PR I address #424, by making the following changes:

  1. All checks whether a comment can be created are now made for all three functions: create_comment, create_comment_suggestion and create_multiline_comment in a dedicated function gitlab.actions.comment.can_make_comment().
  2. The checks are made in a more meaningful order (e.g., checking that cursor is in the reviewer is made before the check that current mode is visual mode for multiline comments).
  3. The check for clean working tree before making comments is now correct and it's made on a per-file basis, i.e., unsaved/uncommitted changes in a different file do not block a comment in a good file.
  4. Similarly, with a dirty working tree, choosing a MR is not blocked if that would not lead to switching the branch.
  5. User-specified imply_local = true is overridden in the settings if the working tree is dirty. Before, it was possible that the setting imply_local was true, while the option was not actually applied to the DiffviewOpen command and so commenting was unnecessarily blocked for modified files.

There are some other improvements:

  1. Visual mode is exited when multiline comment creation fails.
  2. The ERROR level is used for all messages when a comment cannot be created.
  3. A draft reply can now be created even if the discussion tree is "detached" from the reviewer.

I'll be grateful if you find the time to review and merge this.

If `imply_local` is set but unused due to local changes present when review is started,
`imply_local` is overridden so that local changes do not unnecessarily block comments.

Now, uncommitted changes on a file will only block comments when they are made after the review is
started with `imply_local` applied.
The function create_comment_layout should now always return a layout (it used to return nil if the
comment could not be created at all).
The check that `diffview_lib.get_current_view() == nil` is superfluous because we later also check
explicitly that we are in the reviewer tab.
The position data are not necessary for creating a draft reply and requiring it prevented draft
replies in case the discussion tree was open outside of the reviewer pane.
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.

1 participant