Terry J. Reedy added the comment: The question about reusing ConfigParser (or IdleConfigParser) was a good one. I spent at least an hour looking again at the code and configparser doc.
Observations: 1. Dropping 2.7 support allows us to use mapping protocol access: https://docs.python.org/3/library/configparser.html#mapping-protocol-access 2. 'ConfigParser is a misleading name for what is essentially a dict of dict that may never use its parser. Sections are parts of a page of text. 3. idleConf manages 8 pages, organized in 2 groups (default and user) of 4. Each group of 4 represents a 'set of pages' stored in a directory. There is no class for this concept, but some idleConf methods should be methods of the missing class. 4. The 8 pages can also be seen as 4 pairs. Some idleConf methods, especially those getting values, operate on 1 of the 4 pairs. Maybe there should also be a Pair class. Possibly patching config based on these thoughts is for the future, but I decided that Changes needs a Page class. Each tab works with one page, and should call Page methods. Changes is initialized by ConfigDialog proper and its methods are called by its buttons, not by tabs. I am not a fanatic follower of the 'law of demeter' but the idea of direct access when possible is correct. Next steps: Write class PageTest. Don't overdo it. The code is simple and just needs inputs testing each branch. (I know, I violated write the tests first. Hmmm. Seeing the code simplify so nicely convinced me that the design was good ;-) Write ChangesTest and pull in and adapt the existing code for the Changes methods to make them work. For the moment, I say stick with uploads, perhaps until we are ready to remove old code and integrate this into configdialog.py. At some point we might learn how to collaborate via git branches without making PRs. In the meanwhile, right click, save, git apply are easy enough for me. You asked: Move all IdleConf.userCFg and IdleConf.defaultCfg access to Changes to separate that completely from GUI? I don't think this would work because the data structures are different. It also seems conceptually wrong. Changes embodies the concept 'Pending changes', which constitute a potential transaction. If there are repeated pattern of interaction with idleConf that have not already been captured in idleConf methods, we could factor out another class (Config? Options?) in another issue. Key questions: would this make testing easier? would this significantly improve ConfigDialog code? I will try to look at the extension stuff tomorrow or soon thereafter. ---------- Added file: http://bugs.python.org/file46981/changes_class_2.py _______________________________________ Python tracker <rep...@bugs.python.org> <http://bugs.python.org/issue30779> _______________________________________ _______________________________________________ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
