Persistent watch expressions (continuation of #150) #661
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Kevin-Prichard
force-pushed
the
featdev-persistent-watch
branch
from
1ae0f4f to
336b854
Compare
|
Given that the checks have been in an "expected - waiting" state for two weeks, It seems like the CI is hung. |
|
No, sorry, that was me. I needed to click "Approve". |
|
(But I've been a bit preoccupied with the whole thing being quite broken on 3.13: #664) |
…covered bug after merging lechat:save_load_watches into inducer:main
…d by the user, making that item become de-persisted. This fixes an issue where the delete key tends to do nothing in the variables Watch expression subpane, following merge of lechat:save_load_watches into inducer:main. The issue occurs because load_watches() is called every time make_var_view() is invoked by update_var_view(), which itself is invoked frequently -whenever the UI needs redrawing. So, the removal of an expression by the delete key lasts for only approximately two shakes of a lamb's tail.
…code into new class Watches, acting as global container - this resolves the previous dual, competing watch sources: the list FrameVarInfo.watches, plus the file maintained by save_watches() and load_watches(). FrameVarInfo.watches caused apparent non-determinism in the Watches pane, because it was associated with a particular stack frame, so whatever was in .watches on a particular frame would get displayed while the debugger was in that state, but would disappear from the pane when the execution point caused that frame to be pushed or popped. So, in removing FrameVarInfo.watches and consolidating all WatchExpression storage into the single, global container Watches, the watches pane remains consistent no matter the current frame. Caveat that there can be expressions which won't evaluate in the current frame context, of course. That's the trade-off, and I hope that makes sense to other developers.
…ble to new and upgraded pudb users
…and has methods; add some type checking; use dict.get for CONFIG instead of direct item ref. - pudb/test/test_var_view.py: add tests for classes WatchExpression and Watches
…s instead of single quotes
Kevin-Prichard
force-pushed
the
featdev-persistent-watch
branch
from
a817ddc to
03da90e
Compare
|
@inducer I have fixed the ruff and pylint issues within my PR branch, ready for another go |
|
What can I do to help move this to acceptance, @inducer ? I think the workflow needs approval, for one |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
UPDATE
There was a problem with determinism in the watch pane, caused by there being two containers for WatchExpressions: FrameVarInfo.watches (watches specific to one stack frame), and the file (~/.config/pudb/saved-watches-*) maintained by save_watches() -a list of global watches. This combination would occasionally cause all watches to be deleted from the file, because the prior rev sometimes read the watches from the FrameVarInfo.watches which almost never contained the same set of expressions as the file.
This behavior prompted me to consolidate all WatchExpression related code into one class (Watches in var_view.py) to provide a central manager of Watches, rather that behavior being dispersed to various points in the UI code. Easier to maintain, and control the behavior regardless of what code is invoking it.
Summary
Persistence of watch expressions. This pull incorporates and continues with #150's fine work. Mentioned in discussion #660 - Plugins (and persistent Watch expressions)
Details
one tiny mod to var_view.py: to import pudb.debugger.CONFIG into local scope, to fix bug discovered after the mergeanother tiny mod to debugger.py: call save_watches() after a watch expression is deleted by the user, making that item become de-persisted from disk. This fixes an issue where the delete key tends to do nothing in the variables Watch expression subpane, following merge of lechat:save_load_watches into inducer:mainthe issue occurs because load_watches() is called every time make_var_view() is invoked by update_var_view(), which itself is invoked frequently -whenever the UI needs redrawing. So, the removal of an expression by the delete key lasts for only approximately two shakes of a lamb's tailTests