classification
Title: IDLE configdialog: reduce multiple references to Var names
Type: enhancement Stage: test needed
Components: IDLE Versions: Python 3.6
process
Status: open Resolution:
Dependencies: 22115 Superseder:
Assigned To: terry.reedy Nosy List: csabella, terry.reedy
Priority: normal Keywords:

Created on 2016-06-26 02:23 by terry.reedy, last changed 2017-06-22 00:04 by terry.reedy.

Messages (3)
msg269271 - (view) Author: Terry J. Reedy (terry.reedy) * (Python committer) Date: 2016-06-26 02:23
ConfigDialog uses nearl 20 tkinter Vars, and I expect to add more.  The name of each appears in at least 6 places.

In one of 'create tab frame' methods:
1. create the Var
2. use the Var with at least 1 tk widget
In AttachVarCallbacks method:
3, 4. attach trace with callback with derived name
  self.varname.trace_add('write', self.VarChanged_varname)
In remove_var_callbacks method (with delete call already factored out):
5. include Var in for loop tuple, to remove callback with
  "var.trace_remove('write', var.trace_info()[0][1])"
In callback definition:
6. def VarChanged_varname

The above uses the new trace method names that will be introduced in #22115, which I expect will be applied first.  (I might also replace
'VarChanged_' with 'var_changed_' or something shorter, but this is not relevant here.)

I propose to consolidate 1, 3, & 4 by replacing AttachVarCallbacks with

def add_traced_var(varname):
    """Create var; bind to varname, append to list, and trace with callback.

    varname must match use in caller and callback def.
    """
    var = tk.StringVar(self.root)
    setattr(self, varname, var)
    self.vars.append(var)
    cbsuffix = 'font' if varname.startswith('font') else varname
    var.trace_add('write', self.getattr('VarChanged_' + cbsuffix))

The cbsuffix local takes care of the complication that the 3 'fontWxyz' vars need the same callback.  In any Var are not StringVars, add vartype parameter.

Each tab frame creation method would call add_traced_var within a varname loop.  The current varname tuple for 5. would be replaced with self.vars, which should then be cleared.  This will leave 3 name occurrences that must match instead of 6.

---
I believe 1-6 is complete.  Varnames are translated to config item names within the callbacks, so do not have to match.  For instance,

def VarChanged_spaceNum(self, *params):
    value = self.spaceNum.get()
    self.AddChangedItem('main', 'Indent', 'num-spaces', value)

translated 'spaceNum' to 'num-spaces'.  On the other hand, I an not sure why the difference.  For back compatibility, config names are fixed.  The varnames, like method names, are internal to configdialog and can lowercased (PEP8) and otherwised changed.
---

For testing, I will embed the add and delete methods in a dummy class with a couple of dummy callbacks.  Then add, introspect, delete, and introspect again.
msg296497 - (view) Author: Cheryl Sabella (csabella) * Date: 2017-06-20 23:01
Created a pull request for this - "The varnames, like method names, are internal to configdialog and can lowercased (PEP8) and otherwised changed."
msg296608 - (view) Author: Terry J. Reedy (terry.reedy) * (Python committer) Date: 2017-06-22 00:04
I moved PR 2307 to a new issue, #30728 Modernize configdialog. My comment that internal Var names "can lowercased (PEP8) and otherwised changed" was anticipating such an issue.  The point for this issue was to contrast Var names with the cross-version config names that we must not change.

I don't expect anyone else to fully understand this without a couple of hours of code study.

Once #22115 was merged, I could have gone ahead.  Since I did not, I should now wait for at least part of PR 2307 and #30728 since the patch for this would break parts of the existing new patch.  I could work on a new test though.
History
Date User Action Args
2017-06-22 00:04:39terry.reedysetpull_requests: - pull_request2354
2017-06-22 00:04:25terry.reedysetdependencies: + Add new methods to trace Tkinter variables
messages: + msg296608
2017-06-20 23:01:23csabellasetnosy: + csabella
messages: + msg296497
2017-06-20 22:59:50csabellasetpull_requests: + pull_request2354
2017-06-19 18:45:04terry.reedysetcomponents: + IDLE
2016-06-26 02:23:35terry.reedycreate