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

Fire onDidChangeReplElements directly from ReplModel #81392

Merged
merged 1 commit into from Sep 26, 2019

Conversation

@dgozman
Copy link
Contributor

commented Sep 24, 2019

This way debug sessions sharing a single repl will all be notified about new repl elements and UI will be updated.

@isidorn Please take a look. This is a part of #81196 which makes sense by itself.

@isidorn

This comment has been minimized.

Copy link
Contributor

commented Sep 25, 2019

This is a good change. Can you please just resolve the conflict with the master branch and than I will merge this in? Thanks!

@isidorn isidorn added this to the September 2019 milestone Sep 25, 2019
@isidorn isidorn added the debug label Sep 25, 2019
This way debug sessions sharing a single repl will all be notified about new
repl elements and UI will be updated.
@dgozman dgozman force-pushed the dgozman:on-did-change-repl-elements branch from 3f02f13 to 6c1cebb Sep 25, 2019
@dgozman

This comment has been minimized.

Copy link
Contributor Author

commented Sep 25, 2019

Rebased on top of master.

@isidorn

This comment has been minimized.

Copy link
Contributor

commented Sep 26, 2019

Reveiwed again, looks great. Like how we now fire from a small number of places and it seems to cover all the cases as far as I could tell.

Thanks again for this PR

@isidorn isidorn merged commit b0d714d into microsoft:master Sep 26, 2019
2 checks passed
2 checks passed
VS Code #20190925.35 succeeded
Details
license/cla All CLA requirements met.
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.