Skip to content

API toggle_current_linewise not working with single line vim.cmd [[<stuff>]] #135

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
declancm opened this issue Mar 29, 2022 · 5 comments · Fixed by #136
Closed

API toggle_current_linewise not working with single line vim.cmd [[<stuff>]] #135

declancm opened this issue Mar 29, 2022 · 5 comments · Fixed by #136
Labels
bug Something isn't working good first issue Good for newcomers

Comments

@declancm
Copy link
Contributor

This issue occurs when the cursor is within the double square brackets in a vim.cmd [[ <stuff> ]]

vim.cmd [[echohl ErrorMsg | echo "Error: Something goofy happened." | echohl None]]

When using the gcc command on this line, it becomes:

-- vim.cmd [[echohl ErrorMsg | echo "Error: Something goofy happened." | echohl None]]

But if you use lua require('Comment.api').toggle_current_linewise(), it becomes an incorrect comment:

" vim.cmd [[echohl ErrorMsg | echo "Error: Something goofy happened." | echohl None]]

PS. Thanks for the great plugin!

@declancm declancm changed the title API toggle_current_linewise not working with single line vim.cmd([[ <stuff> ]]) API toggle_current_linewise not working with single line vim.cmd [[<stuff>]] Mar 29, 2022
@numToStr
Copy link
Owner

numToStr commented Mar 29, 2022

hmmm... that's certainly a issue. There is one subtle difference b/w gcc and the API and maybe that's what causing the issue but I can't say for sure.

But if you use lua require('Comment.api').toggle_current_linewise(), it becomes an incorrect comment

I would argue that this is the correct behaviour because inside vim.cmd, treesitter injects vimscript that's why it's using " as the commenstring.

Well I have to debug this anyway because both of them should behave the same :)

@numToStr numToStr added good first issue Good for newcomers bug Something isn't working labels Mar 30, 2022
@numToStr
Copy link
Owner

Wow, I never imagined this issue would be that tricky to solve. This bug is caused by the one difference that I talked about earlier.

gcc considers the current column position to end-of-line into its range because it uses a different API require('Comment.api').toggle_current_linewise_op(vim_mode, cfg) whereas require('Comment.api').toggle_current_linewise(cfg) uses the current cursor position as its range.

You can use this for now and let me know whether this fixes your issue or not

require('Comment.api').toggle_current_linewise_op('line')

@declancm
Copy link
Contributor Author

declancm commented Mar 30, 2022

I tried using require('Comment.api').toggle_current_linewise_op('line') but it was commenting the whole file so i tried require('Comment.api').toggle_current_linewise_op() but that has the same original issue. What is currently working is:

local position = vim.api.nvim_win_get_cursor(0)
vim.api.nvim_command('normal! $')
require('Comment.api').toggle_current_linewise()
vim.api.nvim_win_set_cursor(0, position)

I will try and find a proper fix but this would be my first pull request so idk how I'll go haha.

@declancm
Copy link
Contributor Author

I tried changing the region for nil vmode to use the first column instead of the current column which will go outside a vim.cmd [[<stuff>]] to get the correct commentstring.

function U.get_region(vmode)
    if not vmode then
        -- Added this:
        local row = unpack(A.nvim_win_get_cursor(0))
        return { srow = row, scol = 0, erow = row, ecol = 0 }
        -- Removed this:
        -- local row, col = unpack(A.nvim_win_get_cursor(0))
        -- return { srow = row, scol = col, erow = row, ecol = col }
    end

    local m = A.nvim_buf_get_mark
    local buf = 0
    local sln, eln

    if vmode:match('[vV�]') then
        sln, eln = m(buf, '<'), m(buf, '>')
    else
        sln, eln = m(buf, '['), m(buf, ']')
    end

    return { srow = sln[1], scol = sln[2], erow = eln[1], ecol = eln[2] }
end

I have done some testing and it has been working properly. Do you see any issues before I submit a pull request? Thanks!

@numToStr
Copy link
Owner

I tried changing the region for nil vmode to use the first column instead of the current column

Ahh, nice. That should fix the issue. But we need to check this with the treesitter.

Do you see any issues before I submit a pull request? Thanks!

Yeah, go ahead we can discuss other issues on the PR itself.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working good first issue Good for newcomers
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants