Store model text in memory #121
Closed
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
force-pushed
the
refactor-smithy-project
branch
23 times, most recently
from
5cfa250
to
afba16c
Compare
milesziemer
force-pushed
the
refactor-smithy-project
branch
from
afba16c
to
741cc51
Compare
JordonPhillips
approved these changes
Nov 2, 2023
Comment on lines
176
to
177
| if (!existingSourceFiles.contains(new File(changedUri))) { | ||
| existingSourceFiles.add(new File(changedUri)); |
There was a problem hiding this comment.
nit: introduce variable rather than constructing the file twice
| return new HashMap<>(); | ||
| } | ||
|
|
||
| public ValidatedResult<Model> getModel() { |
There was a problem hiding this comment.
public methods should have docs
This is primarily a refactor of SmithyProject to store raw model text in memory. Previously, we only stored file objects for the model, and gave those to the model assembler. There are two issues with this approach: 1. While a file is being editted, the source of truth for the file contents should be what the client sends back to the server in the `didChange` event, not whats actually in the file. To get around this, we've been using a temporary file that stores the contents of the file being editted, and giving that to the model assembler. When publishing diagnostics however, we needed to re-map the file location from the temporary file to the actual file on disk. 2. The language server needs literal text to provide some of its features, meaning we need to read in files frequently. These changes update SmithyProject to store a map of URI -> String for each of the model files in the loaded model by walking the shapes and collecting all unique file locations. SmithyTDS is updated to use that when it needs to have literal text of the model for formatting, version diagnostics, and other features. Various other updates were also made: - Added a list of errors to SmithyProject that store any errors that occur while loading the project. Previously, any "load" methods would return `Either<Exception, SmithyProject>`, which isn't very ergonomic to use. These errors represent failures in the project loading, not validation errors from loading the model. - SmithyProject's loading methods are reworked to provide 2 static methods: one for loading the project in a directory, one for loading from specific smithy-build config, and 2 instance methods: one for regular reloading, one for reloading with specific file changes. Previously, we had to manage the loading of the temporary file (or its removal). This also moves all the loading logic into SmithyProject. - String-literal uris are replaced with URI class since LSP guarantees uris will be valid (see https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17/specification/#uri). - SmithyInterface has new method to load from text-literal sources. - Completions updated to handle invalid model result, so callers don't have to. This allows removing `getCompletions` from SmithyProject. - Various access modifiers adjusted to be more strict, tightening the API to make it easier for us to change implementations. The following test updates were made: - Added multiple test cases for different project loading & reloading situations. - Some tests needed to use URI class instead of uri strings.
milesziemer
force-pushed
the
refactor-smithy-project
branch
from
741cc51
to
1bbe8e6
Compare
|
Done in #146 |
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 is primarily a refactor of SmithyProject to store raw model text in memory. Previously, we only stored file objects for the model, and gave those to the model assembler. There are two issues with this approach:
didChangeevent, not whats actually in the file. To get around this, we've been using a temporary file that stores the contents of the file being editted, and giving that to the model assembler. When publishing diagnostics however, we needed to re-map the file location from the temporary file to the actual file on disk.These changes update SmithyProject to store a map of URI -> String for each of the model files in the loaded model by walking the shapes and collecting all unique file locations. SmithyTDS is updated to use that when it needs to have literal text of the model for formatting, version diagnostics, and other features.
Various other updates were also made:
Either<Exception, SmithyProject>, which isn't very ergonomic to use. These errors represent failures in the project loading, not validation errors from loading the model.getCompletionsfrom SmithyProject.The following test updates were made:
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.