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

Ability to swap diffthis panes #585

Closed
jamischarles opened this issue Jun 20, 2022 · 17 comments · Fixed by #587
Closed

Ability to swap diffthis panes #585

jamischarles opened this issue Jun 20, 2022 · 17 comments · Fixed by #587
Labels
enhancement New feature or request

Comments

@jamischarles
Copy link

jamischarles commented Jun 20, 2022

LOVING gitsigns. Seriously great work!

Is your feature request related to a problem? Please describe.
I use the diffthis view frequently. I always have to swap the panes because I expect the left pane to be the current buffer and the right pane to contain the base to compare against.

Describe the solution you'd like
I'd love if we could pass in a config value for swapping the pane order.

Describe alternatives you've considered

  1. New command:
local gs = require("gitsigns")
vim.api.nvim_create_user_command(
    'Diff',
    function()
      gs.diffthis('~') -- call same as :Gitsigns diffthis
      feedkeys("<C-W><C-x>", "m") -- Swap the panes
    end,
    { nargs = 0 }
)

I really thought this would work, but it doesn't swap the panes. I'm guessing it gets called too soon and I need to delay.

  1. BufEnter and when it enters the gitsigns diff url, then run the "swap panes" command then.

Keep up the great work!

@jamischarles jamischarles added the enhancement New feature or request label Jun 20, 2022
@lewis6991
Copy link
Owner

lewis6991 commented Jun 20, 2022

Yes, diffthis is asynchronous which means the call is non-blocking and doesn't finish until a few main loop iterations later.

One of the current goals is to add support for command modifiers to the Gitsigns command so we can do something like :botright Gitsigns diffthis. At the same time, anything a command can do, the Lua function should be able to do too.

For now we can add an option to diffthis:

diffthis({base}, {opts})                                         *gitsigns.diffthis()*

Where setting something like opts.botright = true will provide the behaviour you want.

In addition, we should also respect the 'splitright' option.

@jamischarles
Copy link
Author

@lewis6991 Thank you!

I pulled down the latest and it works 🎉 !

Took me some trial error to realize I needed to set
vim.o.splitright = true in my lua config file.

I did try variations like these:
gs.diffthis('~', {splitright = true, botright=true})
and

