-
Notifications
You must be signed in to change notification settings - Fork 13
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
Allow memory display configuration per memory view instance #64
Allow memory display configuration per memory view instance #64
Conversation
* Defaults from VS Code setting are initially set from main * From then on, memory display configuration lives in the view state * Remove previous messenger-based handling to change configuration * Consolidate column activation and other display configurations into one type * Add reset button to options which re-reads defaults and sets it to view state Fixes eclipse-cdt-cloud#60
@colin-grant-work Thank you very much for your swift review! You're right the column visibility / activation was somewhat overlapping. I wanted to centralize the view settings specified from the outside (main) including the column visibility so that initializing and resetting from the outside can be done with one event without having to handle column visibility and other settings separately. At the same time I didn't want to change the column contribution service's activation handling, expecting it to be (evolved into) something separate. I've now consolidated the column visibility and activation. Both is effectively managed in the column contribution service owned by the Thus, there are two interfaces:
Consequently, Please let me know what you think! Thanks again! |
From the description I think the idea is that the changes to the VSCode setting shouldn't take effect until the webview asks to be reset to default, but at the moment, it appears that things aren't working quite as intended, and I believe it has to do with the lifecycle of VSCode webviews. In the following sequence:
I think that when revealed, the webview is behaving as though it's new. |
@colin-grant-work Thanks again for testing! Unfortunately, I can't reproduce the issue though. I'm surprised that this happens for you, since we are setting memory-inspector-state.mp4 |
Odd - I'm also no longer able to reproduce it. Not sure what was going on in the one session where it happened consistently. |
What it does
Implements the following behavior for memory display options:
Implementation-wise this applies the following changes:
Fixes #60
How to test
Review checklist
Reminder for reviewers