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

Vimmier mappings #54

Closed
clason opened this issue Mar 14, 2021 · 27 comments
Closed

Vimmier mappings #54

clason opened this issue Mar 14, 2021 · 27 comments
Labels
enhancement New feature or request

Comments

@clason
Copy link

clason commented Mar 14, 2021

I apologize for the click-baity issue title...

My request is quite specific: Right now, the commit window uses the Emacs mapping <c-c><c-c> to save the commit message and perform the commit. Coming from vim-fugitive, I just can't add this to my muscle memory -- I always try to do this with the standard vim mapping :wq (like in vim-fugitive).

Can this be (optionally) supported?

@TimUntersberger
Copy link
Collaborator

Keybindings definitly still have to be adjusted. Initially I just copied all of the keybindings from magit to not have to think about this.

Automatically commiting when using :wq is quite tricky, but I'll try to see how we could do this.

@clason
Copy link
Author

clason commented Mar 14, 2021

I think vim-fugitive's implementation is here:
https://github.com/tpope/vim-fugitive/blob/36f9211da2e07c5c40d378cdf6360e6abd9495ac/autoload/fugitive.vim#L4687

Whether anyone but tpope can understand it is another question...

@clason
Copy link
Author

clason commented Mar 14, 2021

Maybe could be done with BufWritePost/BufUnload/BufDelete/QuitPre autocommands?

The big question would be how to distinguish :q[!] from :wq.

@TimUntersberger
Copy link
Collaborator

The big question would be how to distinguish :q[!] from :wq.

That is also why I didn't implement this in the beginning. I think there should be a way to do this.

@RianFuro
Copy link
Contributor

Not quite. What they do is, they set the $GIT_EDITOR env var to this:
https://github.com/tpope/vim-fugitive/blob/36f9211da2e07c5c40d378cdf6360e6abd9495ac/autoload/fugitive.vim#L2633-L2638

This is basically just a script that only exits when a certain $FUGITIVE.exit file is written OR a $FUGITIVE.edit file no longer exists, both of which i believe happen here:
https://github.com/tpope/vim-fugitive/blob/36f9211da2e07c5c40d378cdf6360e6abd9495ac/autoload/fugitive.vim#L2427-L2453

The .exit seems to handle aborting when vim is exited, while the .edit file looks like a file to synchronize asynchronous jobs over in general, where the commit dialog is one of them.

@RianFuro
Copy link
Contributor

RianFuro commented Mar 14, 2021

On that note, I actually prefer the way magit does it's... magic... in that regard. They use emac's remote capabilities to set $GIT_EDITOR to spawn an emacsclient that connects back to the running instance, opening the buffer to .git/COMMIT_EDITMSG there.

Now, vim has a very similar feature (check :help client-server), however most of the documentation seems to be outdated for neovim (--remote is not a valid option to nvim) and while there are RPC clients for ruby, python, js and whatnot, the internal lua api vim.api does not give remote capabilities.

The best shot for this, sans writing a client in a different language, seem to be a set of remote_ vimscript functions.

@clason
Copy link
Author

clason commented Mar 14, 2021

On that note, I actually prefer the way magit does it's... magic... in that regard. They use emac's remote capabilities to set $GIT_EDITOR to spawn an emacsclient that connects back to the running instance, opening the buffer to .git/COMMIT_EDITMSG there.

Now, vim has a very similar feature (check :help client-server), however most of the documentation seems to be outdated for neovim (--remote is not a valid option to nvim)

No, but nvr (https://github.com/mhinz/neovim-remote) is a drop-in replacement and the official way until --remote is re-implemented in the future. (I believe that's what fugitive uses for neovim?)

@TimUntersberger
Copy link
Collaborator

@clason I don't think we should depend on an external executable for this. Requiring users to install a separate program just feels wrong.

@RianFuro
Copy link
Contributor

I know about nvr; I'm not entirely against using it, but pulling in a non-plugin dependency and having a transient dependency on python as well "just for a nicer keybinding" seems awkard at best.

That being said, since the original topic is about

The big question would be how to distinguish :q[!] from :wq.

Do we have to at all? I mean, all the solutions we discussed basically come down to "let git make the decision". But git does not care how the file was saved, afaik it just checks if there is content; the commit is aborted only if there is no commit message.
I think we could just keep our approach of managing the commit buffer ourselves and implement the same logic.

@TimUntersberger
Copy link
Collaborator

We could make it so that writing the buffer commits the changes.

I don't think you would write the buffer if you don't want to commit with the provided message anyways right?

Example:

:wq commits
:w commits
:q aborts the commit

@clason
Copy link
Author

clason commented Mar 14, 2021

@clason I don't think we should depend on an external executable for this. Requiring users to install a separate program just feels wrong.

I don't know; it's an official neovim recommendation to install this, and other plugins (like vimtex) that need remote capabilities are happy to require it. So I don't see this as a big deal.

But if you don't need it, even better, of course!

@RianFuro
Copy link
Contributor

RianFuro commented Mar 14, 2021

We could make it so that writing the buffer commits the changes.

So basically we commit on BufWritePost? Could be a bit counter-intuitive, but I can definitely see it work if we communicate that clearly enough.

@clason
Copy link
Author

clason commented Mar 14, 2021

That was my initial thought as well, but then I thought "maybe people have muscle memory and :w partial messages expecting to be able to continue editing before committing?"

@clason
Copy link
Author

clason commented Mar 14, 2021

If you do that, you could also wipe the buffer on :w so that people don't keep editing a message after it's been committed ;)

@RianFuro
Copy link
Contributor

RianFuro commented Mar 14, 2021

maybe people have muscle memory and :w partial messages expecting to be able to continue editing before committing?

Yeah I'm worrying about that too; I know I'm one of those people 😅 Personally I'd just commit on BufUnload and abort the commit if the commit message is still empty at that point.
Now that I think about it, if we want to be fancy, we could just set a flag on the buffer at BufWritePost so we know for certain the buffer has been written to at BufUnload. (We might not want to do that when operating with a pre-filled commit message; think amending or reword)

@clason
Copy link
Author

clason commented Mar 14, 2021

That was my thought as well; set a flag on BufWritePost, check that on BufUnload to either go ahead with or abort the commit.

The only question is whether BufWritePost fires on :wq before BufUnload (which it should, right?)

@clason
Copy link
Author

clason commented Mar 14, 2021

(Leaves open the possibility that someone writes some text, :ws, then changes their mind and spams undo followed by :q...)

@RianFuro
Copy link
Contributor

RianFuro commented Mar 14, 2021

The only question is whether BufWritePost fires on :wq before BufUnload (which it should, right?)

From the docs:

BufUnload			Before unloading a buffer, when the text in 
				the buffer is going to be freed.
				After BufWritePost.
				Before BufDelete.

@TimUntersberger
Copy link
Collaborator

This is now supported in the newest version.

@TimUntersberger
Copy link
Collaborator

Closing this because I think this issue is done.

@kevintraver
Copy link

Is there a way to remap keys in the NeogitCommitPopup?

@TimUntersberger
Copy link
Collaborator

@kevintraver you could write a filetype plugin.

What do you want to remap?

@kevintraver
Copy link

kevintraver commented Nov 18, 2021

It would be cool if mapped to Commit (c)

@TimUntersberger
Copy link
Collaborator

@kevintraver you can remap c to <c-c><c-c> in the filetype plugin.

@kevintraver
Copy link

Awesome! Thanks

@kevintraver
Copy link

Another question: Is it possible to easily jump between staged / unstaged? Is there a key command for this?

@TimUntersberger
Copy link
Collaborator

You could try to use { and } of vim itself, but I don't think we support this yet.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

4 participants