echasnovski / mini.nvim

Library of 40+ independent Lua modules improving overall Neovim (version 0.8 and higher) experience with minimal effort
MIT License
5.16k stars 187 forks source link

Beta-testing 'mini.diff' #773

Closed echasnovski closed 4 months ago

echasnovski commented 6 months ago

Please leave your feedback about new mini.diff module here. Feel free to either add new comment or positively upvote existing one.

Some things I am interested to find out (obviously, besides bugs):

Thanks!

bdillahu commented 6 months ago

Seems good and works well... config for 'signs' or 'numbers' is a bit confusing on what the syntax is supposed to look like when combined with the vim.o.number text already there...

echasnovski commented 6 months ago

Seems good and works well... config for 'signs' or 'numbers' is a bit confusing on what the syntax is supposed to look like when combined with the vim.o.number text already there...

I am not sure I understand the issue. Currently default style is "number' if line number are enabled at the time of require('mini.diff').setup() and 'sign' otherwise. If style is 'sign', actual sign text can be configured in view.signs. Hope it helps.

bdillahu commented 6 months ago

I understand I believe, but what syntax to use is unclear to me.

This is in the config information:

    -- Visualization style. Possible values are 'sign' and 'number'.
    style = vim.o.number and 'number' or 'sign',

do I put:

    style = vim.o.number and 'sign',

or

    style = 'sign',

