Skip to content
This repository has been archived by the owner on Aug 12, 2023. It is now read-only.

fix(command_resolver): fix resolve executable on Windows system #1341

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

fix(command_resolver): fix resolve executable on Windows system #1341

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
14e724e
fix(command_resolver): fix resolve executable on Windows system
IronLu233 Jan 13, 2023
e4413db
fix(command_resolver): deep copy the params, preventing potential risk.
IronLu233 Jan 14, 2023
7d260fe
Merge branch 'main' into IronLu233/main
Jan 16, 2023
1e1af9f
refactor(command_resolver): avoid (probably) unnecessary deep copy
Jan 16, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 6 additions & 2 deletions lua/null-ls/helpers/command_resolver.lua
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
local cache = require("null-ls.helpers.cache")
local u = require("null-ls.utils")

local M = {}

--- search for a local executable and its parent directory from start_path to end_path
Expand Down Expand Up @@ -46,7 +45,12 @@ end
M.from_node_modules = function()
local node_modules_resolver = M.generic(u.path.join("node_modules", ".bin"))
return function(params)
return node_modules_resolver(params) or params.command
if u.path.is_windows then
params.command = params.command .. ".cmd"
Copy link
Owner

Choose a reason for hiding this comment

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

Two potential issues with this:

  1. I'm not sure if mutating this directly on the params table will have any unexpected effects, but it seems risky.
  2. If we're looking for eslint and don't find a local executable, then we'll be looking for eslint.cmd in $PATH (I'm actually not sure what this will do, but I suspect it won't work).

I think it would be less risky to declare local command_to_find = params.command and use that, then fall back to params.command.

Copy link
Author

Choose a reason for hiding this comment

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

Unlike Linux, Windows determine a file is executable or not, is rely on file's extension. when file's extension changes, the file will not be executable.

image

I rename prettier.cmd to prettier.cmd3, and it can not be executed.

end

local resolved_executable = node_modules_resolver(params)
return resolved_executable or params.command
end
end

Expand Down
3 changes: 3 additions & 0 deletions lua/null-ls/utils/init.lua
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -238,6 +238,7 @@ end
---@class PathUtils
---@field exists fun(filename: string): boolean
---@field join function(paths: ...): string
---@field is_windows boolean
M.path = {
exists = function(filename)
local stat = vim.loop.fs_stat(filename)
Expand All @@ -246,6 +247,8 @@ M.path = {
join = function(...)
return table.concat(vim.tbl_flatten({ ... }), path_separator):gsub(path_separator .. "+", path_separator)
end,

is_windows = is_windows,
}

--- creates a callback that returns the first root matching a specified pattern
Expand Down