Jeff,
First, let me say I'm flattered re: the comments at the top of your
code; I wish I had had the time last year to write *this* control, as
it would have made the job of writing wxTimeCtrl a lot easier if I had.
But at the time, I was a novice at writing derived controls, and so I
stuck to the specific problem I needed a solution for.
2nd, I have been meaning to write to you ever since you first posted
re: this project, and I apologize for not getting to in sooner.
I would love to help you with this, and to that end...
With your indulgence, I have a few comments about your v0.0.5 code;
this is long and fairly detailed, so bear with me. (It's taken me a
while to process it all, so I hope these comments are still relevant;
I've tried to eliminate feedback that others have already given.)
## Version 0.0.5 Current Issues:
## ===================================
···
##
## 2. Cut/Copy/Paste not yet addressed.
IMO, "cut" should not be allowed in a fixed-position masked control;
However, I'd make the meaning of "cut" be "blank the current
selection, minus the mask, of course... Copy is just fine; I'd leave
that action to be the default for wxTextCtrl.
However, Paste is a bit tricky...
For wxIntCtrl, I validate the paste buffer contents to make sure they're
legal digits, and then construct the candidate new value from the paste,
and then validate; if it ain't legal, it doesn't validate and the bell
sounds... Perhaps you can do something along these lines in a more
generic fashion?
## 3. How can I detect shift-key down when home or end are pressed (to
select?)
Use event.ShiftDown(). (See the wxKeyEvents demo for how.)
Q: Is it your intention to make "home" and "end" mean "home/end" of the
current field, or just the whole control? (either would be good, as
right now, you can't select and then hit backspace or delete...)
Actually, I don't see why you can't just use event.Skip() for these keys,
that is:
elif key == WXK_HOME:
_dbg('key == WXK_HOME')
event.Skip()
charOk = false
elif key == WXK_END:
_dbg('key == WXK_END')
event.Skip()
charOk = false
I tried this, and it worked well; in fact using shift-end, followed by
delete brings the cursor to the start of the deleted value with the mask
intact -- very nice!
## 4. Mask characters [A,a,X,#] cannot appear in the format string as
literals.
I haven't yet determined how difficult this would be, but it seems to
me that the logical means of doing this is to have '\' be another
special syntax character meaning "escape", as it does in regexps;
this would be consistent with "conventional" syntax, and would allow
\A \a \X \# to mean a "literal" (and therefore "mask") character.
(Of course, you have to allow \\ to mean "literal \.")
## 5. It should allow some way to specify repeating masks, e.g.
(#3)-(#3)-(#4) - this
## could be parsed by re and the standard format string could be built
(###-###-####).
## Therefore no additional logic would be needed, just the syntax and the
re parser.
I have some trouble with this idea; how to you distinguish '(#3)'
from '(#3)'?
That is, this creates ambiguous syntax, where one is a shortcut
for ### and the other means '(', a digit, followed by the mask
characters '3)'. However, if you implement my suggestion for 4 above,
then literal numbers could be more "special syntax;" ie. you could
then write: #3-#12-#4 to mean ###-############-####, and
(#\3) to mean (#3).
Alternatively, you could use the same sort of syntax as for regexps,
ie #{3} means ###, and then to use '{' in the mask, you would use
'\{'. This is better, I think, as you could then extend the notion
to include + and * as special mask specification characters too, so
you could specify 'X+' as "1 or more letters, punctuation or digits."
(This would come in handy for dealing with a bug described below...)
## 6. Numeric handler needs re-writing to include all current possible
options, and to
## avoid cases where the user can 'messup' the entered number.
I spent a great deal of time getting this right in wxIntCtrl, and
I didn't tackle floating point at all. (But feel free to lift
whatever you want from there!) However, wxIntCtrl seems to me to
be a bit different than a "masked edit" control, in that it's
supposed to allow arbitrary numbers, without any "masked" parts or
fixed lengths of digits. Does your control allow this flexibility?
Without yet reading the code, I'm curious about what you consider
'messing up' the entered number?
## Question: can we blank the current number if the user starts to type
over
## the current number?
## To-Do's:
## =============================
## 3. New numeric values handler to improve overall functionality in
numeric fields.
##
## 4. Position cursor at next empty -entry- location on mouse click, or
beginning if
## the control is a) empty or b) full
##
## 5. Validate from list, or from file with list of valid values. Popup
list simulates
## combobox. Example: City validates against list of cities, or zip vs
zip code list.
Perhaps 5 should be done with a control derived from wxComboBox, ie.
wxMaskedComboBox? If you write your the bulk of your code for
controlling and validating the text entry as a mixin (I think their
text entry event mechanisms are almost identical), then you could
create both a wxMaskedComboBox and a wxMaskedTextCtrl without
duplicating most of your code... Then wxMaskedTextCtrl and
wxMaskedComboBox would just be something of the form:
class wxMasked<ctrl-type>(wx<ctrl-type>, wxMaskedEditMixin):
def __init__( self, parent, id=-1, value = '',
pos = wxDefaultPosition,
size = wxDefaultSize,
mask = 'XXXXXXXXXXXXX',
formatcodes = '',
fillChar = " ",
excludeChars = '',
includeChars = '',
revalid = '',
autoformat = '',
style = wxTE_PROCESS_TAB,
name = 'formattedtextctrl'):
wx<ctrl-type>.__init__(self, parent, id, value='',
pos=pos, size= self.__calcSize(size),
style=style, name=name)
self.SetupMaskedEdit(mask, formatcodes, fillchar,
excludeChars, includeChars,
revalid, autoformat)
where <ctrl-type> is either TextCtrl or ComboBox, and 90% of the rest of
the code lives in the mixin. Then, for wxMaskedComboBox, as a last step
to the validation, you could check the resulting value against the values
in the dropdown. (Just a thought.)
====================================================================
Functional comments:
1) IMO, a masked edit control should have its mask fixed in space on the
control. I found the fact that the mask characters move around because
of the proportionally spaced font to be very disconcerting; at first I
thought it was inserting characters, rather than replacing spaces!
(I never had to deal with this in wxTimeCtrl, because all of the
characters were always filled in, but in hindsight, I would add
something along the lines of:
font = self.GetFont()
fixedfont = wxFont(font.GetPointSize(), font.GetFamily(),
font.GetStyle(), font.GetWeight(),
font.GetUnderlined(), 'Courier New')
self.SetFont(fixedfont)
to the constructor, to force a fixed width font for the control, thus
eliminating all the distracting useless "motion" of the mask.
(I actually plan to do this in a patch to wxTimeCtrl as well.)
2) The delete key should work, but it doesn't (also commented on on the
mailing list.) Similarly, home/end selection doesn't work (see fix
under "current issues" (3) above and modified fix under (6) below.)
3) The "signed" integer demo allows you to type a minus, but once entered,
you can't erase it. Similarly, you can select the "space"
at the beginning reserved for a '-', but once entered, you can't select
it anymore. (This seems like a bug to me; the '-' seems to be treated
as a mask character once typed, but I don't think it should be.)
4) What you can type is often seemingly inconsistently validated; for
instance,
you can't type anything but A or P to change AM to PM, but the format
mask for the date and time will happily allow 13/13/2003 35:99 PM...
It would be nice if you could optionally require input to be valid to
be entered. (See suggestion at end.)
5) There are several problems with your email regexp:
a) you're using [] when you should be using (). As a result, your
current
control allows "wsadkin@nn" and doesn't allow "wsadkin@aaa.com".
b) It doesn't account for the .edu, .mil, .gov domains, or the country
codes.
c) It doesn't account for dotted usernames like "William.Sadkin@abc.com"
d) It doesnt account for multipart domains, like support.microsoft.com
e) I don't think email address components can start with numbers, so I
think the regexp should be something like:
[a-zA-Z](\.\w+|\w*)@\w+.([a-zA-Z]\w*\.)*(com|org|net|edu|mil|gov|(co\.)?\w\w
) *$
rather than
\w+@\w+[\.\w+]*[com|org|net] *$
(I don't guarantee this is either necessary or sufficient, but it does
come closer.
Actually, I think there's probably an RFC or something that describes
the particulars on valid email addresses; I'll look into that later
and see if I can come up with something that is both.)
6) There's also a problem with using
XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX
as the mask for email addresses; if you hit "end", you end up at the
far end of the "value", nowhere near the end of what you typed, with no
cursor visible. (at least under MSW.)
It seems to me that there ought to be a format character modifier
for "arbitrary length" (see suggestion for "current issue (5) above.)
The control could then know that the text control could "expand" in size
in those portions of the control, and insert rather than replace
characters. The mask could then be "X*" allowing the input to grow as
necessary while still showing when there's a match to a legal value.
If that's too hard, or even if that's done, you still might want to
consider the following change:
elif key == WXK_END:
_dbg('key == WXK_END')
end = len(self.GetValue().rstrip())
if event.ShiftDown():
_dbg("shift-end; select to end of non-whitespace")
# for some reason that escapes me, the following is
necessary to get the
# selection to work:
wxCallAfter(self.SetInsertionPoint, pos)
wxCallAfter(self.SetSelection, pos, end)
else:
pos = end
self.SetInsertionPoint(pos)
charOk = false
However, even using this, I find that what I really want is to go
to the end of what *I've* typed, not the end of the mask. To do this
would involve some sort of more sophisicated accounting...
What I'd *really* like is to have End => end of the current field's
user input, and Ctrl-End => end of all user input. (and shift-end select
to the end of the current field's input, and ctrl-shift-end select to
the end of the control's input.)
7) Your program doesn't exit on "close" if the 2nd window is brought
up, but I can't figure out why not yet...
====================================================================
Code comments:
1) As a matter of style, your code should have more comments;
specifically, every function should have a triple-quoted doc
string that indicates what it does, what it returns, and
what (if any) subtleties it has. This is consistent with the
coding guidelines, and makes it easier to remember a year from
now what exactly you were thinking... Eg:
def isDecimal( fmtstring ):
"""
This function returns true if the the format string allows
of the form of a POSITIVE floating point number.
"""
(Was this what you intended?)
2) string.ascii_{upper,lower}case are new as of python2.2; this will
make your control unusable for anyone using an earlier versions of
python. I recommend instead using string.letters[26:] and
string.letters[:26] for backward compatibility.
3) You should create 2 files before submission, one for the library,
eg. "maskedEdit.py" and one for the wxPython demo. (eg. timectrl.py
for the lib, and wxTimeCtrl.py for the demo.) If you go with the mixin
idea, then I'd create 3, maskedEditMixin.py maskededit.py and
wxMaskedEdit.py. For the demo, I suggest a wxNotebook with your
various demos on the tabs of the notebook; then you can extend the
demo as much as you want by adding tabs as you run out of room...
4) IMO, there should be an overview in the overview string for the
wxPython demo that explains the interface to your control, particularly
the tricky bits. (Eg. the format codes seem a little obscure...)
5) Names like __get_allowedChars should really be __getAllowedChars.
6) There are some bugs in your use of the _dbg() function; you
return from the OnChar function in 2 different places without 1st
calling _dbg(indent=0), resulting in ever increasing indents;
unfortunately the _dbg() function I wrote doesn't know when you
change scope, and so you have to manage the indentation
level before every exit from a function if you're using indenting.
7) Why do WXK_UP and WXK_DOWN result in a return rather than
just "charOk = false" (and keep going) like all the other such keys?
A bug?
8) By convention, 8-space tabs are rarely used; most python I've seen
uses 4 space indenting and no tabs. (So to be consistent
with the rest of the wxPython source, I'd change to this prior to
submission.) Also, Robin prefers no trailing whitespace on any line;
I wrote a little python filter I use to ensure this is the case on
any code I submit, and I've included it with this message for you.
========================================================
Suggested functionality:
I wrote wxTimeCtrl and, recently, wxIntCtrl to be controls that take and
return different types than strings; because of this, I needed the control
to prevent entry of a value that couldn't be converted to the appropriate
type. So, wxTimeCtrl doesn't allow erasure, because it would result
in an illegal time. Similarly, wxIntCtrl allows erasure so long as the
string can still be converted to an integer, or if it allows None as a
value; otherwise, it "does something reasonable", ie. replaces the value
with 0.
In order to use your control as the base class for such controls,
it would be necessary to disallow certain sequences and control what
happens on erase.
It would also need better accounting/validation of "fields" within the
masked
control would help with these things. For instance, for wxTimeCtrl, I
have the option of either military or 12hr time; if military, then an
hour value of 00 -23 is allowed, but not 30; if 12 hour then the 1st digit
can only be 0 if the 2nd is 1-9, and 1 iff the 2nd digit is 0, 1 or 2.
So, if the value is currently 03, and I move the cursor to before the
0, the control needs to disallow any entry to replace the 0, because
no value would be legal. (This might be achievable simply 'seeding'
the control with a legal value and then using the regexp match to
allow/prevent inputs rather than doing only coloring.)
It would also be nice if the behavior of the up and down arrows,
backspace, delete, cut, etc. were configurable, ie. overridable
without requiring rewrite of the entire OnChar function. Perhaps if
you split these things into subroutines, and effectively made a means
of writing "OnBackspace()" or "OnUpArrow()", then it would be easier to
make derived controls.
=========================================================
Whew!
That took all weekend to write... I haven't looked at all of it
in detail, but I figure I better post this before it reaches novel
length, and is all obsolete!
Regards,
/Will Sadkin
Parlance Corporation