Skip to content

[#2297] [WIP] Ensure that LS progress initialize is not duplicated. #2606

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

Closed
wants to merge 2 commits into from

Conversation

d3r3kk
Copy link

@d3r3kk d3r3kk commented Sep 17, 2018

  • Should help with the 'infinite analyzing' issues we are seeing
  • May still have some work to do catching 'stateChange' messages

Fixes #2297

  • Pull request represents a single change (i.e. not fixing disparate/unrelated things in a single PR)
  • Title summarizes what is changing
  • Has a news entry file (remember to thank yourself!)
  • Has unit tests & system/integration tests
  • Handle stateChange message and respond to stopped state indicatingthe LS has terminated.
  • Any new/changed dependencies in package.json are pinned (e.g. "1.2.3", not "^1.2.3" for the specified version)
  • package-lock.json has been regenerated by running npm install (if dependencies have changed)

- Should help with the 'infinite analyzing' issues we are seeing
- May still have some work to do catching 'stateChange' messages
@d3r3kk d3r3kk closed this Sep 19, 2018
@d3r3kk d3r3kk reopened this Sep 19, 2018
// tslint:disable-next-line:no-suspicious-comment
// TODO: (from https://github.com/Microsoft/vscode-python/pull/2597#discussion_r217892043)
// For #2297 (while most of the problem is not here, restart is rare) you need
// to track 'stateChange' on the language client. When it gets to 'stopped' LS has terminated
}
Copy link
Author

Choose a reason for hiding this comment

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

@MikhailArkhipov is there something here I should do to improve things further?

I required this fix in my prior PR to get the behaviour I needed unfortunately.

I think it's best to close this PR unless I need to capture the 'stopped' state.

@@ -29,7 +29,7 @@ export class ProgressReporting {
});

this.languageClient.onNotification('python/beginProgress', async _ => {
if (this.progressDeferred) {
if (this.progressDeferred) { // if we restarted, no worries as reporting will still funnel to the same place.

Choose a reason for hiding this comment

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

There's no change apart from adding a comment!

Copy link

@DonJayamanne DonJayamanne left a comment

Choose a reason for hiding this comment

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

can't see any changes in here

@d3r3kk
Copy link
Author

d3r3kk commented Sep 24, 2018

No need for this PR.

@d3r3kk d3r3kk closed this Sep 24, 2018
@d3r3kk d3r3kk deleted the no_dupe_progress_updates branch September 24, 2018 17:38
@lock lock bot locked as resolved and limited conversation to collaborators Jul 31, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

New language server seems to be frozen at analyzing workspace for more than 30 minutes
2 participants