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

Added a map of open documents to the languageServerBase class. It is … #5019

Closed
wants to merge 2 commits into from

Conversation

erictraut
Copy link
Collaborator

…responsible for applying any delta changes and updating the contents for each of the workspaces associated with that file. This allows source code contents to be shared between workspaces, which reduces memory usage. It also eliminates a correctness issue that can occur if new workspaces are added after files are open. It addresses #4994.

…responsible for applying any delta changes and updating the contents for each of the workspaces associated with that file. This allows source code contents to be shared between workspaces, which reduces memory usage. It also eliminates a correctness issue that can occur if new workspaces are added after files are open. It addresses #4994.
…ol matching when there are multiple levels of protocol inheritance and a method that uses `Self` in its signature. This addresses #4966.
@@ -1210,6 +1233,8 @@ export abstract class LanguageServerBase implements LanguageServerInterface {
workspaces.forEach((w) => {
w.service.setFileClosed(filePath);
});

this.openFileMap.delete(filePath);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should we clear 'openFileMap' in 'onShutdown' ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think there's a need to do so, but it probably also doesn't hurt.

@@ -318,7 +317,7 @@ export class AnalyzerService {
updateOpenFileContents(
path: string,
version: number | null,
contents: TextDocumentContentChangeEvent[],
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@heejaechang I believe this would mean we'd have to do updates for say fixall differently. Maybe other spots too.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this works fine for the current fixall strategy. These changes are applied only to source files associated with a cloned program.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh it looks like fixall uses something you already fixed. the workspaceEditUtils.applyDocumentChanges.

@erictraut erictraut closed this Apr 27, 2023
@erictraut erictraut deleted the shared-source-contents branch April 27, 2023 16:55
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.

5 participants