3rd / image.nvim

🖼️ Bringing images to Neovim.
MIT License
1.03k stars 42 forks source link

Images that have virtual text should follow their virtual text #83

Closed benlubas closed 9 months ago

benlubas commented 10 months ago

I feel like the document integration used to update the position of the image when text moved around it. Currently that's not the case. Images seem to get loaded when the buffer opens and adding lines above the image no longer moves it. @3rd was that intentional or did it get broken by another change?

It would be nice if images that take up space (ie. Have the with_virtual_padding flag set) could follow their extmark when it moves. Especially bc the virtual padding already moves when that happens, the image is just left behind.

I'm not sure if this would be a job for an autocommand or the on_win function that we override. I'm also not sure how laggy it would be.

3rd commented 10 months ago

It used to work, will check!

3rd commented 10 months ago

seems to work ok on master, can you confirm?

benlubas commented 10 months ago

This is the behavior that I see with the current master branch. The same line bug is back (as expected b/c it was causing other issues), and images don't move when there are lines added above or below.

https://github.com/3rd/image.nvim/assets/56943754/4d783968-6168-455a-b859-4a8b2e0f1b87

3rd commented 10 months ago

ah I see now, yep, I think that's only supposed to work with the document integration in the current design. the integration catches buffer changes and repositions them, the only way to do it would be indeed hard binding them to the extmarks and rerendering when those reposition.

https://github.com/3rd/image.nvim/assets/59587503/ef8a2db5-29b5-4332-a36b-95e5c6265db0

3rd commented 10 months ago

ah the overlapping here comes from https://github.com/3rd/image.nvim/commit/4d18722d82e9f3a5983c5a2d640fbf9973688906#diff-90b9887d8ed1bf7a314a020fe9673aacfe3778eac5198d82eafa29f3b2668dc3L212 i think we've outgrown this as a poc and it's time for a rewrite with a new api, tests and everything, i'll set it up.

benlubas commented 10 months ago

Interesting, I'm not even seeing the document integration working. I wonder if I have some other plugin that's interfering with it.

benlubas commented 9 months ago

i think we've outgrown this as a poc and it's time for a rewrite with a new api, tests and everything, i'll set it up.

@3rd do you have updates on this?

3rd commented 9 months ago

Hey, I just got the skeleton ready so far, setting up tests and planning the new API now, and I'll be working on it over the holidays. Regarding if this should be or not in image.nvim, I think it should be, images should not be out of sync with their extmarks, but extmarks were never considered a source of truth in the first iteration.

benlubas commented 9 months ago

Awesome, ty for the update! And for the great work you're going here!

3rd commented 9 months ago

@benlubas should be fixed in meanwhile https://github.com/3rd/image.nvim/commit/c53bbc41debf42e6470235c4fef9868f6be20c12

https://github.com/3rd/image.nvim/assets/59587503/798fa44c-7592-47d5-84b0-ca4f308fb850

benlubas commented 9 months ago

Thanks! this works pretty well. I'm seeing an error seemingly from a missing nil check. I'm going to play around with it and open a PR.

this also makes the multiple extmarks on the same line ordering bug really noticeable in molten so I'll also take another shot at fixing that later today.

3rd commented 9 months ago

Sounds great, thank you so much! I might've rushed it a bit, sorry for any issues.

benlubas commented 9 months ago

I experienced this bug literally the first time I open an image and now I haven't gotten it since, so it's probably not that big a problem.

3rd commented 9 months ago

I think it could be from this line in some weird case https://github.com/3rd/image.nvim/commit/c53bbc41debf42e6470235c4fef9868f6be20c12#diff-3ae45b29c1f8753c3287a7ef1577efc4a433de48f9afa60b7a631377b70678ddR294

benlubas commented 9 months ago

Oh nevermind I got it. When there's a document integration image and a user added image, and you add lines above the document image, you can push the user added image off screen and it throws the error.

also there's something wrong with adding lines normally for me. I wonder if that's a plugin integration thing.

Also I'm on nvim 9.5.

https://github.com/3rd/image.nvim/assets/56943754/61236484-6f25-4d55-97a9-219f61748cf9

benlubas commented 9 months ago

Okay, the weird document integration thing is something that doesn't happen in a minimal config. I have some other plugin that's messing that up.

3rd commented 9 months ago

Found a weird issue with images getting messed up and performance going down and bisected it to the namespace commit, looking into it.

3rd commented 9 months ago

Fixed the namespace issue, would be great to get a screenshot of the error you're getting.

benlubas commented 9 months ago

Fixed the namespace issue

This was actually the thing that caused that crazy image duplication problem, so that's fixed now. My minimal config still had a tag set 😅 so that's why I wasn't seeing problems

benlubas commented 9 months ago

I think this is all good now! If I find other bugs I'll open a new issue.

Thanks for the work

3rd commented 9 months ago

Thanks for all the help Ben <3