to use the sign? (trial and error has it working with the first of my options, but in the spirit of beta feedback on the docs, I thought I would point out where I found it confusing.

echasnovski commented 6 months ago

I understand I believe, but what syntax to use is unclear to me.

If you want to always use 'sign', then:

require('mini.diff').setup({ view = { style = 'sign' } })

Conversely, if you want to always use 'number', then:

require('mini.diff').setup({ view = { style = 'number' } })
231tr0n commented 6 months ago

Hi, and as always i love your plugins. I just tried it out and Hunk staging is not working for me. I am able to reset hunks though. Is there anything specific i have to look out for. Also resetting hunks saves my buffer and writes to it after resetting. Is there anyway i can prevent that by only resetting and not saving the file.

echasnovski commented 6 months ago

Hi, and as always i love your plugins.

:heart:

I just tried it out and Hunk staging is not working for me. I am able to reset hunks though. Is there anything specific i have to look out for.

Sigh... If visualization is done properly, then it will be not easy to debug. If that is the case, my guess is that there is an error during computing or applying patch which gets silently ignored (because I assume/hope that the code will always work). So chances are, there is an error somewhere here. Do you have time/energy/enthusiasm to see where the issue occurs exactly?

First, I'd start with asking what Git version do you have? Second, I'll add some logging behavior here and possibly here to see if there is a non-zero exit code (signalling that there were some error).

Also resetting hunks saves my buffer and writes to it after resetting. Is there anyway i can prevent that by only resetting and not saving the file.

Initially I did not like that it makes the buffer modified and prompts a save. Besides, 'gitsigns.nvim' seems to also not make buffer modified when resetting buffers. But you know what... After you mentioned it, it is a text change which should make buffer be modified. I'll run locally with it reverted to see if there are any other issues.

231tr0n commented 6 months ago

Hi, and as always i love your plugins.

❤️

I just tried it out and Hunk staging is not working for me. I am able to reset hunks though. Is there anything specific i have to look out for.

Sigh... If visualization is done properly, then it will be not easy to debug. If that is the case, my guess is that there is an error during computing or applying patch which gets silently ignored (because I assume/hope that the code will always work). So chances are, there is an error somewhere here. Do you have time/energy/enthusiasm to see where the issue occurs exactly?

First, I'd start with asking what Git version do you have? Second, I'll add some logging behavior here and possibly here to see if there is a non-zero exit code (signalling that there were some error).

Also resetting hunks saves my buffer and writes to it after resetting. Is there anyway i can prevent that by only resetting and not saving the file.

Initially I did not like that it makes the buffer modified and prompts a save. Besides, 'gitsigns.nvim' seems to also not make buffer modified when resetting buffers. But you know what... After you mentioned it, it is a text change which should make buffer be modified. I'll run locally with it reverted to see if there are any other issues.

Hi yeah i am intrested in debugging this out. This is my git version git version 2.34.1 Let me know what else i should be doing Also just to mention i use delta as my pager to view git diffs if that matters here.

echasnovski commented 6 months ago

git version 2.34.1

That should be enough. It looks like it does not support --format option in git ls-files. To double check, would you mind executing the following command from the root of some repo (replace path/to/some/file with actual path to some file inside a repo):

git ls-files --full-name --format="%(objectmode) %(path)" -- path/to/some/file

According to Git documentation, the --format option appeared in 2.38.0, which was released in October 2022. So this command should show an error for you, correct?

I presume you use the one which comes in Ubuntu 22.04? For the time being, updating version of Git should fix things.

231tr0n commented 6 months ago

git version 2.34.1

That should be enough. It looks like it does not support --format option in git ls-files. To double check, would you mind executing the following command from the root of some repo (replace path/to/some/file with actual path to some file inside a repo):

git ls-files --full-name --format="%(objectmode) %(path)" -- path/to/some/file

According to Git documentation, the --format option appeared in 2.38.0, which was released in October 2022. So this command should show an error for you, correct?

I presume you use the one which comes in Ubuntu 22.04? For the time being, updating version of Git should fix things.

yes you are right i am using ubuntu 22.04 in wsl2. Will try updating git first and try staging hunks again.

231tr0n commented 6 months ago

git version 2.34.1

That should be enough. It looks like it does not support --format option in git ls-files. To double check, would you mind executing the following command from the root of some repo (replace path/to/some/file with actual path to some file inside a repo):

git ls-files --full-name --format="%(objectmode) %(path)" -- path/to/some/file

According to Git documentation, the --format option appeared in 2.38.0, which was released in October 2022. So this command should show an error for you, correct?

I presume you use the one which comes in Ubuntu 22.04? For the time being, updating version of Git should fix things.

Yup updating git version fixes the issue. You were right. I added this ppa:git-core/ppa and updated git and staging works now.

But how do i unstage a staged hunk. Like i tried gH and it does not work. It throws out an error saying no hunks found. Also no special colors are shown for CursorLineNr for a staged hunk to identify and unstage it.

echasnovski commented 6 months ago

Yup updating git version fixes the issue. You were right. I added this ppa:git-core/ppa and updated git and staging works now.

Glad it works now!

But how do i unstage a staged hunk. Like i tried gH and it does not work. It throws out an error saying no hunks found. Also no special colors are shown for CursorLineNr for a staged hunk to identify and unstage it.

This is not supported. Mostly because I could not find a reasonable abstraction for this action which can be applied for any source of reference text.

231tr0n commented 6 months ago

Yup updating git version fixes the issue. You were right. I added this ppa:git-core/ppa and updated git and staging works now.

Glad it works now!

But how do i unstage a staged hunk. Like i tried gH and it does not work. It throws out an error saying no hunks found. Also no special colors are shown for CursorLineNr for a staged hunk to identify and unstage it.

This is not supported. Mostly because I could not find a reasonable abstraction for this action which can be applied for any source of reference text.

Yup cool thank you for this wonderful plugin once again.

Ernest1338 commented 6 months ago

Great plugin as always!

Couple of questions.

Your plugins has the vim.b.minidiff_summary_string which is the equivalent of vim.b.gitsigns_status in gitsigns.nvim. Is there a equivalent for vim.b.gitsigns_head to display current branch in the statusline?

Is the current line blame functionality out of the scope of this plugin?

echasnovski commented 6 months ago

Your plugins has the vim.b.minidiff_summary_string which is the equivalent of vim.b.gitsigns_status in gitsigns.nvim. Is there a equivalent for vim.b.gitsigns_head to display current branch in the statusline?

Is the current line blame functionality out of the scope of this plugin?

The answer to both of these is that it is planned as a part of 'mini.git'.

Having current head information provided by 'mini.diff' does not really align with its whole design of having configurable source. There is no reasonable abstraction for Git's HEAD in generic source of reference text.

bassamsdata commented 6 months ago

Continuing the discussion from Reddit since the code formatting here is much better:

Here's what I came up with for the branch name (I added a simple caching mechanism for performance improvements). However, I couldn't implement your second suggestion of using vim.schedule_wrap beforehand because it caused some issues, and I'm not entirely sure why. Here's a breakdown of the code and steps:

  1. Check if it's a valid file and if it's within a git repository before proceeding.
  2. Retrieve the branch name for the buffer and cache it.
  3. If the branch name is already cached, there's no need to call git again.
  4. Clear the cache if the directory changes.

I didn't expect to end up writing around 170 lines of code just because I switched from one plugin to another, but it was quite an interesting experience.

I have another question regarding mini.map, will MiniMap have something like gen_integration.minidiff similar to the functionality of gitsigns ?https://github.com/echasnovski/mini.nvim/blob/0ec3216d3224ebd6828e5637fa9a10a00ba67113/lua/mini/map.lua#L864

Code:

local function is_valid_git_repo(buf_id)
    -- Check if it's a valid buffer
    local path = vim.api.nvim_buf_get_name(buf_id)
    if path == "" or vim.fn.filereadable(path) ~= 1 then
        return false
    end

    -- Check if the current directory is a Git repository
    if vim.fn.isdirectory(".git") == 0 then
        return false
    end

    return true
end

local branch_cache = {}

-- Function to clear the Git branch cache
local function clear_git_branch_cache()
    -- Clear by doing an empty table :)
    branch_cache = {}
