Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

#

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Clink path get broken if clink-completions content is created in a different order #2278

Merged
merged 1 commit into from
Mar 14, 2020
Merged

Clink path get broken if clink-completions content is created in a different order #2278

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
2 changes: 2 additions & 0 deletions vendor/clink.lua
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Copy link
Contributor

Choose a reason for hiding this comment

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

@Mikaz-fr do you think you also need to skip reading .init.lua here as it has been already executed above?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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.

Copy link
Contributor

Choose a reason for hiding this comment

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

Sounds reasonable 👍 Let's keep it as-is then

Expand Down