-
Notifications
You must be signed in to change notification settings - Fork 1
Fix document manager constructor not updating cached values #294
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
base: main
Are you sure you want to change the base?
Conversation
3655caf to
a63b0ba
Compare
a63b0ba to
56095b7
Compare
|
Hold off on merging this until release bugs are fixed |
03e47b0 to
9f49305
Compare
9f49305 to
b5069bb
Compare
ea1654e to
edf7021
Compare
edf7021 to
12faa49
Compare
12faa49 to
29a517a
Compare
be0c017 to
b819f6a
Compare
b819f6a to
ee4b2e6
Compare
c4d5444 to
fd3e4a2
Compare
fd3e4a2 to
31e34a6
Compare
31e34a6 to
5e45c96
Compare
6c85d51 to
7620b6f
Compare
7620b6f to
47ac74c
Compare
5a6cbd1 to
bb9f841
Compare
bb9f841 to
08dc41f
Compare
| s3Service, | ||
| document.documentType, | ||
| document.uri, | ||
| document.contents() ?? '', |
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 should probably throw at line 100
| } | ||
|
|
||
| const content = document.contents(); | ||
| if (!content) { |
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 sill be true if contents is both undefined or empty which breaks line 33
| document.documentType, | ||
| document.uri, | ||
| document.contents(), | ||
| document.contents() ?? '', |
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 should be checked at like 92
| // Replace entire file with properly formatted JSON | ||
| snippetText = docFormattedText; | ||
| const endPosition = { line: document.lineCount, character: 0 }; | ||
| const endPosition = { line: document.lineCount ?? 0, character: 0 }; |
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.
Can we exit early if theres no document?
Issue #, if available:
DocumentsinsideDocumentManagerwhich cache the document object during creation and points to theTextDocumentreferencelanguageId,versionand 'lineCount' were only initialized in the constructor and never updated which caused a stale value issueDescription of changes:
Documentconstructor to pass in theTextDocumentas a function and get all the above mentioned values throughgetreference instead of initializing it in constructor.By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.