Rationalising info and confirmation dialogs in aqt.utils
- Dominant language
- Rust
- Stars
- 30.5k
- Forks
- 3.2k
- Avg merge
- 2d 8h
- Merged PRs (30d)
- 60
Description
I've been looking at a number of the dialogs in `qt/aqt/utils.py` as the next target for removing `exec()`, specifically those showing information or getting a response from the user, but looking over them I think a bigger refactoring might be worthwhile. The main issues I see are:
- A lot of overlap between functions
- Many functions doing too much
- Ineffective split between class and function code
To fix this I'd like to:
- Create a new `InfoDialog` class for displaying a message dialog with a confirmation button, and a few convenience functions along the lines of `showWarning`, `showCritical`, `showInfo`, `showText`. Existing callers of those functions that simply need to show some information can be easily transitioned.
- Rewrite `ButtonedDialog` to utilise callbacks (to avoid the use of exec), with new convenience functions that replace `askUser`, `askUserDialog`, as well as `showInfo` where that is used currently to get a confirmation from the user (e.g. in `aqt.addons.installAddonPackages`).
I think a cleaner split between showing information and getting confirmation/answer from the user would greatly simplify this part of the code, although I realise these functions are used in a lot of places and there may need to be more variations than listed above. In those cases, at least, things can still be improved with simpler and smaller scoped functions, letting the classes do the heavy lifting behind the scenes where necessary.
Some of the calls to showInfo and showText could instead be replaced with tooltips e.g. when a user tries to rebuild or empty a regular deck, or tries to undo when there's nothing to undo - these situations probably don't warrant a dialog popup in my opinion. This can be discussed in the PRs of course.
I assume a fair few add-ons use these functions so they'll have to stick around for a while, but I still think they are worth replacing rather than refactoring. New functions makes it easier for existing callers to be moved incrementally, and they'd otherwise end up more complicated rather than less.
I'll submit the changes incrementally, implementing the new functions and transitioning a few callers at a time to keep things manageable. I'm sure you're busy with 2.1.50, so no rush with this, but I wanted to get the go ahead and any guidance you might have before I dive in it since the changes are larger in scope and will take a bit of work.
Contributor guide
Assessment
This issue has not been assessed yet.