-
Notifications
You must be signed in to change notification settings - Fork 297
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
Rework Debug CodeLens to display on correct documents #198
Conversation
The Debug CodeLensProvider incorrectly adds CodeLenses with decorations that may or may not have been updated for the active text editor, and does not handle the case when there is more than one visible text editor. This change shuffles around the Debug CodeLens files, and moves the responsibility for fetching and caching test results into a new `TestResultProvider` class. The relevant methods to parse, join, and sort test results have been extracted from `JestExt`. To improve support for multiple visible text editors, when a test run is complete `updateWithData` will call `triggerUpdateDecorations` for all visible text editors.
@orta, the changelog check is working. |
Ace 👍 |
|
||
const filePath = document.fileName | ||
this.testResultProvider.removeCachedResults(filePath) | ||
} |
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.
nice, breaking up the monolith
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.
Thanks. Figured I'd practice some test-driven development. I'm happy we both like how it forces you to add seams to write tests. I'm curious if they'll help integrate changes in the long run or just end up "too fragile".
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.
likely the former, people can trust their changes work as expected
src/extension.ts
Outdated
}) | ||
}), | ||
|
||
vscode.workspace.onDidCloseTextDocument(extensionInstance.onDidCloseTextDocument) |
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 is throwing an error from onDidCloseTextDocument()
because this
is null
. 😠
Alright, things look good to me. I'll clone it and have a test look on some projects before shipping - nice work |
looks good - shipped. #116 looks important |
Rework Debug CodeLens to display on correct documents
The Debug CodeLensProvider incorrectly adds CodeLenses with decorations that may or may not have been updated for the active text editor, and does not handle the case when there is more than one visible text editor.
This change shuffles around the Debug CodeLens files, and moves the responsibility for fetching and caching test results into a new
TestResultProvider
class. The relevant methods to parse, join, and sorttest results have been extracted from
JestExt
.To improve support for multiple visible text editors, when a test run is complete
updateWithData
will calltriggerUpdateDecorations
for all visible text editors.This fixes #178.
No rush on this one, it's a meaty change. I'd rather we make sure we're invalidating the test result cache is invalidated properly before we ship.