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

Activity

1. 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>]][/+] on Mar 29, 2022
2. numToStr commented on Mar 29, 2022

   

   numToStr

   on Mar 29, 2022

   Last edited by numToStr

   Owner

   More actions

   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 :)

   ❤️React with ❤️1Reacted by Declan Mullen
3. numToStr

   added

   good first issueGood for newcomersGood for newcomers

   bugSomething isn't workingSomething isn't working

   on Mar 30, 2022
4. numToStr commented on Mar 30, 2022

   

   numToStr

   on Mar 30, 2022

   Owner

   More actions

   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')
5. declancm commented on Mar 30, 2022

   

   declancm

   on Mar 30, 2022

   Last edited by declancm

   ContributorAuthor

   More actions

   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.
6. declancm commented on Mar 30, 2022

   

   declancm

   on Mar 30, 2022

   ContributorAuthor

   More actions

   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!
7. numToStr commented on Mar 30, 2022

   

   numToStr

   on Mar 30, 2022

   Owner

   More actions

   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.

   ❤️React with ❤️1Reacted by Declan Mullen
8. numToStr

   linked a pull request that will close this issuechange: use starting column when using `*_current_*` apis #136on Mar 31, 2022
9. numToStr

   closed this as completedin #136on Apr 1, 2022