classification
Title: IDLE, configdialog: Factor out GenTab class from ConfigDialog
Type: enhancement Stage: resolved
Components: IDLE Versions: Python 3.7, Python 3.6
process
Status: closed Resolution: fixed
Dependencies: 31004 Superseder:
Assigned To: terry.reedy Nosy List: cheryl.sabella, terry.reedy
Priority: normal Keywords:

Created on 2017-07-26 19:18 by terry.reedy, last changed 2017-07-30 23:07 by terry.reedy. This issue is now closed.

Pull Requests
URL Status Linked Edit
PR 2952 merged cheryl.sabella, 2017-07-30 20:14
PR 2955 merged terry.reedy, 2017-07-30 22:41
Messages (9)
msg299259 - (view) Author: Terry J. Reedy (terry.reedy) * (Python committer) Date: 2017-07-26 19:18
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.
msg299492 - (view) Author: Terry J. Reedy (terry.reedy) * (Python committer) Date: 2017-07-30 05:53
Now that we know what we are doing, we can simplify the steps.  These assume that #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.
msg299518 - (view) Author: Cheryl Sabella (cheryl.sabella) * (Python committer) Date: 2017-07-30 20:12
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.  :-)
msg299519 - (view) Author: Terry J. Reedy (terry.reedy) * (Python committer) Date: 2017-07-30 20:29
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.
msg299521 - (view) Author: Terry J. Reedy (terry.reedy) * (Python committer) Date: 2017-07-30 21:06
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.
msg299523 - (view) Author: Terry J. Reedy (terry.reedy) * (Python committer) Date: 2017-07-30 21:35
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.
msg299528 - (view) Author: Terry J. Reedy (terry.reedy) * (Python committer) Date: 2017-07-30 22:39
New changeset e8eb17b2c11dcd1550a94b175e557c234a804482 by Terry Jan Reedy (csabella) in branch 'master':
bpo-31050: IDLE: Factor GenPage class from ConfigDialog (#2952)
https://github.com/python/cpython/commit/e8eb17b2c11dcd1550a94b175e557c234a804482
msg299529 - (view) Author: Terry J. Reedy (terry.reedy) * (Python committer) Date: 2017-07-30 23:02
New changeset 8c4e5be1dfb4d1c74e8cc7ac925e3196818e07ac by Terry Jan Reedy in branch '3.6':
[3.6] bpo-31050: IDLE: Factor GenPage class from ConfigDialog (GH-2952) (#2955)
https://github.com/python/cpython/commit/8c4e5be1dfb4d1c74e8cc7ac925e3196818e07ac
msg299530 - (view) Author: Terry J. Reedy (terry.reedy) * (Python committer) Date: 2017-07-30 23:07
Much easier the 2nd time ;-)
History
Date User Action Args
2017-07-30 23:07:56terry.reedysetstatus: open -> closed
resolution: fixed
messages: + msg299530

stage: needs patch -> resolved
2017-07-30 23:02:53terry.reedysetmessages: + msg299529
2017-07-30 22:41:31terry.reedysetpull_requests: + pull_request3003
2017-07-30 22:39:20terry.reedysetmessages: + msg299528
2017-07-30 21:35:16terry.reedysetmessages: + msg299523
2017-07-30 21:06:51terry.reedysetmessages: + msg299521
2017-07-30 20:29:18terry.reedysetmessages: + msg299519
2017-07-30 20:14:31cheryl.sabellasetpull_requests: + pull_request3000
2017-07-30 20:12:09cheryl.sabellasetmessages: + msg299518
2017-07-30 05:55:02terry.reedysetdependencies: + IDLE, configdialog: Factor out FontTab class from ConfigDialog
2017-07-30 05:53:57terry.reedysetmessages: + msg299492
2017-07-26 19:19:10terry.reedysetassignee: terry.reedy
stage: needs patch
type: enhancement
components: + IDLE
versions: + Python 3.6, Python 3.7
2017-07-26 19:18:52terry.reedycreate