end

-- Autocommand to clear the Git branch cache when the directory changes
vim.api.nvim_create_autocmd("DirChanged", {
    callback = clear_git_branch_cache,
})

local function update_git_branch(data)
    if not is_valid_git_repo(data.buf) then
        return
    end

    -- Check if branch is already cached
    local cached_branch = branch_cache[data.buf]
    if cached_branch then
        vim.b.git_branch = cached_branch
        return
    end

    local stdout = vim.uv.new_pipe(false)
    local handle, pid
    handle, pid = vim.uv.spawn(
        "git",
        {
            args = { "-C", vim.fn.expand("%:p:h"), "branch", "--show-current" },
            stdio = { nil, stdout, nil },
        },
        vim.schedule_wrap(function(code, signal)
            if code == 0 then
                stdout:read_start(function(err, content)
                    if content then
                        vim.b.git_branch = content:gsub("\n", "") -- Remove newline character
                        branch_cache[data.buf] = vim.b.git_branch -- Cache the branch name
                        stdout:close()
                    end
                end)
            else
                stdout:close()
            end
        end)
    )
end

-- Call this function when the buffer is opened in a window
vim.api.nvim_create_autocmd("BufWinEnter", {
    callback = update_git_branch,
})

thank you

echasnovski commented 6 months ago

Here's what I came up with for the branch name (I added a simple caching mechanism for performance improvements). However, I couldn't implement your second suggestion of using vim.schedule_wrap beforehand because it caused some issues, and I'm not entirely sure why.

Thanks for posting it here!

I was wrong in suggesting that that function is better be extracted out of the call because it uses stdout, which is created on every vim.loop.spawn() call. Better formatting made me see this :)

I have another question regarding mini.map, will MiniMap have something like gen_integration.minidiff similar to the functionality of gitsigns ?

Yes, it is planned. Along with similar 'mini.statusline' updates.

echasnovski commented 6 months ago

@231tr0n, the latest main now treats resetting hunks as a regular text edit, i.e. it does not save file to preserve 'modified' (as was before). Also there is a clarification about Git minimum version 2.38.0 for applying/staging hunks. Thanks for the feedback!

GitMurf commented 6 months ago

Seems good and works well... config for 'signs' or 'numbers' is a bit confusing on what the syntax is supposed to look like when combined with the vim.o.number text already there...

For the record, I agree this was a bit confusing for me too. But I think one of the confusing parts was my own assumption that you could choose to do both sign and number at same time. So I was trying to understand how I could do that.

So my comment here is to 1) agree that it was a little confusing and 2) to request the ability to choose both line number coloring and signs at the same time (I like it to be super obvious haha).

Also as one more point of feedback on the confusion part... while I know a bit of Lua from hacking around, non technical people / non lua folks may be confused by the vim.go.number and 'number' or 'sign'.

GitMurf commented 6 months ago

@echasnovski initial feedback:

Overall, Awesome! I have been able to disable gitsigns because mini.diff can do most of the important things I cared about with gitsigns. The only 3 remaining items that I would love to see to make my "transition" complete are:

  1. As stated in my previous comment, be able to turn on both number and signs for the view style.
  2. Have a "hover" preview option to show a single hunk diff. Essentially show what the overlay shows but in a hover that I can quickly view without turning on the entire overlay feature on/off.
  3. A "rolling" goto meaning if I goto "next" and it hits the last hunk in my buffer, if i do next again it brings me back to the top to continue to cycle through. And same with reverse if I use previous to go to the very top and then have it cycle to the bottom in a looping fashion. Currently it sends a notification saying "no more hunks in that direction".

Again, awesome work! Everything else seems to be working very nicely.

echasnovski commented 6 months ago

@GitMurf, thanks for the feedback!

Seems good and works well... config for 'signs' or 'numbers' is a bit confusing on what the syntax is supposed to look like when combined with the vim.o.number text already there...

For the record, I agree this was a bit confusing for me too. But I think one of the confusing parts was my own assumption that you could choose to do both sign and number at same time. So I was trying to understand how I could do that.

So my comment here is to 1) agree that it was a little confusing and 2) to request the ability to choose both line number coloring and signs at the same time (I like it to be super obvious haha).

No, I don't think this is a reasonable feature. Although actual implementation is pretty straightforward, actual design of how users enable that will be somewhat confusing. It would have to be a set of booleans like "view.signs = true/falseandview.line_number = true/false`, which I don't like that much.

Also as one more point of feedback on the confusion part... while I know a bit of Lua from hacking around, non technical people / non lua folks may be confused by the vim.go.number and 'number' or 'sign'.

I can see that. That's why there is a comment above it: "Default: 'number' if line numbers are enabled, 'sign' otherwise.".

  1. Have a "hover" preview option to show a single hunk diff. Essentially show what the overlay shows but in a hover that I can quickly view without turning on the entire overlay feature on/off.

No, I don't see it be valuable enough to warrant separate functionality. Besides, going away from "preview single hunk" was one of the goals of 'mini.diff' for me: I didn't really like its workflow. So toggling overlay is the method to preview hunks in 'mini.diff'.

  1. A "rolling" goto meaning if I goto "next" and it hits the last hunk in my buffer, if i do next again it brings me back to the top to continue to cycle through. And same with reverse if I use previous to go to the very top and then have it cycle to the bottom in a looping fashion. Currently it sends a notification saying "no more hunks in that direction".

The wrapping around edges is a tough one. I'll duplicate my answer from Reddit:

I spent too much time weighing pros and cons of either wrap should be enabled by default and/or should it be configurable.

Recently I've changed my mind from "wrap is always better" to "wrap sometimes harms the workflow". In this particular instance I do believe that wrapping brings more harm than good. Mostly because it is usually a bit disorienting having to spend fraction of a second to deduce whether it was the last hunk or not. So there is no wrap by default.

Making it configurable is certainly possible, but it would need a slightly more complex and not as straightforward code as there is now.

So at least for the time being, there will be no configurable wrapping, sorry.

GitMurf commented 6 months ago

Thanks for your responses to my feedback.

As for the last item, wrapping around edges... would it be reasonable enough to have MiniDiff.goto() return a value? either true if successfull or false if "edge of file"? because then I could have my keymap be a function that when doing goto next, if it returns false, then I can do a goto "first" following it because I know I am at the bottom of the file and therefore rolling over would just be jumping to the first hunk. Does that make sense? thoughts?

