Skip to content

✨ Feature: API to enable for specific buffer/win? #216

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

Closed
1 of 4 tasks
ibhagwan opened this issue Dec 4, 2024 · 45 comments
Closed
1 of 4 tasks

✨ Feature: API to enable for specific buffer/win? #216

ibhagwan opened this issue Dec 4, 2024 · 45 comments
Assignees
Labels
enhancement New feature or request

Comments

@ibhagwan
Copy link

ibhagwan commented Dec 4, 2024

The nature of the feature:

  • Parser(syntax, conceal etc. related)
  • Renderer(style, options etc. related)
  • Support(plugin support, language support etc. related)
  • Other

Description:

Hi again @OXY2DEV!

Would love to support your amazing plugin in fzf-lua previews, similar to what I did with render-markdown.nvim ibhagwan/fzf-lua#1546.

Is there a single command I can run, similar to what is described here:

require("render-markdown.core.ui").update(preview_buf, preview_win, "Fzf", true)

Where I'm able to attach to a specific window/buffer and trigger a redraw (so that I can call it upon scroll/resize/etc) that does not rely on autocmds (those are limited/ignored in the preview buffer)?

@ibhagwan ibhagwan added the enhancement New feature or request label Dec 4, 2024
@OXY2DEV
Copy link
Owner

OXY2DEV commented Dec 5, 2024

On the dev branch you can do,

local cmd = require("markview").commands;

-- Attach to a buffer.
cmd.attach(1);
-- Detach from a buffer.
cmd.detach(1);

-- Draw preview to a buffer(this will not change `conceallevel` & `concealcursor`, and will not update itself).
cmd.redraw(1);
-- Clear previews from a buffer.
cmd.clear(1);

These can also be used as regular commands.

:Markview attach 1
:Markview detach 1

:Markview redraw 1
:Markview clear 1

@OXY2DEV
Copy link
Owner

OXY2DEV commented Dec 5, 2024

It unfortunately doesn't allow passing a window as the renderers don't care about the window

Rather they run a utility function to get the most likely correct window.

@ibhagwan
Copy link
Author

ibhagwan commented Dec 5, 2024

It unfortunately doesn't allow passing a window as the renderers don't care about the window

Rather they run a utility function to get the most likely correct window.

Shouldn’t make a difference as the preview buffer is “unique”, I’ll test this tomorrow and let you know.

@ibhagwan
Copy link
Author

ibhagwan commented Dec 5, 2024

@OXY2DEV, this was completed in ibhagwan/fzf-lua@ed6539e.

Unfortuntaely, it's a bit laggy, when scrolling it's also not very funcitonal, as it's very slow and almost impossible to cancel, for example, if I press scroll 5 times I have to wait a while for the result to be displayed and the intreface is laggy while this happens.

Let me know if there's a better way than calling attach followed by render as I couldn't find a method for is_attached but looking at the code it seemed harmless as it would just discover buffer is already attached and exit the function early.

@OXY2DEV
Copy link
Owner

OXY2DEV commented Dec 6, 2024

That's weird. It shouldn't be lagging when scrolling.

if I press scroll 5 times I have to wait a while for the result to be displayed

Do you have anything that modifies the buffers text? The default denounce value is 50ms which is noticable in most cases so it could be just the plugin waiting for the denounce period to finish.

Can you tell me how fzf-lua is using markview, so that I can test it?

Let me know if there's a better way than calling attach followed by render as I couldn't find a method for is_attached but looking at the code it seemed harmless as it would just discover buffer is already attached and exit the function early.

The reason there's not method for that is because the attach() function should be checking if the plugin can attach or not.

I will add a method to check it.

@ibhagwan
Copy link
Author

ibhagwan commented Dec 6, 2024

Do you have anything that modifies the buffers text? The default denounce value is 50ms which is noticable in most cases so it could be just the plugin waiting for the denounce period to finish.

Nothing special, scroll is just a call to ctrl-d/u.

Can you tell me how fzf-lua is using markview, so that I can test it?

