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.

Sign up for GitHub

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

Jump to bottom

Fix activation of linters on unopened files caused by markdown plugin #80506

Merged
merged 11 commits into from
Sep 11, 2019
Merged

Fix activation of linters on unopened files caused by markdown plugin #80506

merged 11 commits into from
Sep 11, 2019

Conversation

solomatov
Copy link
Contributor

Here's the issue: #50874

Solution:

  • Bug was caused by invocations of vscode.workspace.openTextDocument during indexing process
  • I create a document index which maintains a map from uri to TextDocument
  • When we need a document, we
    • Check whether it's already loaded and if it is, we use loaded document
    • Otherwise we read it through fs API

@solomatov
Fix bug causing a large number of linters to be activated due to the …
9bacdba 
…markdown extension opening TextDocuments during indexing
@msftclas
Copy link

msftclas commented Sep 7, 2019 •
edited
Loading

CLA assistant check
All CLA requirements met.

OmarTawfik
OmarTawfik reviewed Sep 7, 2019
View reviewed changes
extensions/markdown-language-features/src/commands/openDocumentLink.ts Outdated Show resolved Hide resolved
extensions/markdown-language-features/src/docIndex.ts Outdated Show resolved Hide resolved
extensions/markdown-language-features/src/docIndex.ts Outdated Show resolved Hide resolved
extensions/markdown-language-features/src/features/workspaceSymbolProvider.ts Outdated Show resolved Hide resolved
extensions/markdown-language-features/src/features/workspaceSymbolProvider.ts Outdated Show resolved Hide resolved
extensions/markdown-language-features/src/features/workspaceSymbolProvider.ts Outdated Show resolved Hide resolved
extensions/markdown-language-features/src/lines.ts Outdated Show resolved Hide resolved
extensions/markdown-language-features/src/tableOfContentsProvider.ts Outdated Show resolved Hide resolved
extensions/markdown-language-features/src/tableOfContentsProvider.ts Outdated Show resolved Hide resolved
extensions/markdown-language-features/src/tableOfContentsProvider.ts Outdated Show resolved Hide resolved
@solomatov solomatov changed the title Fix activation of a large number of linters caused by markdown plugin Fix activation of linters on unopened files caused by markdown plugin Sep 7, 2019
@mjbvz mjbvz added this to the September 2019 milestone Sep 9, 2019
@solomatov
Copy link
Contributor Author

@mjbvz Do you have any other things to fix in the review?

mjbvz
mjbvz requested changes Sep 11, 2019
View reviewed changes
Copy link
Collaborator

@mjbvz mjbvz left a comment

Choose a reason for hiding this comment

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

This is going in the correct direction but want to make sure we don't make the more common uses of TableOfContentsProvider more inefficient. Let me know if you have any questions about the comments

extensions/markdown-language-features/src/lines.ts Outdated Show resolved Hide resolved
extensions/markdown-language-features/src/lines.ts Outdated Show resolved Hide resolved
extensions/markdown-language-features/src/features/workspaceSymbolProvider.ts Outdated Show resolved Hide resolved
extensions/markdown-language-features/src/tableOfContentsProvider.ts Outdated Show resolved Hide resolved
extensions/markdown-language-features/src/docIndex.ts Outdated Show resolved Hide resolved
@solomatov
Copy link
Contributor Author

@mjbvz It seems that I fixed all your suggestions. Thanks for them, they made the code better and much shorter.

@mjbvz mjbvz merged commit 5db460d into microsoft:master Sep 11, 2019
@mjbvz
Copy link
Collaborator

mjbvz commented Sep 11, 2019

Thanks for the PR and for the changes! Looks good to me.

This will be in the next VS Code insiders build and is scheduled to ship with VS Code 1.39

@mjbvz mjbvz mentioned this pull request Sep 11, 2019
Closed
@solomatov solomatov deleted the md-opendoc-bug branch September 12, 2019 01:30
@anthonydresser anthonydresser mentioned this pull request Sep 13, 2019
Merged
@github-actions github-actions bot locked and limited conversation to collaborators Mar 27, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@OmarTawfik OmarTawfik OmarTawfik left review comments

@mjbvz mjbvz mjbvz requested changes

Assignees

@mjbvz mjbvz

Labels
None yet
Projects
None yet
Milestone
September 2019
Development

Successfully merging this pull request may close these issues.
4 participants
@solomatov @msftclas @mjbvz @OmarTawfik