-
-
Notifications
You must be signed in to change notification settings - Fork 24
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
feat(#44): support env var specification #47
Conversation
lua/guard/filetype.lua
Outdated
@@ -42,6 +42,20 @@ local function box() | |||
return self | |||
end | |||
|
|||
function tbl:env(...) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
env just receive a param env table<string, string>
. if vim.tbl_count(env) == 0 return
then we can se tool.env == env
add a validate first by using vim.validate
for param env.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Attempted to resolve in next commit. Do the table keys and values also need to be validates like so?
for k, v in pairs(env) do
vim.validate({
k = { k, 'string' },
v = { v, 'string' }
})
end
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also note that we cannot set tool.env = env
because uv.os_environ() returns the environment like:
{ key = val }
and uv.spawn() needs the env like:
{ 'key=val' }
end | ||
tool.env = {} | ||
env = vim.tbl_extend('force', vim.uv.os_environ(), env or {}) | ||
for k, v in pairs(env) do |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hmm why we need these codes. just tool.env = env
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I say that above:
Also note that we cannot set tool.env = env because uv.os_environ() returns the environment like:
{ key = val }
and uv.spawn() needs the env like:
{ 'key=val' }
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
that's make sense but do we need extend os_environ
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes. I believe applying an env override removes all environment variables from the uv.spawn() process or something.
As evidence, it just doesn't work with spawning certain processes (like prettierd, for example) and breaks. Keeping uv.os_environ() fixes it. null-ls also does it.
Co-authored-by: Raphael <[email protected]>
support env var spec like so:
maintains existing env vars (does not override other than what's specified)