Clink path get broken if clink-completions content is created in a different order #2276
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
(and some comments on the user-profile.cmd file) Inspired by the comments from #193 and my personal need to use pageant instead of OpenSSH authentication agents (which is more Window user-friendly), I have used this approach which works as expected. Keeping the spirit of the current scripts, I left it disabled, and with some comments explaining what they all do.
…nt, leading to .init.lua not being run first lua path being broken
Contributor
Author
|
Not sure why 4243c5c is coming with this pull request, my changes have nothing to do with init.bat (I just branched from development branch directly). |
Member
|
@Mikaz-fr Branch from master. We do not use development any longer. Sorry for the confusion. |
Contributor
Author
|
I understand, would you update the Contributing guidelines ? |
Contributor
Author
|
Closing this one, I've opened #2278 on master instead. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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 that I followed the Contributing Guidelines (https://github.com/cmderdev/cmder/blob/master/CONTRIBUTING.md) but it seems that the development branch to use for pull request is quite outdated compared to master. Let me know if I should have done differently.