-
Notifications
You must be signed in to change notification settings - Fork 20
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
Support projects in subdirectories #167
Merged
milesziemer
merged 3 commits into
smithy-lang:main
from
milesziemer:multi-root-workspaces
Oct 28, 2024
Merged
Support projects in subdirectories #167
milesziemer
merged 3 commits into
smithy-lang:main
from
milesziemer:multi-root-workspaces
Oct 28, 2024
Conversation
This file contains 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
milesziemer
requested review from
kstich and
hpmellema
and removed request for
kstich
September 25, 2024 17:26
milesziemer
force-pushed
the
multi-root-workspaces
branch
4 times, most recently
from
September 25, 2024 19:26
6b1e516
to
2bdab11
Compare
This commit makes the language server work with Smithy projects in subdirectories of the folder (or folders, in the case of e.g. vscode workspace folders) open in the editor. I previously added support for multiple _workspace folders_ (an LSP concept), but I assumed only one _project_ (Smithy LS concept) per workspace folder. So this commit fixes that mixing, allowing many projects per workspace folder. Now, the language server will search through subdirectories of all workspace folders (by default, one workspace folder is open in the client) to find projects. Changes to build files, i.e. smithy-build.json, .smithy-project.json, are now tracked at the workspace level, so you can add a new project to an existing workspace. I also did _some_ cleanup of the project/workspace synchronization code, and moved some things around. A note on some tests: I'm using a `Files.createTempDirectory`, and adding the `TestWorkspace` as a subdir. In a follow-up commit, I will be changing `TestWorkspace` to be something like `TestProject`, which is more accurate. I didn't include it here to avoid a bunch of noise.
milesziemer
force-pushed
the
multi-root-workspaces
branch
from
September 25, 2024 20:30
2bdab11
to
f7d843e
Compare
Blocking on the future creating the progress token with the client means the server can't actually receive the response from the client for that request. Tests don't catch this because the mock client is called directly, rather than through the server proxy. I decided to just remove the progress token code for now so didChangeWorkspaceFolders can work at all, rather than trying to make the method work asynchronously, which is a larger lift considering it mutates the server state. That change is coming though.
We Are looking forward for this 🎉 |
hpmellema
requested changes
Oct 28, 2024
src/main/java/software/amazon/smithy/lsp/ProjectRootVisitor.java
Outdated
Show resolved
Hide resolved
hpmellema
reviewed
Oct 28, 2024
milesziemer
force-pushed
the
multi-root-workspaces
branch
from
October 28, 2024 20:46
b716764
to
b2ffd65
Compare
hpmellema
approved these changes
Oct 28, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
This commit makes the language server work with Smithy projects in subdirectories of the folder (or folders, in the case of e.g. vscode workspace folders) open in the editor. I previously added support for multiple workspace folders (an LSP concept), but I assumed only one project (Smithy LS concept) per workspace folder. So this commit fixes that mixing, allowing many projects per workspace folder.
Now, the language server will search through subdirectories of all workspace folders (by default, one workspace folder is open in the client) to find projects. Changes to build files, i.e. smithy-build.json, .smithy-project.json, are now tracked at the workspace level, so you can add a new project to an existing workspace.
I also did some cleanup of the project/workspace synchronization code, and moved some things around. A note on some tests: I'm using a
Files.createTempDirectory
, and adding theTestWorkspace
as a subdir. In a follow-up commit, I will be changingTestWorkspace
to be something likeTestProject
, which is more accurate. I didn't include it here to avoid a bunch of noise.Also:
I fixed a bug in #160 that caused a deadlock.
Issue #, if available:
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.