IDLE, configdialog: Factor out GenTab class from ConfigDialog

[terryjreedy]

BPO	31050
Nosy	@terryjreedy, @csabella
PRs	bpo-31050: IDLE: Factor GenPage class from ConfigDialog #2952 [3.6] bpo-31050: IDLE: Factor GenPage class from ConfigDialog (GH-2952) #2955
Dependencies	bpo-31004: IDLE, configdialog: Factor out FontTab class from ConfigDialog

^{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:

assignee = 'https://github.com/terryjreedy'
closed_at = <Date 2017-07-30.23:07:56.389>
created_at = <Date 2017-07-26.19:18:52.478>
labels = ['expert-IDLE', 'type-feature', '3.7']
title = 'IDLE, configdialog: Factor out GenTab class from ConfigDialog'
updated_at = <Date 2017-07-30.23:07:56.388>
user = 'https://github.com/terryjreedy'

bugs.python.org fields:

activity = <Date 2017-07-30.23:07:56.388>
actor = 'terry.reedy'
assignee = 'terry.reedy'
closed = True
closed_date = <Date 2017-07-30.23:07:56.389>
closer = 'terry.reedy'
components = ['IDLE']
creation = <Date 2017-07-26.19:18:52.478>
creator = 'terry.reedy'
dependencies = ['31004']
files = []
hgrepos = []
issue_num = 31050
keywords = []
message_count = 9.0
messages = ['299259', '299492', '299518', '299519', '299521', '299523', '299528', '299529', '299530']
nosy_count = 2.0
nosy_names = ['terry.reedy', 'cheryl.sabella']
pr_nums = ['2952', '2955']
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'enhancement'
url = 'https://bugs.python.org/issue31050'
versions = ['Python 3.6', 'Python 3.7']

[terryjreedy]

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.

[terryjreedy]

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:

• copy general block after FontPage;
• add 'class GenPage(Frame):' and def __init__ modeled on FontPage.__init__, but no highlight parameter;
• replace 'frame = dialog.tabpages...' at top of create_page_general with 'frame = self';
• comment out old code;
• in create_widgets change 'self.create_page_general' to 'GenPage(note)'.

For test_configdialog:

• change 'GeneralTest' to 'GenPageTest
• change setUpClass similarly as in FontPageTest;
• change test functions similarly as in FontPageTest and otherwise as needed to keep tests passing.

[csabella]

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. :-)

[terryjreedy]

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.
# def create_widgets():

• Pick names for the pages. They are both tab labels and page ids.
• Send names to tabbed_pages.
• Call create_page_name functions.
  # def load_configs():
• Call each load_name_config.
  # def create_page_name(): for each name.
• Extract frame from tabbed_pages using name.
• Define tk vars.
• Create subframes and widgets
• Pack widgets
  # def var_changed_var_name(), for each tk var
• Perhaps send change to changes dict.
  # def other_method():, for each name, semi-sorted
  # def load_name_config() for each name.
  # Tack on extensions mostly ignoring the above.

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.

[terryjreedy]

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.

[terryjreedy]

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.

[terryjreedy]

New changeset e8eb17b by Terry Jan Reedy (csabella) in branch 'master':
bpo-31050: IDLE: Factor GenPage class from ConfigDialog (bpo-2952)
e8eb17b

[terryjreedy]

New changeset 8c4e5be by Terry Jan Reedy in branch '3.6':
[3.6] bpo-31050: IDLE: Factor GenPage class from ConfigDialog (GH-2952) (bpo-2955)
8c4e5be

[terryjreedy]

Much easier the 2nd time ;-)