fix: compatibility issue with noice

[ttytm]

This PR fixes some issues that are related to using the "BufUnload" event as trigger for the commit action.

It adds a dedicated action for sending a commit. This allows for things like checking the status of the confirmation first and then processing further logic (like sending a close request to the buffer). Instead of having all the logic bound to when the buffer is closed. This simplifies and allows more control over what is happening.

The cost of this is that a commit needs to be sent explicitly instead by closing the buffer e.g. :wq. This explicit way is also the way magit is doing it #441 (comment).

The action is mappable and defaults to <leader>c. (A user can assign multiple mappings, for me personally this also contains ;q which I use as my usual "quick-confirm-quit" mapping instead of :wq). And while adding a "commit" related mapping table, the ability to customize the "close" mapping - which is currently hard coded to "q" - was added as well.

mappings = {
  commit = {
    close = { "q" },
    send = { "<leader>c" },
  },
  ...
}

Last, the setting disable_commit_close_on_deny was added. It allows to configure if the commit buffer should stay open when a user denies the confirmation dialog of a commit send action. To not deprecate / break anything, it follows the current scheme of options. Personally I would prefer:

commit_confirmation = {
  enabled = true,
  close_on_deny = false,
}
-- instead of
disable_commit_confirmation = false,
disable_commit_close_on_deny = false,

Let me know how it works for you and if any changes are desired.

---

edit(2023.07.15): update description after rebase on master

[abusch]

I haven't tried this yet, but I'm curious: what happens if I do :wq instead of <leader>c?

[ttytm]

It would close the commit split without committing.

The committing action must be triggered before the close event if a confirmation dialog is to work as a popup.
At least this was the only option I could see without a bigger architectural change. That's why this adds the dedicated action for sending commits, hoping that it is not to far from the spirit of magit.

To give an insight from my personal config:
;q and ;w is mapped to speed up their command line equivalents (; still triggers a repeat after timeoutlen if one would use it for f,t, etc.).

-- Quick Commands
map("n", ";w", "<Cmd>w<CR>")
map("n", ";q", "<Cmd>confirm quit<CR>")

For neogit now i have mapped ;q to the send commit action. Which sends the commit and closes the buffer after confirmation.

[abusch]

FWIW, this is also the approach magit uses:

C-c C-c (with-editor-finish)
Finish the current editing session by returning with exit code 0. Git then creates the commit using the message it finds in the file.

C-c C-k (with-editor-cancel)
Cancel the current editing session by returning with exit code 1. Git then cancels the commit, but leaves the file untouched.

[gegoune]

Not sure what consensus and being true magit-way is but I liked :wq. Just an opinion.

[tilupe]

Any update here?

[wrightjjw]

This additionally looks like it could be a solution to our discussion in #552. It does change the core functionality of :wq... not a fan tbh. maybe a community poll to see how people would feel about that?

[CKolkey]

I've been thinking about it, and seeing as noice is explicitly experimental and says up front that stuff might break... going out of our way to support it seems silly.

[ttytm]

@wrightjjw yes, it would solve it. Using this branch since months, it also solves the linked commit amend issue. Also @CKolkey's reasoning that noice is experimental is valid.

So I wouldn't sell this PR anymore as fixing noice mainly, but the other issues and compatibility noice gets fixed along the way.

I'll rebase it on current master to get all the sweet recent commits and re-author the description to be a little easier to follow. Maybe a community poll would still be a thing, but deciding that is beyond my powers.

[CKolkey]

If it solves the amend issue, then that is something I'd say is worth looking into.

[ttytm]

@CKolkey any update in regards of merging this change? I'm still relying on the branch and would work through the merge conflicts to make it ready.

[CKolkey]

So, I think the biggest issue I have with this is that it changes the default experience so a user can't just save the buffer and close it. I don't have a lot of time right now to think about this, but two things come to mind:

1. BufWritePost autocmd to invoke the "send" action
2. use a conditional to check for noice integration, and disable the BufUnload autocmd if it's present.

[ttytm]

So, I think the biggest issue I have with this is that it changes the default experience so a user can't just save the buffer and close it. I don't have a lot of time right now to think about this, but two things come to mind:

1. BufWritePost autocmd to invoke the "send" action
2. use a conditional to check for noice integration, and disable the BufUnload autocmd if it's present.

BurwritePost would also change default behavior. Personally I wouldn't want a send action to be invoked after it.

No. 2 is genius I think.

[CKolkey]

I think this is addressed here: #1038

No more confirmations in the autocmd. Thanks for the ideas and discussion.

[ttytm]

@CKolkey thanks for the work. I tried the changes. Cancelling an amend will still send a commit.

Repor:

• ca in the status buffer to amend.
• Change something in the commit message - don't save the buffer.
• q to quit. Hit no in the saves changes confirmation dialog.
• See the commit sha changing.

This was not the case with the PR. So at neogits current master, when something should not be amended after opening the amend dialog, the workaround again would become to close nvim 😅

Also, when switching tabs and closing a tab that has a neogit buffer open, commits are made without confirmation. So for me the changes in the PR are still required to be able to use neogit more reliable.

[CKolkey]

Ahh, yeah, 'Close' isn't going to abort. Only 'Abort' (default c-c c-k, like Magit) will do that. I kept Close around to not break people's workflows too much, but I think you're better off with the more explicit commands. Do you think thats reasonable?

But I do take your point for something like amend or reword. Ill think about this. But noice is playing nicely?

[ttytm]

Yep it's playing nicely with noice now. Good work, Thank you!

Definitely agreed that it's good to have things explicit. Though it's a bug to commit when there is a dialog that asks to confirm changes -> a user denies it -> but the commit is made no matter what.

[CKolkey]

Alright, thats a good point.