Skip to content

Commit

Permalink
WIP: more complete checking of names in code; re psychopy#1132
Browse files Browse the repository at this point in the history
Unresolved:
- warning msg not printed on first param page (basic params), but does show for advanced params (e.g., text - flip)
  and does properly disable OK button for for error on basic params pages
  ?? somewhere `parent.nameOKlabel.SetLabel(message)` is getting lost / obscured / pre-empted??
- only attempted for a few specific fields:        elif fieldName in ('text', 'color', 'image', 'flip', 'opacity')
- need to think through more whether names from compiled code is always appropriate
  (and whether to always flag all namespace collisions)
  • Loading branch information
jeremygray committed Mar 8, 2016
1 parent b919f33 commit cfdfd73
Show file tree
Hide file tree
Showing 2 changed files with 66 additions and 12 deletions.
18 changes: 12 additions & 6 deletions psychopy/app/builder/dialogs/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -235,9 +235,12 @@ def __init__(self, dlg, label, param, parent, fieldName,
if len(param.allowedVals) == 1 or param.readOnly:
self.valueCtrl.Disable() # visible but can't be changed

# add a NameValidator to name valueCtrl
# add a Validator to the valueCtrl
if fieldName == "name":
self.valueCtrl.SetValidator(validators.NameValidator())
elif fieldName in ('text', 'color', 'image', 'flip', 'opacity'):
# ... and anything that is valType code, or can be with $ ...
self.valueCtrl.SetValidator(validators.CodeValidator())

# create the type control
if len(param.allowedTypes):
Expand Down Expand Up @@ -682,8 +685,11 @@ def addParam(self, fieldName, parent, sizer, currRow, advanced=False,
advanced=advanced, appPrefs=self.app.prefs)
self.paramCtrls[fieldName] = ctrls
if fieldName == 'name':
ctrls.valueCtrl.Bind(wx.EVT_TEXT, self.checkName)
ctrls.valueCtrl.Bind(wx.EVT_TEXT, self.doValidate)
ctrls.valueCtrl.SetFocus()
elif fieldName in ('text', 'color', 'image', 'flip', 'opacity'):
ctrls.valueCtrl.Bind(wx.EVT_TEXT, self.doValidate)

# self.valueCtrl = self.typeCtrl = self.updateCtrl
_flag = wx.ALIGN_RIGHT | wx.ALIGN_CENTRE_VERTICAL | wx.LEFT | wx.RIGHT
sizer.Add(ctrls.nameCtrl, (currRow, 0), border=5, flag=_flag)
Expand Down Expand Up @@ -775,7 +781,7 @@ def show(self):
self.OKbtn.Bind(wx.EVT_BUTTON, self.onOK)
self.OKbtn.SetDefault()

self.checkName() # disables OKbtn if bad name
self.doValidate() # disables OKbtn if bad name
buttons.Add(self.OKbtn, 0, wx.ALL, border=3)
CANCEL = wx.Button(self, wx.ID_CANCEL, _translate(" Cancel "))
buttons.Add(CANCEL, 0, wx.ALL, border=3)
Expand Down Expand Up @@ -1019,8 +1025,8 @@ def _checkName(self, event=None, name=None):
else:
return "", True

def checkName(self, event=None):
"""Issue a form validation on name change.
def doValidate(self, event=None):
"""Issue a form validation on event, e.g., name or text change.
"""
self.Validate()

Expand Down Expand Up @@ -1168,7 +1174,7 @@ def makeGlobalCtrls(self):
flag=wx.EXPAND | wx.ALIGN_CENTRE_VERTICAL | wx.ALL)
row += 1

self.globalCtrls['name'].valueCtrl.Bind(wx.EVT_TEXT, self.checkName)
self.globalCtrls['name'].valueCtrl.Bind(wx.EVT_TEXT, self.doValidate)
self.Bind(wx.EVT_CHOICE, self.onTypeChanged,
self.globalCtrls['loopType'].valueCtrl)
return panel
Expand Down
60 changes: 54 additions & 6 deletions psychopy/app/builder/validators.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,23 +3,24 @@
'''
import wx
from ..localization import _translate
from . import experiment


class NameValidator(wx.PyValidator):
class BaseValidator(wx.PyValidator):
"""
Component name validator for _BaseParamsDlg class. It depends on accesss
Component name validator for _BaseParamsDlg class. It depends on access
to an experiment namespace. Validation checks if it is a valid Python
identifier and if it does not clash with existing names.
@see: _BaseParamsDlg
"""

def __init__(self):
super(NameValidator, self).__init__()
super(BaseValidator, self).__init__()
self.message = ""

def Clone(self):
return NameValidator()
return self.__class__()

def Validate(self, parent):
"""
Expand All @@ -31,7 +32,7 @@ def Validate(self, parent):
parent = parent.GetParent()
except Exception:
print("Couldn't find the root dialog for this event")
message, valid = self.checkName(parent)
message, valid = self.check(parent)
parent.nameOKlabel.SetLabel(message)
return valid

Expand All @@ -41,7 +42,15 @@ def TransferFromWindow(self):
def TransferToWindow(self):
return True

def checkName(self, parent):
def check(self, parent):
raise NotImplementedError


class NameValidator(BaseValidator):
def __init__(self):
super(NameValidator, self).__init__()

def check(self, parent):
"""checks namespace, return error-msg (str), enable (bool)
"""
control = self.GetWindow()
Expand All @@ -65,3 +74,42 @@ def checkName(self, parent):
return msg, True
else:
return "", True


class CodeValidator(BaseValidator):
"""
Component code validator for _BaseParamsDlg class. It depends on access
to an experiment namespace. Validation checks if it is a valid Python
code, and if so, whether it contains identifiers that clash with
existing names (in the to-be-generated python script).
@see: _BaseParamsDlg
"""

def __init__(self):
super(CodeValidator, self).__init__()

def check(self, parent):
"""checks intersection of names in code and namespace
"""
control = self.GetWindow()
if not hasattr(control, 'GetValue'):
return '', True
val = control.GetValue() # same as parent.params[self.fieldName].val
codeWanted = experiment._unescapedDollarSign_re.search(val)
if codeWanted: # ... or valType == 'code' -- require fieldName?
# get var names from val, check against namespace:
code = experiment.getCodeFromParamStr(val)
try:
names = compile(code, '', 'eval').co_names
except SyntaxError:
pass
else:
namespace = parent.frame.exp.namespace
for name in names:
# params['name'] not in namespace yet if its a new param
if (name == parent.params['name'].val or
namespace.exists(name)):
msg = _translate('Name `{}` is already used')
return msg.format(name), False
return '', True

0 comments on commit cfdfd73

Please sign in to comment.