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

Feature Request: function to toggle alignment #5

Open
igorlfs opened this issue Jun 16, 2024 · 8 comments
Open

Feature Request: function to toggle alignment #5

igorlfs opened this issue Jun 16, 2024 · 8 comments

Comments

@igorlfs
Copy link

igorlfs commented Jun 16, 2024

Currently, the plugin exposes align_csv and align_csv_clear, but I'd be nice to have another function for toggling, as it simplifies the keymap setup by not needing two mappings.

@emmanueltouzery
Copy link
Owner

i would be ok to add a toggle, but are you sure it would really be handy? because if you're going to edit the CSV then, despite the autocommand, you'll probably have to trigger re-aligning every once in a while. so from that point of view it's handy to allow to align multiple times.

Also I'm not sure when one would really need to disable the aligning after all. What's the downside?

@igorlfs
Copy link
Author

igorlfs commented Jun 16, 2024

i would be ok to add a toggle, but are you sure it would really be handy?

You do make some fair points. Initially, I was using an autocmd (FileType) to trigger align_csv, but that didn't work out very well when opening large files (as one would expect). I could tweak the autocmd to check file size first, but then I thought I'd be simpler to just have a keymap to enable the plugin and, if I noticed any lag, I'd just use the keymap again to disable it. But I do understand this might be a niche approach.

@emmanueltouzery
Copy link
Owner

So you would disable back due to the lag. But by that time you've already paid the price right? It aligned it. It won't try to re-align it automatically if the initial align took more than 50ms by default. So from then on it'll be fast. And you can ask to re-align manually after you make changes through the shortcut.

I can add the toggle, already someone asked for that. I'm just trying to Keep the plugin simple. But I'm guessing there is a use case for the toggle. I just don't quite see it.

@emmanueltouzery
Copy link
Owner

emmanueltouzery commented Jun 16, 2024

Maybe the align code could also abort and revert on its own if alignment took more than X ms. The checks whether we are too long could slow down things a little, but I would probably check the timer every X rows.. maybe every 100 or every 1000 rows for instance.

Edit: there'd have to be a flag: abort if too slow or keep going no matter what. Maybe.

@emmanueltouzery
Copy link
Owner

I wonder, how many rows are in the CSV files that were problematic for you?

@igorlfs
Copy link
Author

igorlfs commented Jul 6, 2024

Hey, I'm sorry for the delay!

I was using a file with about 300K lines.

My main issue was that besides the initial lag, actions like scrolling become noticeably slower.

A setting to abort and revert alignment sounds nice, considering the use case of "I want to always align my CSVs, except if it would take too long".

@emmanueltouzery
Copy link
Owner

that's very interesting! I only tested up to 100k lines (and the aligning took ~2.7s on my computer which didn't strike me as too bad), it didn't cross my mind that scrolling might become slower due to the extmarks. an option could be to lazily add extmarks only to the rows displayed on screen but.. it gets messy, not sure it's worth it. and it would be ugly on scrolling (no lining up, it would line up only when you stop scrolling).

i'll add some settings to auto-abort. maybe just number of lines, or file size.

@igorlfs
Copy link
Author

igorlfs commented Jul 7, 2024

Yeah, I think lazily adding extmarks would be messy, specially when handling multiple windows.

My current workaround:

vim.api.nvim_create_autocmd("FileType", {
    desc = "Align CSV",
    pattern = { "csv" },
    callback = function(args)
        local MAX_FILESIZE = 1000 * 1024 -- 1 MB
        local ok, stats = pcall(vim.uv.fs_stat, vim.api.nvim_buf_get_name(args.buf))
        if ok and stats and stats.size < MAX_FILESIZE then
            require("decisive").align_csv({})
        end
    end,
})

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

No branches or pull requests

2 participants