Markview is enabled by default, use the latest fzf-lua and have markview plugin loaded and it should get called automatically in the preview buffer.

@OXY2DEV
Copy link
Owner

OXY2DEV commented Dec 6, 2024

Nothing special, scroll is just a call to ctrl-d/u.

Uhh, so how do I scroll? I can't use Shift+Up/Down due to being on Android.

@ibhagwan
Copy link
Author

ibhagwan commented Dec 6, 2024

Nothing special, scroll is just a call to ctrl-d/u.

Uhh, so how do I scroll? I can't use Shift+Up/Down due to being on Android.

Change the bindings to something you can use.

For a single call picker call:

:lua require("fzf-lua").files({keymap={builtin={["<c-f>"]="preview-page-down"}}})

Permanently:

require("fzf-lua").setup({
  keymap = {
    builtin = {
      true, -- inherit other default binds
      -- Also try preview-half-page-{up|down}
      ["<c-f>"]="preview-page-down",
      ["<c-b>"]="preview-page-up"
    }
  }
})

@OXY2DEV
Copy link
Owner

OXY2DEV commented Dec 6, 2024

Ah, thanks. That works.

I am noticing lags when switching between files. But I don't see any lags when doing preview-page-up & preview-page-down.

Does the lag happen even when viewing small files on your end?

@ibhagwan
Copy link
Author

ibhagwan commented Dec 6, 2024

Does the lag happen even when viewing small files on your end?

Happens on the fzf-lua project REAME.md, let me know if you’d like I’ll record a comparison between how it behaves with render-markdown vs markview.

@OXY2DEV
Copy link
Owner

OXY2DEV commented Dec 6, 2024

So, I am noticing that on each scroll attach() & redraw() are being called.

attach() is exiting property. So, I am thinking that lag could potentially be from having 2 redraws(1 from redraw() and another from the CursorMovedI autocmd used by attach()).


I don't think you need to run attach() because, the user might not want the buffers to be attached without actually opening them.

You can however do.

--- I will be adding a way to retrieve the buffer if you don't have access to it.
cmd.clear(bufnr);
cmd.redraw(bufnr);

You are running it on each scroll. So, this should be a lot simpler and easier.

@OXY2DEV
Copy link
Owner

OXY2DEV commented Dec 6, 2024

Yup, it's definitely the 2 redraws.

I changed attach() with clear() and the lag seems to not happen on scroll anymore.


But this means that conceallevel & concealcursor aren't set.

Since render-markdown & markview.nvim both use these options. Is it possible to change their value based on filetype on your end?(Because I don't want JSON files to also use conceals if these are set globally).

@ibhagwan
Copy link
Author

ibhagwan commented Dec 6, 2024

Changed to:

cmd.clear(bufnr);
cmd.redraw(bufnr);

Similar slow and laggy behavior on scrolls.

@ibhagwan
Copy link
Author

ibhagwan commented Dec 6, 2024

But this means that conceallevel & concealcursor aren't set.

Since render-markdown & markview.nvim both use these options. Is it possible to change their value based on filetype on your end?(Because I don't want JSON files to also use conceals if these are set globally).

It seems this is still rendered regardless of the options (I didn't check if it's set or not), but if these are needed and I call clear or redraw doesn't it make more sense to have these enabled on markview's end?

@OXY2DEV
Copy link
Owner

OXY2DEV commented Dec 6, 2024

Changed to:

cmd.clear(bufnr);
cmd.redraw(bufnr);

Similar slow and laggy behavior on scrolls.

Uhh, does it look like this on your end? I can't really tell which one(Termux, neovim, fzf-lua or markdown) is lagging.

Screenrecorder-2024-12-06-08-53-04-455.mp4

@OXY2DEV
Copy link
Owner

OXY2DEV commented Dec 6, 2024

It seems this is still rendered regardless of the options

Yes, because I don't actually set any options. I just run a function(that can be defined by the user too) when previews are enabled.

But I guess it makes sense to also call them on redraw() & clear().


Also, I am seeing that you have pushed changes to main in your repo.

Unfortunately, the whole commands thing only works on the dev branch.

So, I think you should comment it out until v25 is released.

@ibhagwan
Copy link
Author

ibhagwan commented Dec 6, 2024

Also, I am seeing that you have pushed changes to main in your repo.
Unfortunately, the whole commands thing only works on the dev branch.
So, I think you should comment it out until v25 is released.

I see, perhaps it's better off I will comment this out as well and we do the testing again once this is on the main branch, otherwise it's just asking for issues.

@OXY2DEV
Copy link
Owner

OXY2DEV commented Dec 6, 2024

Can you send a recording of the lags?

ibhagwan added a commit to ibhagwan/fzf-lua that referenced this issue Dec 6, 2024
ibhagwan added a commit to ibhagwan/fzf-lua that referenced this issue Dec 6, 2024
@ibhagwan
Copy link
Author

ibhagwan commented Dec 6, 2024

Can you send a recording of the lags?

Recoding now, I found the issue is actually much worse, just having markview enabled makes the scroll of anything in fzf-lua worse, even when markview rendering is disabled in the previewer, recording now.

@OXY2DEV
Copy link
Owner

OXY2DEV commented Dec 6, 2024

I think I found the issue.

For some reason, I added CursorMoved to attach() instead of being a global event.

This caused the plugin to become extremely laggy since every time attach() is called a new autocmd getes created resulting in this mess.

@ibhagwan
Copy link
Author

ibhagwan commented Dec 6, 2024

I just recorded using software called peek but it's impossible to see the issue as the frame rate is so bad, I'll have to research something that records terminal in high FPS.

Basically if I hold down shift-down to scroll, enabling markview.nvim causes very bad lag whereas without the plugin the scrolling is super speedy and instant

I think I found the issue.
For some reason, I added CursorMoved to attach() instead of being a global event.

It certainly does feel like markview is doing something regardless of it happening in an unattached buffer.

@OXY2DEV
Copy link
Owner

OXY2DEV commented Dec 6, 2024

I have done some test. And the lag is definitely coming from the movement autocmds.

I will see where the issue is.

OXY2DEV added a commit that referenced this issue Dec 6, 2024
Ref: #177

fix: Removed per buffer CusorMoved autocmd.

Ref: #216
OXY2DEV added a commit that referenced this issue Dec 6, 2024
@OXY2DEV
Copy link
Owner

OXY2DEV commented Dec 6, 2024

I have made some changes to how redrawing works.

Can you test it out(as I can't test it properly on a phone)?

Here's a minimal config to make it easier,

vim.env.LAZY_STDPATH = ".repro"
load(vim.fn.system("curl -s https://raw.githubusercontent.com/folke/lazy.nvim/main/bootstrap.lua"))()

require("lazy.minit").repro({
        spec = {
                {
                        "nvim-treesitter/nvim-treesitter",
                        build = ":TSUpdate",

                        ensure_installed = { "markdown", "markdown_inline" }
                },
                {
                        "OXY2DEV/markview.nvim",
                        branch = "dev",
                        lazy = false
                },
                {
                        "ibhagwan/fzf-lua",
                        commit = "ed6539e018a522f176ad1f1ee6cb2e0172e8379a",
                        pin = true,
                        opts = {
                                keymap = {
                                        builtin = {
                                                ["<PageUp>"] = "preview-page-up",
                                                ["<PageDown>"] = "preview-page-down",
                                        }
                                }
                        }
                }
        },
});

vim.o.conceallevel = 3;
vim.api.nvim_set_keymap("n", "<space>q", "<CMD>:q<CR>", {});

@ibhagwan
Copy link
Author

ibhagwan commented Dec 6, 2024

Can you test it out(as I can't test it properly on a phone)?

Going to sleep, will test tomorrow, now regarding this part…

I can't test it properly on a phone

Is this a temporary a thing or are you telling me you’ve developed this plugin in its entirety on a freaking phone, and why do I get the feeling it’s the latter (as evident in this comment)? 🤯🤯🤯

If so, I gotta step up my game, this is some next level dedication lol

@OXY2DEV
Copy link
Owner

OXY2DEV commented Dec 6, 2024

Is this a temporary a thing or are you telling me you’ve developed this plugin in its entirety on a freaking phone,

Yes, all 24,461(or around 18K if you only count code) lines of it 💀.
Screenshot_2024-12-06-12-43-47-712_com termux-edit

Just to be clear, ~6,000 lines of it are just for storing math symbols, math fonts, HTML entities etc.

@OXY2DEV
Copy link
Owner

OXY2DEV commented Dec 6, 2024

If so, I gotta step up my game, this is some next level dedication lol

Those 4k+ stars(in your repos) would like to disagree with you.

@ibhagwan
Copy link
Author

ibhagwan commented Dec 6, 2024

Yes, all 24,461(or around 18K if you only count code) lines of it 💀.

Couldn’t help it lol

https://www.reddit.com/r/neovim/comments/1h7vhmg/bro_been_developing_his_2k_star_plugin_on_a/

Those 4k+ stars(in your repos) would like to disagree with you.

You’re too kind :)

OXY2DEV added a commit that referenced this issue Dec 6, 2024
@OXY2DEV
Copy link
Owner

OXY2DEV commented Dec 6, 2024

So, I have looked into the issue a bit more and here's what I found,

  1. The culprit for the lag was redraw(). It is meant to render the preview only once and doesn't do denouncing. So, this causes a lot of lags(as literally calling it causes it to redraw the buffer 20-30 times a second).
  2. Making the entire thing asynchronous isn't an option as one of the core feature of the plugin is the ability to change the config at any time(even without calling setup()). And trying to make the entire thing async kinda doesn't work with this.
  3. Using attach() from the previewers side isn't a good option as it would relay on the mode for showing the preview. The user could have changed the mode option which would result in the preview not showing at all.

Here's what I think should be done.

Allow attach() to take a second argument. If the second argument is not nil it will ignore the mode option.

OXY2DEV added a commit that referenced this issue Dec 6, 2024
Allows buffers to show previews even if previews are disabled for that
mode.

Ref: #216
@OXY2DEV
Copy link
Owner

OXY2DEV commented Dec 6, 2024

Here's a simple comparison(with the minimal config,

Pay attention to when I let go of the buttons.

Before

before.mp4

After

after.mp4

Note

I deleted mv.redraw() and used mv.attach(bufnr, true).

@ibhagwan
Copy link
Author

ibhagwan commented Dec 6, 2024

Pay attention to when I let go of the buttons.

Can def see the difference.

I deleted mv.redraw() and used mv.attach(bufnr, true)

Just making sure I understand this right,I should reduce calls to atrach only? No need to call mv.redraw anymore?

@OXY2DEV
Copy link
Owner

OXY2DEV commented Dec 6, 2024

Just making sure I understand this right,I should reduce calls to atrach only? No need to call mv.redraw anymore?

Yes, just calling mv.attach(bufnr, true) will be enough.

Let me know, if the lag still occurs.

@ibhagwan
Copy link
Author

ibhagwan commented Dec 6, 2024

Let me know, if the lag still occurs.

It's fantastic now, no lag!

While adding support for render-markdown I realized I could minic the split view (which you have added to this plugin), looks quite wonderful doesn't it?

We can close this issue with this beautiful screenshot, cheers @OXY2DEV 🚀

image

@ibhagwan ibhagwan closed this as completed Dec 6, 2024
@OXY2DEV
Copy link
Owner

OXY2DEV commented Dec 6, 2024

It's fantastic now, no lag!

Awesome!

While adding support for render-markdown I realized I could minic the split view (which you have added to this plugin), looks quite wonderful doesn't it?

Indeed, it does. Even though I added splitview & hybrid_mode. I never actually used them(outside of testing).

So, feedbacks are welcome.

We can close this issue with this beautiful screenshot, cheers @OXY2DEV 🚀

Cheers!

@ibhagwan
Copy link
Author

ibhagwan commented Dec 6, 2024

Just re-enabled support for markview, although it's still on dev branch I used the existence of the redraw method to distinguish between main/dev, let me know if this looks acceptable (unless you plan to delete the redraw method from commands, this should work):

ibhagwan/fzf-lua@a569876

  elseif package.loaded["markview"] then
    local cmds = package.loaded["markview"].commands
    if cmds and cmds.redraw then cmds.attach(bufnr, true) end
  end

@OXY2DEV
Copy link
Owner

OXY2DEV commented Dec 6, 2024

Yes, this looks good.

@DT021
Copy link

DT021 commented Dec 6, 2024

Is this a temporary a thing or are you telling me you’ve developed this plugin in its entirety on a freaking phone,

Yes, all 24,461(or around 18K if you only count code) lines of it 💀. Screenshot_2024-12-06-12-43-47-712_com termux-edit

Just to be clear, ~6,000 lines of it are just for storing math symbols, math fonts, HTML entities etc.

Is this a preference thing or do you not have a pc to be able to code on?

@OXY2DEV
Copy link
Owner

OXY2DEV commented Dec 7, 2024

Is this a preference thing or do you not have a pc to be able to code on?

I don't have a PC to code on.

@zachlatta
Copy link

If you're interested, there are some people in https://www.reddit.com/r/neovim/comments/1h7vhmg/bro_been_developing_his_2k_star_plugin_on_a/ who are interested in starting a GoFundMe for you to get a laptop!

@OXY2DEV
Copy link
Owner

OXY2DEV commented Dec 8, 2024

If you're interested, there are some people in https://www.reddit.com/r/neovim/comments/1h7vhmg/bro_been_developing_his_2k_star_plugin_on_a/ who are interested in starting a GoFundMe for you to get a laptop!

There's already #218 about that.

GoFundMe only works on selected countries, so it wouldn't really work out.

@zachlatta
Copy link

What country are you in? I might be able to help coordinate. I run https://hackclub.com which has given grants to high schoolers all over the world.

@OXY2DEV
Copy link
Owner

OXY2DEV commented Dec 8, 2024

What country are you in?

Bangladesh.

high schoolers

I am not a high schooler though(unless high school also includes the 11th & 12th grade).

@siduck
Copy link

siduck commented Dec 10, 2024

@OXY2DEV you should be able to setup buymecoffee or ko-fi at least 🤔

@Myzel394
Copy link

@OXY2DEV if you can cash it out, I (and surely other people as well) could also send you some crypto currencies! I'd be happy to send you some Bitcoin or Monero :)

@xz-dev
Copy link

xz-dev commented Dec 13, 2024

@OXY2DEV if you can cash it out, I (and surely other people as well) could also send you some crypto currencies! I'd be happy to send you some Bitcoin or Monero :)

Although cryptocurrencies are convenient for international transactions, I don’t think there is a way for a minor to cash out for fiat currency.

@Leichesters
Copy link

Although cryptocurrencies are convenient for international transactions, I don’t think there is a way for a minor to cash out for fiat currency.

It is good idea. He has parents that can cash out or he can wait until he is no longer minor. I can also send xrp.

OXY2DEV added a commit that referenced this issue Jan 4, 2025
Use `markview.render()` instead.

Ref: #177, #216
OXY2DEV added a commit that referenced this issue Jan 10, 2025
Ref: #177

fix: Removed per buffer CusorMoved autocmd.

Ref: #216
OXY2DEV added a commit that referenced this issue Jan 10, 2025
OXY2DEV added a commit that referenced this issue Jan 10, 2025
OXY2DEV added a commit that referenced this issue Jan 10, 2025
Allows buffers to show previews even if previews are disabled for that
mode.

Ref: #216
OXY2DEV added a commit that referenced this issue Jan 10, 2025
Use `markview.render()` instead.

Ref: #177, #216
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
Development

No branches or pull requests

8 participants