echasnovski commented 6 months ago

As for the last item, wrapping around edges... would it be reasonable enough to have MiniDiff.goto() return a value? either true if successfull or false if "edge of file"? because then I could have my keymap be a function that when doing goto next, if it returns false, then I can do a goto "first" following it because I know I am at the bottom of the file and therefore rolling over would just be jumping to the first hunk. Does that make sense? thoughts?

It is already kind of possible with a more straightforward approach. It can be checked if cursor actually moved after goto_hunk('next'), i.e. 1) cache cursor position; 2) execute goto_hunk('next'); 3) check if cursor has changed and if not - do goto_hunk('first'). This will still produce a notification about no hunks in direction "next", but this might be a good thing.

231tr0n commented 6 months ago

@231tr0n, the latest main now treats resetting hunks as a regular text edit, i.e. it does not save file to preserve 'modified' (as was before). Also there is a clarification about Git minimum version 2.38.0 for applying/staging hunks. Thanks for the feedback!

Awesome thank you for the change.

pkazmier commented 6 months ago

Love the plug-in, specifically the overlay toggle. Two comments / questions for you:

echasnovski commented 6 months ago
  • Because the overlay is one of the coolest features of the plugin, I'm curious why you chose not to provide a mapping. The plug-in provides several other mappings. I mapped mine to \D as I use mini.basics and it fits with the rest of the other user toggle-able mappings defined there.

No objective reason per se. Just that this type of mapping for me fits more as the <Leader> mapping (I use <Leader>go), which is a no go as default mapping.

  • I rely on CursorLineNr (I bold mine) solely to identify the line number my cursor is currently on (cursorlineoptr = "number"), but when I use view = { style = "number" } }, there is no means to identify what line number I'm on anymore. Is there a way to prioritize CursorLineNr over MiniDiffSign* highlight groups?

Me too ('mini.hues' also uses bold), but I don't think there is a way to do this with current Neovim functionality. I'd expect setting the lowest priority for extmarks (in 'mini.diff' via config.view.priority) would solve the issue, but it does not. Integrating Vim's UI highlighting and extmark highlighting is hard.

Also, both cursor and cursorline should be enough to quickly identify where cursor is at.

That said, I think overriding is even not the best outcome here, as those should combine. I.e. line number of the current and added line should be bold green in this case. I have opened an issue in Neovim for that (exactly as the result of what we discuss here): neovim/neovim#27555.

Or, even better, any chance of getting MiniDiffSignAddNr, MiniDiffSignChangeNr, and MiniDiffSignDeleteNr highlight groups?

I am not sure why this is better or even that useful at all. Using MiniDiffSignXXX groups for both "sign" and "number" styles seems reasonable because in both cases the target is in the gutter (right hand are with line information) and mostly need to highlight the text.

lervag commented 6 months ago

Thanks for this plugin. I never used gitsigns or similar plugins before (didn't really think I needed them). But for some reason, I just tried this and I realize it is actually quite useful.

I've customized mini.diff to use these signs:

  view = {
    signs = {
      add = "▕▏",
      change = "▕▏",
      delete = "▁▁"
    },
  },

One thing I noticed: gitsigns.nvim have some additional signs that you did not implement yet. Specifically, the topdelete sign comes to mind. I think perhaps it makes sense to add that one as well to mini.diff, e.g.:

  view = {
    signs = {
      add = "▕▏",
      change = "▕▏",
      delete = "▁▁"
      topdelete = "▔▔"
    },
  },
echasnovski commented 6 months ago

One thing I noticed: gitsigns.nvim have some additional signs that you did not implement yet. Specifically, the topdelete sign comes to mind. I think perhaps it makes sense to add that one as well to mini.diff

Thanks for the suggestion!

This sign is only useful when first lines are deleted in order to be able to distinguish whether deleted lines were above the current first line or below. Other than that, "delete" sign always indicates that some text was deleted below the line with the visualization.

I think this is not that useful feature as it is usually either obvious from the current text or can be confirmed via overlay. So it was intentionally not added to reduce config options.

lervag commented 6 months ago

This sign is only useful when first lines are deleted in order to be able to distinguish whether deleted lines were above the current first line or below. Other than that, "delete" sign always indicates that some text was deleted below the line with the visualization.

I think this is not that useful feature as it is usually either obvious from the current text or can be confirmed via overlay. So it was intentionally not added to reduce config options.

I agree it's not very useful. But during testing, I realized it was quite annoying in the edge case where it is useful. I'll respect if you choose to not implement it, of course. But I thought it was useful to lift this as something I noticed as a beta tester. :)