require('gitsigns').setup {
  sign_priority = 20,
  diff_opts = {
    splitright = true,
    botright = true,
  }

Works great now! Thanks!

@lewis6991
Copy link
Owner

lewis6991 commented Jun 20, 2022

I did try variations like these:
gs.diffthis('~', {splitright = true, botright=true})
and

require('gitsigns').setup {
  sign_priority = 20,
  diff_opts = {
    splitright = true,
    botright = true,
  }

Hmm, this shouldn't do anything. The only option pulled out of diff_opts here is vertical.

The change the makes it so diffthis respects 'splitright'.

lewis6991 added a commit that referenced this issue Jun 20, 2022
When running an action via the :Gitsigns command, the internal plumbing
can now pass the entire command context/options directly to a
specialised action command (per action) that knows how to translate
command parameters to the specific action parameters.

Example #1: `:belowright Gitsigns diffthis` now translates to
`diffthis(nil, {split='belowright'}`.

Example #2: `:6,10Gitsigns stage_hunk` now translates to
`stage_hunk({6,10})`.

Note this change reverts the behaviour of the previous commit that
respects 'splitright'

Fixes #585
@lewis6991
Copy link
Owner

Ok, I worked on this a little and decided to revert the behaviour introduced in #586.

Now diffthis contains an opts argument:

diffthis({base}, {opts})                                 *gitsigns.diffthis()*
                Perform a |vimdiff| on the given file with {base} if it is
                given, or with the currently set base (index by default).

                If {base} is the index, then the opened buffer is editable and
                any written changes will update the index accordingly.

                Parameters: ~
                    {base}   (string|nil): Revision to diff against. Defaults
                             to index.
                    {opts}   (table|nil):
                             Additional options:
                             • {vertical}: {boolean}. Split window vertically. Defaults to
                             config.diff_opts.vertical. If running via command line, then
                             this is taken from the command modifiers.
                             • {split}: {string}. One of: 'aboveleft', 'belowright',
                             'botright', 'rightbelow', 'leftabove', 'topleft'. Defaults to
                             'aboveleft'. If running via command line, then this is taken
                             from the command modifiers.

Additionally if using via the :Gitsigns command, modifiers like rightbelow etc will now be respected.

E.g. :rightbelow Gitsigns diffthis

lewis6991 added a commit that referenced this issue Jun 20, 2022
When running an action via the :Gitsigns command, the internal plumbing
can now pass the entire command context/options directly to a
specialised action command (per action) that knows how to translate
command parameters to the specific action parameters.

Example #1: `:belowright Gitsigns diffthis` now translates to
`diffthis(nil, {split='belowright'}`.

Example #2: `:6,10Gitsigns stage_hunk` now translates to
`stage_hunk({6,10})`.

Note this change reverts the behaviour of the previous commit that
respects 'splitright'

Fixes #585
lewis6991 added a commit that referenced this issue Jun 20, 2022
When running an action via the :Gitsigns command, the internal plumbing
can now pass the entire command context/options directly to a
specialised action command (per action) that knows how to translate
command parameters to the specific action parameters.

Example #1: `:belowright Gitsigns diffthis` now translates to
`diffthis(nil, {split='belowright'}`.

Example #2: `:6,10Gitsigns stage_hunk` now translates to
`stage_hunk({6,10})`.

Note this change reverts the behaviour of the previous commit that
respects 'splitright'

Fixes #585
@jamischarles
Copy link
Author

Perfect! Thanks!!

@jamischarles
Copy link
Author

@lewis6991 Did some quick testing of all the options:

Works perfectly 🎉

local gs = require("gitsigns")
gs.diffthis('~', {split = "botright"}

Results in bug

require('gitsigns').setup {
  diff_opts = {
    split = "botright"
  }
}
-- same bug
:botright Gitsigns diffthis

Here's the bug:
Screen Shot 2022-06-20 at 9 34 44 AM

@lewis6991
Copy link
Owner

require('gitsigns').setup {
  diff_opts = {
    split = "botright"
  }
}

This isn't supported, split can only be passed to diffthis(). See :help gitsigns-config-diff_opts

@lewis6991
Copy link
Owner

Also there was a typo in the code which I have fixed with 618be8e

@jamischarles
Copy link
Author

@lewis6991 Understood!
Awesome!

I'll test :botright Gitsigns diffthis again when the commit is on main (didn't seem to work on cmdarg branch because a commit from main is missing)

@lewis6991
Copy link
Owner

Ah, I committed to the wrong branch. Should be on main now.

@jamischarles
Copy link
Author

Aaaaalmost there lol. Thank you, hopefully this is helpful 😄 .

That removes the bug, though the swap doesn't happen for these 2 use cases:
:botright Gitsigns diffthis
:Gitsigns diffthis ~ {'split': 'botright'}

With either of these the split happens in the original layout.

@lewis6991
Copy link
Owner

Thanks for the feedback. I found another issue and raised #588.

:Gitsigns diffthis ~ {'split': 'botright'} isn't supported. It should be :Gitsigns diffthis ~ split=botright. Note I'm still working on the interface here so it isn't fully documented.

:botright Gitsigns diffthis and :Gitsigns diffthis ~ split=botright now both work for me.

@jamischarles
Copy link
Author

@lewis6991
Nice! I pulled down that branch.

This now works 🎉
:Gitsigns diffthis ~ split=botright

this doesn't
:botright Gitsigns diffthis

@lewis6991
Copy link
Owner

What version of neovim are you running?

@jamischarles
Copy link
Author

jamischarles commented Jun 20, 2022 via email

@lewis6991
Copy link
Owner

Ok, you might need 0.8dev for that specific form to work.

You no longer need to set splitright

@jamischarles
Copy link
Author

Ok. Sounds good.

I'll test that out later when 0.8 is stable. I'll use one of the options that works for now.

Thank you!

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
2 participants