-
Notifications
You must be signed in to change notification settings - Fork 208
Add path completion support inside require
calls
#259
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
Conversation
01e6e47
to
9489f44
Compare
This is awesome ❤️ . I wonder if it would be worth finding files in the current directory as well? For example, if you create a brand new file and want to require it, it won't be in the load path yet. |
9489f44
to
74e9cc4
Compare
require
callsrequire
calls
@Morriar and I reworked this PR so that it uses a more efficient Trie (which is also called a Prefix Tree) data structure to index files in the load path and brought it up to work against the changes in latest @vinistock Currently we reindex the files on the load path for every request to completion, so we would be able to discover any newly added files. I know this is not the most efficient way of doing it, but based on my usage testing the responsiveness seems to be just fine for now. |
74e9cc4
to
0c7537b
Compare
0c7537b
to
c776598
Compare
05f14b9
to
98db569
Compare
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.
This works really well! Some comments about the code structure
Co-authored-by: Ufuk Kayserilioglu <ufuk@paralaus.com> Signed-off-by: Alexandre Terrasa <alexandre.terrasa@shopify.com>
98db569
to
4abb52c
Compare
Signed-off-by: Alexandre Terrasa <alexandre.terrasa@shopify.com> Co-authored-by: Ufuk Kayserilioglu <ufuk@paralaus.com>
4abb52c
to
076ebc6
Compare
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.
Amazing!! 🚀
…ypescript-eslint/eslint-plugin-5.40.1 Bump @typescript-eslint/eslint-plugin from 5.40.0 to 5.40.1
* Remove mention of `ruby_lsp_rails.server` from readme * Fix link in readme
Motivation
I got inspired by the recently released Markdown Language Server doing completion for references to other files. I realized that we can match
require
strings against a list of files in the$LOAD_PATH
and suggest completions like that.Note: This is still missing some of the functionality, but since it is gated behind a feature flag that should be turned on by the VSCode extension, I think it is safe to land. The things that are missing are:
.
or..
which make them relative requires.Rails.root / "lib"
andRails.root / "app/models"
, etc to the load paths, which this extension will not be able to see (since we don't boot the application we are running inside).require_relative
.Implementation
/
characterrequire
call.$LOAD_PATH
and offer completions.Automated Tests
Added a small automated test
Manual Tests
package.json
file and a:ruby-lsp
project in the Extension Host window.require
s and see autocompletion for paths.