Skip to content
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

Port deprecated urwid accessors #603

Merged
merged 2 commits into from
May 13, 2023
Merged

Conversation

alexfikl
Copy link
Contributor

urwid started being a bit more vocal about deprecations in urwid/urwid@8a0a6d1. 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
Copy link
Contributor Author

alexfikl commented May 11, 2023

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.

@alexfikl alexfikl force-pushed the port-deprecated-urwid branch from e262a98 to a716972 Compare May 11, 2023 16:57
@inducer
Copy link
Owner

inducer commented May 11, 2023

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 alexfikl force-pushed the port-deprecated-urwid branch from a716972 to fe58164 Compare May 11, 2023 17:22
@alexfikl alexfikl marked this pull request as ready for review May 12, 2023 15:31
@alexfikl
Copy link
Contributor Author

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

@inducer
Copy link
Owner

inducer commented May 12, 2023

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

@alexfikl
Copy link
Contributor Author

alexfikl commented May 12, 2023

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
urwid/urwid@05e1e6d#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
urwid/urwid@e3e013f

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

@inducer inducer merged commit 7642bea into inducer:main May 13, 2023
@inducer
Copy link
Owner

inducer commented May 13, 2023

Agreed. Thanks!

@alexfikl alexfikl deleted the port-deprecated-urwid branch May 15, 2023 17:26
inducer added a commit that referenced this pull request May 23, 2023
@inducer
Copy link
Owner

inducer commented May 23, 2023

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

inducer added a commit that referenced this pull request May 23, 2023
@alexfikl
Copy link
Contributor Author

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 😞

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants