-
Notifications
You must be signed in to change notification settings - Fork 230
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
Added auto save/load of watch expressions #150
base: main
Are you sure you want to change the base?
Conversation
Thanks for your contribution! I'm not completely sure about this functionality though. I think of a watch expression as something you add while poking around in a single function--and so having them saved globally sort of assumes that you're going to be poking around the same function after the next restart of the debugger. I'm also not quite sure I like how the code is written, since the saving happens as part of what is supposed to be a UI update. The pain point solved by this is that retyping watch expressions is cumbersome and takes time. An alternate UI design might be to keep a history of watch expressions that a user typed in in the past and to offer those up when a new watch expression is being created. It would probably be nice to make it efficient to quickly add multiple expressions from that history in one go. What do you think? |
ca1dd37
to
755119c
Compare
755119c
to
821a9b0
Compare
I think that option to have persistent watch expressions is very important for me. I'm entering the same function over and over and over, and writing the same things is annoying. Also, it would be nice if console remember history (for same reasons) and panels remember their sizes (but it's BTW) |
I think you could say the same thing about breakpoints. Once you're done debugging something you don't really need your breakpoints any more. But while debugging, it's good to have them saved, because you typically have to restart the debugger many times. So I think saving them is nice, as long as it's also easy to clear them once you no longer need them. |
Breakpoints are auto-saved out of the box. |
821a9b0
to
1ad4f86
Compare
@lechat Thanks a lot for this contribution. It's a saver. I had to start debugger over and over with several watch expressions. Having to type them each time was really annoying. |
1ad4f86
to
2da2020
Compare
I also like to see this in the upstream, which makes the debugger much useful for me. When I debug a function, I need to restart the program multiple times to see if my modification is effective. In addition, I need to track multiple variables inside a function. Retyping those variables in a watch every time is not acceptable for me. For now, I need to print them on the screen. I wonder what is the designed workflow of |
@@ -544,6 +544,23 @@ def make_var_view(frame_var_info, locals, globals): | |||
ret_walker = BasicValueWalker(frame_var_info) | |||
watch_widget_list = [] | |||
|
|||
if CONFIG["persist_watches"]: |
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.
At this point CONFIG isn't in scope (on inducer:main today), so it needs to be imported
Interestingly, here in the future this patch still works when applied to inducer:main. One tiny exceptions, the var pudb.debugger.CONFIG needs importing in var_view.make_var_view. One other minor issue, it's hard to delete a watch expression now. Seems that once it gets written to ~/.config/pudb/saved-watches-#.#, it keeps coming back. load_watches() is invoked every time a next statement is executed, and a deleted expression is restored into pudb. Manually editing ~/.config/pudb/saved-watches-#.# is a short-term work-around. I made a commit for the above two issues, which includes calling save_watches() after delete is pressed in the Watch pane. But the branch for this PR is too old, so I am submitting it, along with lechat:save_load_watches merged into it, as a new PR. Unless you'd rather take it over from here, it's your original work @lechat. Update: pull added #661 |
No description provided.