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

helpers: toLuaObject fails to escape strings containing ' #71

Closed
rummik opened this issue Nov 21, 2022 · 8 comments · Fixed by #72
Closed

helpers: toLuaObject fails to escape strings containing ' #71

rummik opened this issue Nov 21, 2022 · 8 comments · Fixed by #72
Labels
bug Something isn't working

Comments

@rummik
Copy link
Contributor

rummik commented Nov 21, 2022

Plugin affected: many
Nixpkgs channel: 22.05
Home-manager version: 22.05

Description

It looks like the issue is toLuaObject is using escapeShellArg, which doesn't properly handle escaping strings in this situation.

https://github.com/pta2002/nixvim/blob/c320be04c82914933fe94f64282e646d97d342d4/plugins/helpers.nix#L23-L25

When given the string "<cmd>lua require('gitsigns').blame_line{full=true}<cr>"
it outputs '<cmd>lua require('\''gitsigns'\'').blame_line{full=true}<cr>'

This results in the following error when starting Neovim:

Error detected while processing /nix/store/77xmlnk9bfjw7xd811rf8yipw0jkc2kv-home-manager-files/.config/nvim/init.lua:
E5112: Error while creating lua chunk: ...1rf8yipw0jkc2kv-home-manager-files/.config/nvim/init.lua:92: '}' expected near '\'

Config

{
  programs.nixvim.maps.normal."<leader>hb" = "<cmd>lua require('gitsigns').blame_line{full=true}<cr>";
}
@rummik rummik added the bug Something isn't working label Nov 21, 2022
@rummik
Copy link
Contributor Author

rummik commented Nov 21, 2022

A workaround seems to be using double quotes instead of single quotes within the passed string

@pta2002
Copy link
Collaborator

pta2002 commented Nov 22, 2022

Ah, seems to be an issue with using escapeShellArg. Guess it's about time I implement my own escaping function (it escapes ' to ''\''', which works in shell due to implicit concatenation but not in lua!)

@rummik
Copy link
Contributor Author

rummik commented Nov 22, 2022

@pta2002 I actually have some progress towards this if you'd like me to submit a PR

@rummik
Copy link
Contributor Author

rummik commented Nov 22, 2022

For now I'd suggest using builtins.toJSON specifically for string conversion, since it should be compatible in most cases

@rummik
Copy link
Contributor Author

rummik commented Nov 22, 2022

I'm a bit confused about this part of toLuaObject

https://github.com/pta2002/nixvim/blob/a601a75d0cae8229f1416506bd172e4e4aef3ce7/plugins/helpers.nix#L17-L20

Specifically I'm not sure I understand what the special case for filtering out nil and empty tables is for, and I don't know what the significance of @ prefixed keys are in Lua 😅

@rummik
Copy link
Contributor Author

rummik commented Nov 22, 2022

As far as I can tell, the condition on line 17 is never reached in the current codebase

@pta2002
Copy link
Collaborator

pta2002 commented Nov 22, 2022

I'm a bit confused about this part of toLuaObject

https://github.com/pta2002/nixvim/blob/a601a75d0cae8229f1416506bd172e4e4aef3ce7/plugins/helpers.nix#L17-L20

Specifically I'm not sure I understand what the special case for filtering out nil and empty tables is for, and I don't know what the significance of @ prefixed keys are in Lua sweat_smile

Honeslty I genuinely do not remember why I put that there... I'm filtering out nil so code does not end up with a bunch of x = nil, because that'll be by far the most common value and I don't want to bloat up the vimrc

@rummik
Copy link
Contributor Author

rummik commented Nov 24, 2022

For now it's just a simple fix. I don't really feel comfortable changing much in helpers without some kind of tests to make sure I don't break everything 😅

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

Successfully merging a pull request may close this issue.

2 participants