bassamsdata commented 6 months ago

I'm experiencing an issue with the overlay feature where it doesn't work if I delete the first line of text in the buffer alone or with a paragraph below it. However, if I exclude the first line, then it works as expected. the video demonstrating the issue is below:

https://github.com/echasnovski/mini.nvim/assets/105807570/60722576-5688-4e18-bf49-29b9b5c58ed6

echasnovski commented 6 months ago

I'm experiencing an issue with the overlay feature where it doesn't work if I delete the first line of text in the buffer alone or with a paragraph below it. However, if I exclude the first line, then it works as expected. the video demonstrating the issue is below:

This is a current Neovim behavior of virtual lines being placed above the first line. This is documented and needs manual scroll with <C-y> for them to become visible. Unfortunately, there is no proper approach to handle this at the moment.

bassamsdata commented 6 months ago

got it, now that makes sense. <c-y> worked, thank you. I just remembered that I actually experienced a similar issue before because I use molten where virtual text below the last line won't appear unless more lines are added.

thanks again

pkazmier commented 6 months ago

I've noticed that signs/numbers are not shown correctly (or at all) for buffers opened that are not in a window when resuming a mksession via nvim -S. Buffers that are shown on-screen are rendered correctly, but if you switch buffers to one that is not currently in a window, its signs/numbers are not shown correctly (or at all). When I switch to a buffer where signs are incorrect, one must toggle the overlay to correct the state—a simple :edit does not suffice.

echasnovski commented 6 months ago

I've noticed that signs/numbers are not shown correctly (or at all) for buffers opened that are not in a window when resuming a mksession via nvim -S. Buffers that are shown on-screen are rendered correctly, but if you switch buffers to one that is not currently in a window, its signs/numbers are not shown correctly (or at all). When I switch to a buffer where signs are incorrect, one must toggle the overlay to correct the state—a simple :edit does not suffice.

Huh, indeed. I can reproduce. Thanks!

It seems like either buffer or reference text is not up to date for those once. I'll take a look.

Edit: the reason is that those buffers are not loaded (although exist), hence Neovim thinks that they are empty. The fix is to simply loading them inside setup(). I hope to fix this tomorrow.

echasnovski commented 6 months ago

@pkazmier, the issue of enabling hidden buffers from session should now be resolved on latest main.

pkazmier commented 6 months ago

@pkazmier, the issue of enabling hidden buffers from session should now be resolved on latest main.

Thank you. Confirmed on my side as well. Really enjoying the plug-in, toggling the virtual overlay is becoming a good part of my workflow versus having to preview individual hunks. I'm still running gitsigns though for mini.statusline, but I disabled its signs in favor of mini.diff.

echasnovski commented 6 months ago

Thank you. Confirmed on my side as well. Really enjoying the plug-in, toggling the virtual overlay is becoming a good part of my workflow versus having to preview individual hunks. I'm still running gitsigns though for mini.statusline, but I disabled its signs in favor of mini.diff.

Nice! There is a planned 'mini.git' for tracking Git data specifically and then rewrite of default content for 'mini.statusline' to prefer 'mini.git' and 'mini.diff'. I think I have most of design figured out and implementation should not be complicated.

GitMurf commented 6 months ago

This will still produce a notification about no hunks in direction "next", but this might be a good thing.

Good idea @echasnovski! I agree actually this is a nice thing to let me know it went from bottom to top essentially. Thanks!

Ernest1338 commented 6 months ago

Is it possible to configure the overlay to show the diff based on removed and added lines only? Without showing the differences between changed lines? So instead of this: Screenshot_20240401_183717 It would show something like this: Screenshot_20240401_183831

echasnovski commented 6 months ago

Is it possible to configure the overlay to show the diff based on removed and added lines only? Without showing the differences between changed lines?

Unfortunately, no, not really. Recognizing changed lines as much as possible and showing word diff right next to the changed line was pretty much the main drive for creating this overlay feature.

The best way to have this as less as possible is to disable line matching (if you are on Neovim>=0.9). So setting config.options.linematch = 0 should reduce the number of times the word diff is shown.

SanchayanMaity commented 6 months ago

Thank you for writing this module. Am moving from gitsigns and have two questions. Hope I am not missing anything. Below is the current change in configuration I have to enable this module.

require('mini.diff').setup({
  view = {
    style = 'sign',
    signs = { add = '+', change = '~', delete = '-' },
  },
  mappings = {
    textobject = 'gh',
    goto_prev  = '[c',
    goto_next  = ']c',
    goto_first = '<Leader>hf',
    goto_last  = '<Leader>hl',
    apply      = '<Leader>hs',
    reset      = '<Leader>hr',
  },
})

In gitsigns, with ih and map({'o', 'x'}, 'ih', ':<C-U>Gitsigns select_hunk<CR>'), doing vih will visually select the hunk. This is not possible with gh text object here?

Any plans on implementing cycling between hunks? Thought I did ask, not a problem though.

echasnovski commented 6 months ago

In gitsigns, with ih and map({'o', 'x'}, 'ih', ':<C-U>Gitsigns select_hunk<CR>'), doing vih will visually select the hunk. This is not possible with gh text object here?

Yes, Visual mode for textobject is not supported. For at least two reasons:

Any plans on implementing cycling between hunks? Thought I did ask, not a problem though.

If you mean wrapping around edges (like if on last hunk in the buffer, "next" navigates to the first hunk), then this comment has the reasoning for not adding it. Both as default and as an option. Although I am starting to maybe lean towards adding it as option. We'll see.

SanchayanMaity commented 6 months ago

In gitsigns, with ih and map({'o', 'x'}, 'ih', ':<C-U>Gitsigns select_hunk<CR>'), doing vih will visually select the hunk. This is not possible with gh text object here?

Yes, Visual mode for textobject is not supported. For at least two reasons:

  • Actually visually selecting hunk is rarely the end goal in itself. Usually it is to perform some action on it (apply hunk with ghgh, delete it with dgc, etc.). So the textobject can only be used inside an operator.
  • It does not work with default gh mappings for "apply" action. This design is similar to 'mini.comment' (although that one has separate comment_visual mapping which I a bit regret adding, to be honest).

Any plans on implementing cycling between hunks? Thought I did ask, not a problem though.

If you mean wrapping around edges (like if on last hunk in the buffer, "next" navigates to the first hunk), then this comment has the reasoning for not adding it. Both as default and as an option. Although I am starting to maybe lean towards adding it as option. We'll see.

Thank you for clarifying. Once again thank you for all this work.

jan-xyz commented 6 months ago

Hey your plugin looks really interesting! I am currently working on https://github.com/jan-xyz/lsp-preview.nvim/ to preview language-server workspace edits, like in renames or code-actions. It is currently showing the diff as a diff --git ... output but instead I would like to show it as a normal buffer and support scrolling up and down, showing in line diffs and so on. I assumed I could use mini.diff to display it since it supports setting alternative text as sources. I am currently doing

MiniDiff.enable(bufnr)
MiniDiff.set_ref_text(bufnr, old_text)

vim.api.nvim_buf_set_lines(bufnr, 0, -1, false, vim.split(new_text, "\n", { plain = true }))

bufnr is the buffer number which has the new text applied and old_text is the buffer content without any changes applied. I currently get an error mini.nvim/lua/mini/diff.lua:570: attempt to index a nil value which points here: https://github.com/echasnovski/mini.nvim/blob/5d841fcca666bc27ca777807a63381ce2cf6e2f9/lua/mini/diff.lua#L570

