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

Mappings with variables outside of scope #84

Open
wants to merge 8 commits into
base: main
Choose a base branch
from
Open

Mappings with variables outside of scope #84

wants to merge 8 commits into from

Conversation

tris203
Copy link
Owner

@tris203 tris203 commented Jan 12, 2024

When a mapping is done via whichkey, we pass the table into whichkey for parsing.

If there are assignments that are done outside of the functional call we can't capture them. This means the loadfile() fails.

I don't know how to handle this

Here is a failing test to demonstrate the problem

Related to issue #81

@tris203 tris203 added bug Something isn't working help wanted Extra attention is needed labels Jan 12, 2024
Copy link
Collaborator

@willothy willothy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mostly lgtm, but some small ideas. I'm not sure this handles closure upvalues.

--Remove wrapping parens and wrap in table and unpack - issue #81
strObj = strObj:gsub("^%s*%(%s*", ""):gsub("%s*%)%s*$", "")
return loadstring("return {" .. strObj .. "}")()
local func = loadstring("return {" .. strObj .. "}")
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you may need upvalues as well, and this could still throw an error in some cases I think, like when the value returned here is not what's expected.

end
local wkMapping = which_key.parse(unpack(tableObj))

for _, mapping in ipairs(wkMapping) do
if mapping.cmd == nil then
mapping.cmd = "Function uses out of scope variables"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this should go on a separate error prop that can be displayed if the main prop is nil. Feels a bit weird to just include an error message without any kind of flag for it.

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

Successfully merging this pull request may close these issues.

2 participants