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

[BUG] api.uncomment.linewise.* give error when nothing to uncomment #221

Closed
bew opened this issue Aug 25, 2022 · 6 comments · Fixed by #285
Closed

[BUG] api.uncomment.linewise.* give error when nothing to uncomment #221

bew opened this issue Aug 25, 2022 · 6 comments · Fixed by #285

Comments

@bew
Copy link

bew commented Aug 25, 2022

Plugin version: 728f38e

Using following config:

require('Comment').setup {
  mappings = {
    extended = true
  }
}

Given some file foo.c:

abc

With the cursor on the line, using g<c will give the following error:

E5108: Error executing lua Vim(lua):E5108: Error executing lua ...m-wip/managed-plugins/Comment.nvim/lua/Comment/utils.lua:65: attempt to get length of local 'iter' (a nil value)
stack traceback:
        ...m-wip/managed-plugins/Comment.nvim/lua/Comment/utils.lua:65: in function 'is_empty'
        ...m-wip/managed-plugins/Comment.nvim/lua/Comment/utils.lua:297: in function 'uncomment'
        ...-wip/managed-plugins/Comment.nvim/lua/Comment/opfunc.lua:176: in function 'linewise'
        ...-wip/managed-plugins/Comment.nvim/lua/Comment/opfunc.lua:68: in function 'opfunc'
        ...vim-wip/managed-plugins/Comment.nvim/lua/Comment/api.lua:284: in function 'current'
        [string ":lua"]:1: in main chunk
        [C]: at 0x004609a0
stack traceback:
        [C]: at 0x004609a0
@numToStr
Copy link
Owner

I would say that it's not a bug to uncomment a line which is not commented to begin with, but the panic makes it look like one.

@bew
Copy link
Author

bew commented Aug 25, 2022

Yeah it's definitely useless, but can happen by mistake.. I think it should just do nothing

@numToStr
Copy link
Owner

To make this nicer I have to add checks in multiple places that's why left the panic as it is. But I'll think about it as I might also need to handle some other cases as well.

@winkee01
Copy link

winkee01 commented Oct 7, 2022

I have encountered the same issue.

@numToStr
Copy link
Owner

numToStr commented Oct 7, 2022

FYI, extended keybindings are deprecated 20772ed but that is not an excuse to hide the underlying issue which is with API itself. I am going fix the panics sooner or later.

@numToStr numToStr changed the title [BUG] g<c give error when nothing to uncomment [BUG] api.linewise.comment give error when nothing to uncomment Oct 7, 2022
@numToStr numToStr changed the title [BUG] api.linewise.comment give error when nothing to uncomment [BUG] api.comment.linewise.* give error when nothing to uncomment Oct 7, 2022
@numToStr numToStr changed the title [BUG] api.comment.linewise.* give error when nothing to uncomment [BUG] api.uncomment.linewise.* give error when nothing to uncomment Nov 28, 2022
numToStr added a commit that referenced this issue Dec 24, 2022
This adds error handling to the plugin, implemented with the help of
`assert` and `xpcall`. Adding explicit error handling for the following
cases for now:

- Invalid commenstring / Commenting is not supported like `json`
- When filetype doesn't support block comments like `yaml`
- Graceful handling of certain cases like uncommenting when there is
nothing to uncomment.

---

fixes #221 
supersedes #277
@bew
Copy link
Author

bew commented Dec 24, 2022

Great, thanks 😀👍

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

Successfully merging a pull request may close this issue.

3 participants