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

bugfix: Remove old semanticdb files when not needed #5789

Merged
merged 1 commit into from
Oct 26, 2023

Conversation

tgodzik
Copy link
Contributor

@tgodzik tgodzik commented Oct 26, 2023

Previously, we would try to remove semanticdb files from indexes while we used the actual source file. Now, we properly remove files in all indexes and also from interactive semanticdb.

Related to #5748

Previously, we would try to remove semanticdb files from indexes while we used the actual source file. Now, we properly remove files in all indexes and also from interactive semanticdb
Copy link
Contributor

@kasiaMarek kasiaMarek left a comment

Choose a reason for hiding this comment

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

Otherwise looks good

@@ -1168,6 +1168,7 @@ class MetalsLspService(
compilers.didClose(path)
trees.didClose(path)
diagnostics.onClose(path)
interactiveSemanticdbs.onClose(path)
Copy link
Contributor

Choose a reason for hiding this comment

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

There is also def onDelete, shouldn't this be done there too?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think this should be fine. If it's deleted and was open we will first get close notification. If a file is deleted that is not open it will not end up in InteractiveSemanticdb in the first place.

@tgodzik tgodzik merged commit c06e806 into scalameta:main Oct 26, 2023
@tgodzik tgodzik deleted the remove-properly branch October 26, 2023 15:18
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.

3 participants