Clink path get broken if clink-completions content is created in a different order #2278
Conversation
…nt, leading to .init.lua not being run first lua path being broken
|
@vladimir-kotikov what do you think of this soution? |
|
@daxgames sounds good to me! When I tested this it seemed to me that the naming plays the role and the starting |
| @@ -431,6 +431,8 @@ clink.prompt.register_filter(svn_prompt_filter, 50) | |||
| clink.prompt.register_filter(percent_prompt_filter, 51) | |||
|
|
|||
| local completions_dir = clink.get_env('CMDER_ROOT')..'/vendor/clink-completions/' | |||
| -- Execute '.init.lua' first to ensure package.path is set properly | |||
| dofile(completions_dir..'.init.lua') | |||
| for _,lua_module in ipairs(clink.find_files(completions_dir..'*.lua')) do | |||
| -- Skip files that starts with _. This could be useful if some files should be ignored | |||
| if not string.match(lua_module, '^_.*') then | |||
There was a problem hiding this comment.
@Mikaz-fr do you think you also need to skip reading .init.lua here as it has been already executed above?
There was a problem hiding this comment.
I thought about this option but I thought it would make the code more complex with not much added benefit.
With the current proposal .init.lua will be executed twice, and thus the lua package.path will end up with duplicated entry pointing to clink-completions\modules\.
I think the performance impact is really minimal as search for module should stop on first hit (so having duplicated path should slow that down) and executing the 1 line of .init.lua a second time should not be longer than checking for file name in the for loop.
Overall I don't have a strong opinion and I went for the simplest change but I'm fine with any other proposal as long as package.path is set before the for loop starts.
There was a problem hiding this comment.
Sounds reasonable 👍 Let's keep it as-is then
In clink.lua line 434 we use clink.find_files() to call each lua script from clink-completions folder:
The find_file function is implemented in the Clink dll (https://github.com/mridgers/clink/blob/0.4.9/clink/dll/lua.c#L215) and make use of readdir() to list files present. But readdir() doesn't ensure any order for file being returned (see http://man7.org/linux/man-pages/man3/readdir.3.html). It seems that on Windows the order returned depends on the file creation order.
This means currently the order of execution for scripts inside clink-completions is not enforced, but .init.lua must be executed first since it's setting the path for
requirecalls made after.If for some reason somebody had their clink-completion folder content created in a different order (like it happened for me) then when Clink is started an error is printed:
module '<NAME>' not found.This can be tested by deleting and recreating .init.lua inside clink-completion folder after installation.
To avoid this issue I think it would be safer to always call .init.lua first before looping through the content of clink-completion. This is the change proposed with this pull request.
Note: this pull request replace #2276 made on the wrong branch.