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

C-c C-k deletes my window #253

Open
aecepogluARUP opened this issue Dec 18, 2023 · 3 comments
Open

C-c C-k deletes my window #253

aecepogluARUP opened this issue Dec 18, 2023 · 3 comments

Comments

@aecepogluARUP
Copy link

aecepogluARUP commented Dec 18, 2023

Describe the bug
A clear and concise description of what the bug is.

To Reproduce
Steps to reproduce the behavior:

  1. Create 2 windows side by side: a left_window and a right_window.
  2. Start a code review in the left_window
  3. Press Enter to add a new comment. It'll open in the right_window
  4. C-c C-k to give up
  5. right_window is deleted

Expected behavior
right_window shouldn't be deleted. I Created it. code-review should not delete a window it did not create itself.

code-review-new-buffer-window-strategy is its default: switch-to-buffer-other-window .

Desktop (please complete the following information):

  • code-review 20230724.2927 on Emacs 29.1
@SqrtMinusOne
Copy link

I fixed this like that:

(defun my/code-review-comment-quit ()
  "Quit the comment window."
  (interactive)
  (magit-mode-quit-window t)
  (with-current-buffer (get-buffer code-review-buffer-name)
    (goto-char code-review-comment-cursor-pos)
    (code-review-comment-reset-global-vars)))

(with-eval-after-load 'code-review
  (advice-add #'code-review-comment-quit :override #'my/code-review-comment-quit))

That way, C-c C-k behave the same as closing a magit-status buffer.

@aecepogluARUP
Copy link
Author

So is it the intended behaviour or not? If not, then we can just open a PR to address it. If it is, then it would amke sense to patch it on my end like in your example @SqrtMinusOne

@SqrtMinusOne
Copy link

I think it should be up to the user to decide, but I wouldn't say the current solution is strictly wrong.

Feel free to make a PR if you want.

It's just that the repo has been inactive for the last two years, the package is pretty huge, it would take quite an amount of work to make the UX as good as Magit and I'm not sure if it's worth it. And I suspect the same is the case for the author.

I'll probably just target GitLab with more limited scope if I ever try to write something like that.

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

No branches or pull requests

2 participants