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

feat(actions): Automatic inline diff preview on hover #817

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

lnc3l0t
Copy link

@lnc3l0t lnc3l0t commented Jun 16, 2023

Introducing the ability to preview inline diffs when hovering over a hunk that has been added, modified, or removed.

To activate this functionality, you can either call the Gitsigns automatic_hunk_inline function directly to toggle it or have it by default in a new buffer by enabling Config.automatic_hunk_inline option.

When the cursor is placed on a hunk, the preview_hunk_inline function will be triggered automatically. Notably, if the cursor moves within the hunk's boundaries, the extmark (marker) will not flicker. However, if the cursor exits the hunk, the extmarks will be cleared automatically.

This behavior is achieved through an autocommand that listens for the CursorMoved event. Additionally, the implementation takes into consideration the _inline2 variant.

By calling `Gitsigns automatic_hunk_inline` or by enabling
`Config.automatic_hunk_inline` when the cursor is on an hunk (added,
modified or removed) `preview_hunk_inline` is called automatically.
If the cursor moves inside the hunk's bounds the extmark doesn't
flicker, if it exits the hunk the extmarks are cleared automatically.
It works via an autocommand triggering on CursorMoved.
It takes `_inline2` into consideration.

`Config.automatic_hunk_inline` is an option to enable the functionality
by default on new buffers.
@lewis6991
Copy link
Owner

lewis6991 commented Jun 16, 2023

I'm not sure if this is necessary. inline_preview is already maintained if you navigate between hunks.

There is also show_deleted() which essentially does an inline diff for all hunks simultaneously.

@lnc3l0t
Copy link
Author

lnc3l0t commented Jun 16, 2023

I think there are may be the following benefits:

  • When calling Gitsigns preview_hunk_inline and moving inside the hunk you don't need to call it again and again (it basically prevents the clear_preview_inline on CursorMoved if still inside the hunk
  • User doesn't have to call Gitsigns preview_hunk_inline before navigating between hunks, if it is useful to him to have the extmarks always so that the hunk pops. Activating Gitsigns toggle_linehl may be too much since it highlights all the hunks
  • Most notably there won't be the following behaviour: when show_deleted is true and we are on a modified hunk and call Gitsigns preview_hunk_inline there will be 2 deleted extmarks one from the preview and one from toggle_deleted (maybe it is a bug and should be fixed but still)
    image

I understand it may not be that useful even if I find it so, thank you for considering it!

@lewis6991
Copy link
Owner

  • When calling Gitsigns preview_hunk_inline and moving inside the hunk you don't need to call it again and again (it basically prevents the clear_preview_inline on CursorMoved if still inside the hunk

In that case I think you'd be better off opening the full diffview or opening regular preview so you can navigate the hunk. Note that you cannot navigate deleted lines in inline preview.

Otherwise we could probably explore other ways to invoke clear_preview_inline rather than it being hardcoded to CursorMoved.

  • User doesn't have to call Gitsigns preview_hunk_inline before navigating between hunks, if it is useful to him to have the extmarks always so that the hunk pops. Activating Gitsigns toggle_linehl may be too much since it highlights all the hunks

They still have to call this new function though? I can't imagine anyone having this enabled by default. It's cool eye-candy, but I don't think it's that practical, especially since other features can already achieve roughly the same thing.

Most notably there won't be the following behaviour: when show_deleted is true and we are on a modified hunk and call Gitsigns preview_hunk_inline there will be 2 deleted extmarks one from the preview and one from toggle_deleted (maybe it is a bug and should be fixed but still)

This is something that can be fixed. Though I do plan on deprecating show_deleted and replacing it with something like inline_preview_all, or some alternative to a split diff.

Can you not get a similar effect of this by just calling gitsigns.preview_hunk_inline() in a CursorMoved autocmd in your config?

@lnc3l0t
Copy link
Author

lnc3l0t commented Jun 17, 2023

Note that you cannot navigate deleted lines in inline preview

This would be cool. Wouldn't it be possible with _inline2 since it opens a new window?

Otherwise we could probably explore other ways to invoke clear_preview_inline rather than it being hardcoded to CursorMoved

I will think about a solution, though I think the cursor movement is the primary reason for wanting to clear the preview. I don't think making the user call the clear themself is a good solution maybe a better idea will come up.

They still have to call this new function though?

They would only need to do it once per buffer or through the option. (Perhaps nobody would want it enabled by default idk)

Though I do plan on deprecating show_deleted and replacing it with something like inline_preview_all

This would be interesting for me I think

I tried with the CursorMoved autocommand in the config.on_attach but:

vim.api.nvim_create_autocmd("CursorMoved", {
    buffer = 0,
    command = "Gitsigns preview_hunk_inline"
})
  • moving the cursor inside the hunk makes the hunk preview disappear which was my main concern
  • it flickers a lot:
flickering.mp4

I understand your concerns against my PR. Thank you once again for considering it, I genuinely enjoy your plugins, have a nice day

@lewis6991
Copy link
Owner

I might be a bit more open to having this as an option to the eventual inline_preview_all.

I think it's mostly the API surface area and generally how the feature is presented is my main concern.

If it can integrate better with existing (or future in this case) features, then that's an easier sell.

So the answer is not "no", but we'll need to sit on this for a while. And given how I'm stretched across nvim core, it could be a while. Hope that's ok.

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

Successfully merging this pull request may close these issues.

2 participants