search

inducer / pudb

Full-screen console debugger for Python
https://documen.tician.de/pudb/
Other
2.94k stars 226 forks source link

Port deprecated urwid accessors #603

Closed alexfikl closed 1 year ago

alexfikl commented 1 year ago

urwid started being a bit more vocal about deprecations in https://github.com/urwid/urwid/commit/8a0a6d166e4cef88a7e7b644abee8712eacee7f6. The recommended variants (e.g. w.focus vs w.get_focus()) seem to have been around for a while, so this just ports everything to use the property-based interface.

I am very unsure this is correct, so probably needs some proper testing or a knowledgeable reviewer. I tried using most shortcuts I know and opening things (variables, stack, breakpoints, etc) and it seems to work.

alexfikl commented 1 year ago

The deprecations from urwid started being spammed in the command line widget. Is that intended?

EDIT: I have PYTHONWARNINGS=default set in the environment, so this is not something that would normally happen.

inducer commented 1 year ago

The deprecations from urwid started being spammed in the command line widget. Is that a intended?

It's not not intended. :) It's certainly better than having them land on just stdout/stderr (where they would haphazardly overwrite the UI).

alexfikl commented 1 year ago

@inducer This should be good to go for a review.

inducer commented 1 year ago

Do we know when urwid introduced these constants? (And should we set a version bound to make sure we have them?)

alexfikl commented 1 year ago

Do we know when urwid introduced these constants? (And should we set a version bound to make sure we have them?)

Do you mean those urwid.WEIGHT things? Those were introduced over a decade ago, e.g. in https://github.com/urwid/urwid/commit/05e1e6d52d6f70c85bbcf67d9625f3e1a694cb3e#diff-29a00abb3c654d8ca892d9b10e535a2342f0e403192cbcf8f164420fe66f8bc8 but some seem to be older. The properties like widget.focus seem to also have been mostly introduced a long time ago, e.g. in https://github.com/urwid/urwid/commit/e3e013f716a8783b7685c3884a71eb959f3c81c2

So it seems safe to just let them be at this point?

inducer commented 1 year ago

Agreed. Thanks!

inducer commented 1 year ago

Turns out this was still quite broken in places. #604 fixes up the issues that I'm aware of.

alexfikl commented 1 year ago

Turns out this was still quite broken in places. #604 fixes up the issues that I'm aware of.

Ouf, sorry about that (and thanks for fixing it)! Feel free to ping me if something's still broken :disappointed: