-
-
Notifications
You must be signed in to change notification settings - Fork 31.6k
IDLE, configdialog: Factor out GenTab class from ConfigDialog #75233
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
Comments
Followup to 31003, tests, and 30853, tracers, similar to 31004, FontTab. After creating new class, we can change names without worry about clashing with names elsewhere in ConfigDialog. |
Now that we know what we are doing, we can simplify the steps. These assume that bpo-21004, PR2905, which prepares create_widgets and fixes the GeneralTest that was broken by Notebook, has been merged. For configdialog:
For test_configdialog:
|
Made all the changes without any issue. One thing - I noticed that the var_changed_autosave, etc for the General tab were back, even though they aren't used by VarTrace. I know I had deleted them, so I'm not sure how they came back. Unless you re-added them? That's what happened to me yesterday -- changes I knew I had made were back again. Anyway, if you want them back, sorry about deleting them again. I just figured it was something that happened on my end. Also, I snuck in a change to the Notebook. I know it should be in its own PR. If you like it, I can leave it here or make a new PR. :-) |
Above, I left out the last step, "* remove old general block". The old configdialog had an unwritten and undocumented template and rules for pages on the dialog. It went something like the following.
What I said in the message above is that we have redefined the template well enough to follow it. We have a mostly abstract template. class NamePage(Frame):
def __init__(self, master):
super().__init__(master)
self.create_page_name()
self.load_name_cfg()
... Note that the parameter is 'master', not 'parent', as is standard for widgets. It is also not saved, as it is not needed. TkVars for the page can use self as master. I considered making that an actual base class. But there is little common code; it does not quite fit FontPage; and would require uniform 'create_page' and 'load_cfg' names. The more specific names are easier to find with ^F ;-). I do want to add a comment block above FontPage giving the design. I also want to change FontPage to use self as master for tk vars and other widgets. Lets freeze the General page block until this is done. |
I did not refresh this before writing the above. It still applies, including deleting the commented out code once tests pass. See review. Whoops: I forgot that using make_default means that we can (and should) delete the replaced var_changed defs. I added the one for space_num back because I mistakenly thought it was needed to make the test pass. I will open a new issue to add a comment documenting the new TabPage(Frame) class design. The new rules will include using default var_changes when possible. Patch will include adjusting FontTab to follow the new rules. |
I intentionally left using non-default options other than traversal for a new issue or issues, after we finish ttk replacement. I imagine there will be discussions about choices, and I will invite other participants. I would like to direct visual design discussions, as opposed to internal implementation discussion, to idle_dev. |
Much easier the 2nd time ;-) |
Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.
Show more details
GitHub fields:
bugs.python.org fields:
The text was updated successfully, but these errors were encountered: