vim-fugitive icon indicating copy to clipboard operation
vim-fugitive copied to clipboard

Add fixup/squash keybindings when at blame window

Open lieryan opened this issue 5 years ago • 4 comments

This allows you to use the the keybinding cf, cs, cA to create a fixup/squash commits for the selected line in :Gblame window, with similar semantic to :Gstatus window.

I find this very useful because a lot of the time the lines around the lines you are fixing up often were modified in the commit that you want to update.

lieryan avatar Oct 17 '19 15:10 lieryan

There is also cF and cS – any reason to skip them?

odnoletkov avatar Oct 17 '19 21:10 odnoletkov

If we do this, it should be all commit maps, and then the docs can just link to fugitive_c.

Calling :Gcommit from a blame splits the blame window to edit the commit message. This is not an issue for cf and cs but is problematic for cA and most of the other maps.

tpope avatar Oct 17 '19 22:10 tpope

Thanks @odnoletkov, @tpope for getting back to me.

I've updated the pull request to add cF and cS mapping as well as all of the fugitive_c, fugitive_cr, and fugitive_cm mappings where it makes sense.

I noticed that cS (squash+immediately rebase) does not seem to work for some reason, even in :Gstatus window, it doesn't apply the staged changes to the selected commit. I've included the fix for that on this PR, let me know if I should create a separate PR for it instead.

Another mapping that doesn't seem to work is cr<CR> (immediate? revert), it currently simply returns the git revert's help message. I can't think of a sensible interpretation of this command and it's undocumented. Maybe it can be rewritten to map to the same command as cr<Space> or simply removed?

Calling :Gcommit from a blame splits the blame window to edit the commit message

Hmm, there are a few possible way I can think of to avoid that:

  1. force the commit message's editing window to always open at :topleft (like :Gstatus). This has the drawback that the user may have to spend a few extra steps to return to the window that they were previously working on after finishing the commit message.
  2. move the cursor back to the edited file before opening the comand line (this would be somewhat buggy if the file to the right of the blame window is no longer the edited file)
  3. recommend people to set minwinwidth/winwidth if this bothers them

I've updated the PR to implement (1) for now. Thoughts?

lieryan avatar Oct 20 '19 07:10 lieryan

I noticed that cS (squash+immediately rebase) does not seem to work for some reason, even in :Gstatus window, it doesn't apply the staged changes to the selected commit. I've included the fix for that on this PR, let me know if I should create a separate PR for it instead.

I don't know how this happened. Did the --squash default behavior change from --no-edit to --edit at some point? Quite possible I just made a bad assumption. I've gone ahead and fixed this on both cs and cS; thanks for the heads up.

Another mapping that doesn't seem to work is cr<CR> (immediate? revert), it currently simply returns the git revert's help message. I can't think of a sensible interpretation of this command and it's undocumented. Maybe it can be rewritten to map to the same command as cr<Space> or simply removed?

It's for symmetry with the other <CR> maps. Might get tweaked later but for now don't worry about it.

Calling :Gcommit from a blame splits the blame window to edit the commit message

Hmm, there are a few possible way I can think of to avoid that:

  1. force the commit message's editing window to always open at :topleft (like :Gstatus). This has the drawback that the user may have to spend a few extra steps to return to the window that they were previously working on after finishing the commit message.
  2. move the cursor back to the edited file before opening the comand line (this would be somewhat buggy if the file to the right of the blame window is no longer the edited file)
  3. recommend people to set minwinwidth/winwidth if this bothers them

I've updated the PR to implement (1) for now. Thoughts?

Best would be (2). Doesn't need to be buggy; something similar to s:BlameLeave() would work. I wouldn't reject (1) in theory, but copying, pasting, and slightly changing 16 lines of maps is a hard deal breaker.

I'm also fine to ignore it for now if you leave out the explicit documentation.

tpope avatar Oct 20 '19 19:10 tpope