Plugins (and persistent Watch expressions)

[Kevin-Prichard]

pudb has become a regular part of my Python development cycle, and I use it daily. Great, lightweight package that complements Python so well.

There are, however, a few small features things I'd like to add. Number one would be persistent Watch expressions. I'm sure we've all been there, debugging a tough problem with countless restarts of the program, and you gotta add those Watch expressions each time.

For that, @mnieber provided an automation recipe, via AutoKey, in #583 (comment). In that discussion, @inducer brings up the question "How can we better support user-driven automation? ("plugins"?) Should we?"

I'm here to respond to that. Yeah, there are a few choices:

1. Make use of an external utility to do a thing (this tends to be brittle, as pointed out by @inducer, and the features will be limited)
2. Modify pudb source to add the feature, and submit a pull to share it with the community -or, operate from your own fork/branch if not accepted
3. Create a way to extend pudb with plugins

Here are some thoughts that occurred when reading @inducer's response to that thread.

• What's a good UI for selecting variable names/other things in the source code? (Vimium-style? Urwid does have mouse support, even if we don't do very much with it ATM?)

Clicking on things like instance properties would be cool, though they'd need to be made visible somewhere in pudb's UI. While it would save some keystrokes over having to manually type in the reference, it is still limited by repetition.

• How can we better support user-driven automation? ("plugins"?) Should we?

I found this discussion topic because I wanted a way to persist Watch expressions across python/pudb sessions. Having to add them over and over gets tedious, of course. My thought process started like this- "But, wait- something similar is done for Breakpoints, right? Those are automatically persisted across sessions, loaded from filenames stored in saved-breakpoints-. and breakpoints-.. So, how could I do that for Watch expressions?"

One idea

• provide a plugin API of some sort, that grants access to certain pudb resources and features
• perhaps in the form of a pair of base classes-
  • PluginBase: developers subclass this to implement their plugin. It wouid be instantiated in class Debugger, and be provided references to some of the resources that allow the plugin to add features to pudb. Give it life cycle methods so developers can setup and breakdown their plugin's needs, and perhaps provide a way to persist arbitrary information across sessions.

What are appropriate ways to provide access to pudb resources?

• you could just provide direct access to the urwid objects that implement the different panels, but it could get messy with multiple code modifying them
• another way would be to supply the PluginBase subclass instance with a unified context object that acts as a proxy layer- going between between plugin developer code and the various resources held by Debugger / DebuggerUI. It would provide developers with the API for talking to pudb by providing proxy methods and properties that offer the features necessary to extend pudb, but gives you (@inducer) full control over scope and access level to pudb features and resources.

What could this look like?

class PluginBase:
    def ___init___(*args, **kwargs):
        pass

    @abc.abstract
    def setup(self, context: PluginContext):
        pass

    @abc.abstract
    def frame_notify(self, frame_info: FrameType):
        pass

    @abc.abstract
    def reset(self, ...):
        pass

    @abc.abstract
    def break_down(self):
        pass


class PluginContext:
    """
    Proxy class through which a Plugin can modify aspects of Debugger & DebuggerUI

    For safety, the instance properties of those classes should only be indirectly visible and modifiable via these methods and properties.  Direct references to Debugger/DebuggerUI and their resources should be avoid to prevent conflicts and giving up too much control.  The threshold for Python's "We're All Consenting Adults" sometimes is vague, but as a developer wanting to maintain the integrity of your package and control how it is used is never inappropriate, it's software architecture.
    """

    @property
    def var_list(self) -> FrozenSet[Variable]:
        ...

    @property
    def watch_list(self) -> FrozenSet[Watchable]:
        ...

    @property
    def breakpoint_list(self) -> FrozenSet[Breakpoint]:
        ...

    def add_watch(self, watch_exr: Union[Text|Watchable]):
    def is_watch(self, watch_exr: Union[Text|Watchable]):
    def rm_watch(self, watch_exr: Union[Text|Watchable]):
        ...

    def add_breakpoint(self, breakpoint: Union[Text|Breakpoint]):
    def is_breakpoint(self, breakpoint: Union[Text|Breakpoint]):
    def rm_breakpoint(self, breakpoint: Union[Text|Breakpoint]):
        ...

    def add_persistable(self, name: str, obj: Any):
        ...

    def add_hook(self, hook_type: str, callback: Callable):
        # provides ability to handle (or co-handle) some of class Debugger's and DebuggerUI's responsibilities

    def add_keyhandler(self, key_code: Text, callback: Callable, chain_or_override: bool=True):
        # provide way to extend or supplant the main UI's keystroke responses

One challenge for supporting something like this is that much of DebuggerUI's interactivity is essentially a private module defined inside of DebuggerUI.init(). For those data structures and functions to become available, they would need to be hoisted out and become either part of DebuggerUI's attributes, or perhaps another class that is a friend and collaborator (in the C++ sense, of shared access to structures and methods.) I would experiment with just hoisting them to DebuggerUI and then making private/protected those which aren't meant to be directly accessed by external code. However, that is what a proxy class such as PluginContext would do, provide access to those features that are useful to plugin developers.

If the skeleton class outlines I provided above seem incomplete, it is because I have not fully familiarized myself with all aspects of pudb's code, so the full set of features and insertion points that could be provided to PluginBase and PluginContext has yet to occur to me.

Having thought about this, for now I'll probably just modify pudb to provide Watch expression persistence, and maybe submit a PR when I'm happy with it. A big refactor of pudb's internal structure would impact its simplicity, and I don't see many requests or discussion for adding plugins.

UPDATE: I see that @lechat submitted a pull to persist Watch expressions... eight years ago, but it wasn't accepted. I'll see what can be learned from that pull, perhaps worthwhile to catch it up with inducer:main and hopefully get this seemingly useful feature accepted.

[inducer]

Thanks for all the thought you put into this. Compared to the discussion in #150, I'm more sympathetic to saving watch expressions out of the box. I think some of that code can probably be dusted off, but I still don't like the aspect that save_watches is called as part of a UI update.

[Kevin-Prichard]

Gotcha, thanks for considering! Yesterday I made a new PR #661 -which is mostly #150's commits- but I agree that invoking load_watches and save_watches on every UI update doesn't make sense. I mean, while debugging it isn't noticeable, but as an engineer it can be more efficient and improved upon. There also is a problem where deleting a watch doesn't always take, which makes me believe it's possible there's a timing or threading issue -one that might be cured by a) reading and writing only when necessary (as you desire), and b) if that doesn't cure the issue to throw a lock around the write. I'll try it in the morning.

[Kevin-Prichard]

I have moved saves so they occur only in the case of new watch insertion, Delete key in the vars pane, and Delete in the watch expression edit dialog. They all work, and writes to disk are minimized -however, there are a couple of related glitches that I am continuing to debug, so I am holding off on adding those commits to #661, until I've figured them out and fixed -- likely in the next 24 hours.

[mnieber]

Thanks guys, looking forward to this improvement!

[Kevin-Prichard]

I think the pull is ready for review (#661). Note that I pivoted the code a bit, by consolidating all the watch expression persisting code into a central class (Watches). This is meant to eliminate redundancies, cure a problem with attaching watch expressions to stack frames, reduce reads and writes from/to the watch expressions file, and preserve the functionality of the original pull that started this journey (#150).

Please offer comment and critique, and please test the PR branch. In case you are not familiar, in wherever you are doing some python development, such as a virtual environment, after cloning this repo and checkout my dev branch-

# Clone the repo, if you don't already have it locally
$ cd /path/to/development  # to wherever you clone external projects
$ git clone https://github.com/inducer/pudb pudb-dev
$ cd pudb-dev
$ git pull origin

# Add my remote
$ git remote add kevinp git@github.com:Kevin-Prichard/pudb
$ git pull kevinp featdev-persistent-watch
$ git checkout featdev-persistent-watch

# Remove existing pudb installation from your current Python environment, if installed
$ cd /to/your/project
$ pip uninstall pudb

# Add the development version
$ pip install --editable /path/to/development/pudb-dev

Your Python environment should have pudb from my pull, now -let me know if any snags.

[jimn]

Thanks for doing this work @Kevin-Prichard