Do I need to do additional setup for a buffer so that the cache isn't set to nil for the buffer?

echasnovski commented 6 months ago

Do I need to do additional setup for a buffer so that the cache isn't set to nil for the buffer?

This error means that buffer was not enabled properly earlier (here and in the previous enable() call).

What is the full setup of 'mini.diff'? By default it uses Git source, so it might not attach properly and hence not enable buffer. If example is how code is executed (i.e. no vim.schedule()d stuff and so on), then my guess would be that it tries to attach to a non-file buffer (i.e. there is no proper path).

If you only want to use current buffer with custom reference text, then you'd have to customize its source with something like this (prior to the set_ref_text()):

vim.b[bufnr].minidiff_config = { source = { attach = function() end } }

Actually adding this type of source to built-ins might be a good idea.


Also, there is no need to enable() if you are going to set_ref_text(). I thought this was documented, but it is not. I'll fix it later.

jan-xyz commented 6 months ago

Thanks! That was fast and worked like a charm! Indeed I didn't do any setup because I don't want the plugin to rely on the config of a user in case they are using the plugin for other things.

jan-xyz commented 6 months ago

Another feedback: Would be nice to have an enable_overlay function. My workaround for now is to check if it is enabled for the buffer with get_buf_data and if it is disabled call toggle_overlay.

echasnovski commented 6 months ago

Another feedback: Would be nice to have an enable_overlay function. My workaround for now is to check if it is enabled for the buffer with get_buf_data and if it is disabled call toggle_overlay.

Yeah, I'd say this is a suggested way (at least for now). I imagine it mostly will be used interactively so single toggle_overlay() should be enough. For a plugin/script author an extra get_buf_data() call should not be a big problem.

echasnovski commented 6 months ago

I've pushed new updates to 'mini.diff'.


There is now a config.options.wrap_goto config setting which can be used to enable wrapping around edges during hunk navigation. It will produce a notification in case of an actual wrap so that it will be less disorienting. And it is disabled by default.

@GitMurf, @SanchayanMaity, mentioning you as you asked about it in this thread.


@jan-xyz, set_ref_text() now is documented to also enable buffer prior to setting text, while giving an informative error if it fails.

There is also a gen_source.none() built-in source in case user wants to manually set reference text for a buffer. It is essentially the same as in this comment.

xzbdmw commented 6 months ago

Hi, sometimes I want to copy some text from hunks, but I can't do that in extmark_based hunk message, the only way is to reset, copy and undo.

echasnovski commented 6 months ago

Hi, sometimes I want to copy some text from hunks, but I can't do that in extmark_based hunk message, the only way is to reset, copy and undo.

This is a very interesting idea! My main concern is that it should be not that complicated to implement on user side. And the most logical mapping for this operation (ghy) slightly conflicts with the nature of gh operator.

Here is the approximate implementation:

local copy_hunk_ref_text = function()
  local buf_data = MiniDiff.get_buf_data()
  if buf_data == nil then return end

  -- Get hunk under cursor
  local cur_line, cur_hunk = vim.fn.line('.'), nil
  for _, h in ipairs(buf_data.hunks) do
    local count = math.max(h.buf_count, 1)
    if h.buf_start <= cur_line and cur_line <= h.buf_start + count then cur_hunk = h end
  end
  if cur_hunk == nil then return end

  -- Get hunk's reference lines
  local ref_lines = vim.split(buf_data.ref_text, '\n')
  local from, to = cur_hunk.ref_start, cur_hunk.ref_start + cur_hunk.ref_count - 1
  local hunk_ref_lines = vim.list_slice(ref_lines, from, to)

  -- Populate register '"' (to be usable with plain `p`) with target lines
  vim.fn.setreg('"', hunk_ref_lines, 'l')
end

vim.keymap.set('n', 'ghy', copy_hunk_ref_text, { desc = "Copy hunk's reference lines" })

I'll test drive this and see if it worth adding to 'mini.diff' directly.