Skip to content
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

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

milesziemer
Copy link
Contributor

@milesziemer milesziemer commented Sep 25, 2024

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.

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.

@milesziemer milesziemer requested a review from a team as a code owner September 25, 2024 17:26
@milesziemer milesziemer requested review from kstich and hpmellema and removed request for kstich September 25, 2024 17:26
@milesziemer milesziemer force-pushed the multi-root-workspaces branch 4 times, most recently from 6b1e516 to 2bdab11 Compare September 25, 2024 19:26
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.
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.
@cgeduhn
Copy link

cgeduhn commented Sep 27, 2024

We Are looking forward for this 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants