Community
Participate
Working Groups
Build ID: I20070102 Steps To Reproduce: When passing null as the BindSpec before the API change and renames, you used to get a PartialValidator for simple types (Double, Integer etc) for free. Now you don't. Instead you get a validator that returns STATUS.OK no matter what. You can see this in DefaultBindSpec, line 382. As this is (IMO) by far the commonest case when using forms to edit objects (ie. don't allow user to enter invalid value), I would like that back, please :) It makes no sense to allow text in a text box that binds to a number, and the user won't find out until they press OK. Since the plumbing for assigning validators already exists, and works, I wonder why this functionality was removed. More information:
This is how I bind my stuff: dbc.bindValue(SWTObservables.observeText(bodyWeight, SWT.Modify),BeansObservables.observeValue(measurement, "bodyWeight"), null);
I would also like to propose, if this is going to be fixed, to take into account the issues about i18n and l10n. In the previous implementation, regexps where used, as far as I remember. This provided a basic functionality, but it was too simple: my users are Greek (so am I) and we use different delimiters for the thousands and decimals: 10.000.00,32 instead of 10,000,00.32 I just told them to use the English way, and they didn't mind (much). But I always felt that this should be fixed.
I removed the partial validator implementations exactly for that reason (l10n). I'm from Germany where numbers are written the Greek (or Arab?) way too. Brad - the Eclipse way would be to use ICU4J's DecimalFormat, are you OK with adding a dependency on ICU4J for this? See http://wiki.eclipse.org/index.php/ICU4J.
I'm not Brad, but I'm a committer so I'll +1 on this. :)
Since DecimalFormat is a part of the replacement plug-in I'm in. + 1
Sorry I probably wasn't fully awake when I looked at this. The problem is how do we get something like a regexp, or even a list of valid characters for a data type like float in a way that works with i10n? Does anybody know more about DecimalFormat.toLocalizedPattern() and if it could be used for this purpose?
I don't know about DecimalFormat, but the naive way for doing this would be trying to use Float.parseFloat(string) that AFAIK uses the current Locale to do it's thing, and catch any NumberFormatExceptions that arise...
(In reply to comment #7) > I don't know about DecimalFormat, but the naive way for doing this would be > trying to use Float.parseFloat(string) that AFAIK uses the current Locale to do > it's thing, and catch any NumberFormatExceptions that arise... This does not work because there are substrings of valid floats that don't successfully parse, e.g. "-", or ".".
I said it was naive, didn't I ? :) I'll try to come up with something better, as this is a major no go for me...
(In reply to comment #6) > Sorry I probably wasn't fully awake when I looked at this. The problem is how > do we get something like a regexp, or even a list of valid characters for a > data type like float in a way that works with i10n? Does anybody know more > about DecimalFormat.toLocalizedPattern() and if it could be used for this > purpose? > Are you saying that you don't think the format symbols defined for DecimalFormat will support our needs?
I'll take a look into this. I really want to see this done for 1.0.
We have a few issues here... 1. We're not localizing the display of integers, floats, or doubles by default. 2. We're not partiallly validating any of the above types by default. #1 should be able to use the com.ibm.icu.text.DecimalFormat for formatting and parsing of the value in the default converter. We would assume the current locale by default and if any customization is necessary the consumer would have to write their own converter. Also since DecimalFormat is a part of the replacement plugin this wouldn't have much of an impact on consumers. #2 is trickier. We can retrieve the format symbols from DecimalFormatSymbols. It provides API to retrieve the grouping separator, decimal separator, etc. and would allow us to cover the case of "-", ".", etc. But the current design of partial validation is going to make this difficult and costly. The value received by the partial validator is what the full value will be rather than what was just entered. We'd have to parse the value and if that fails we'd have to check for combinations of any of the format symbols on every key stroke. We can't rely upon java.util.regex for this as it is not part of the CDC 1.0 foundation. We'd either have to roll our own or include a third party library. If the partial validator was passed only what changed we'd be able to partially validate this but this is not the case today. Dave or Boris if you have any input on why this was written this way I'd appreciate knowing. My guess is that this was meant to be an abstraction for masking. My feelings are that since we're losing the context of the change this could become costly. If we were to validate each character of the value in order to do this properly we'd have to add a dependency on the full version of com.ibm.icu.text and disallow the replacement plugin option. The reason being that in JSE 1.4.x Character.isDigit(char) doesn't take into account supplementary characters[1]. This is a part of ICU4J and also Java 5. There might be a way to not require this if building against a 1.5 JDK but I'd have to do some digging. I'm going to look into this more this week but needed to get my thoughts out. [1] http://java.sun.com/developer/technicalArticles/Intl/Supplementary/
I'm working on converting the converters to use ICU4J and that's going fine. But the "partial validation" is making me pretty uneasy. As a result I'm thinking about removing it... or I guess not adding it back. Bear with me... Partial validation was meant as an abstraction for masking Text widgets. I don't know of a use case other than Text and if someone has one I'd like to hear it. But partial validation abstracts so much that it's going to be costly in regards to performance. Most masking implementations that I've written or seen convey the value being added and it's position. This allows for fine grained processing which intelligent masking requires. By providing an abstraction we're forcing the validator to unravel the diff to determine what was entered and where in was entered. So my opinion at this point in time is that we should remove partial validation but provide API to expose what is necessary for a consumer to implement intelligent masking. We would need to provide examples of this as it will be a common request so that with minimal effort the consumer could get up and running with it. Comments? This might be a good topic to discuss at EclipseCon in the presence of beer. ;)
>..but provide API to expose what is necessary for a consumer to implement >intelligent masking. We would need to provide examples of this as it will be a >common request so that with minimal effort the consumer could get up and >running with it. As a consumer I would like to see data binding simplify the chore of validating what goes into the widget from SWT. Intelligent masking would be very nice. At Eclipsecon 2007 BOF we saw a great presentation. I asked about using databinding to filter the allowed keys. My example was a US Zip code. It should be 5 numeric characters. My question was " could non numeric characters be ignored. An example program was modified to uppercase and then only vowels for the name. This was cool and impressive but there were two problems. One was identified immediately by the developers. The "aeiou" was translated to "AEIOU" in the field in the table but not in the widget in which it was being typed. The second problem, to my mind, was that when the first non-vowel was entered the field was invalid. How does the user simply fix it? As the programmer I would like to simple ignore characters which don't fit the rules. If this were typed aeIoYU I want to see in the widget to which I am entering the data. AEIOU The lower case keys uppercased the the "Y" simply dropped. I like the idea of using regular expressions and encourage the team to put them back in. This history shows how regular expressions don't work well for numbers. I suggest that something else supplement, or be used in place of, regular expressions. It seems that text representing numbers is not well suited to be evaluated as regular expressions but they do have some fairly simple rules. digits only except - none or one decimal delimiter dependent on the nl - none, one or many thousands delimited depending on the nl - none or one negative sign at the beginning or end of the string (nl dependent) It seems that this would work for simple data types (int, float, etc) and BigDecimal. Please don't forget BigDecimal (and I suppose the other Bigs). I think that there is a difference between a number and text which contains numbers. I may be wrong but I don't see that there is really much intellegent masking, except for range perhaps, for true numbers. For example, a "telephone number" is not a numeric value. You dont add it or manipulat it as a number. It is simply text which is formatted by rules. Things that are true numbers don't have much formatting and simple validation. Counts, amounts, measurements, etc. The validation that they do have isn't done well by regex. Why not treat them differently? In summary I want to see from databinding (or maybe parly from SWT) - intellegent masking - basic keystroke validation - option to use regular expression for keystroke validation - choice to simply ignore keystrokes which are valid as user enters them - option to use regular expressions to validate field - testing / editing text with is going into a primitive or BigXXXXX with rules appropriate to the numeric data in place of regular expressions. If databinding can't provide all of this, then I would like to see hooks to do it myself and plenty of examples how. Thanks you all very much. Great work.
I have some code locally but it's not worth publishing at this point. I'm adding the "helpwanted" keyword as I won't be getting back to this soon.
Removing blocking of bug 154132 (plan item). Also I'm rewording the summary as partial validation has been removed from the binding pipleine and this request is now for examples of how to perform partial validation. The i18n issues mentioned in comment 2 are being handled by our default converters and validators. If not please open a new bug with details of the issue.
In case this is of use to anyone, I'm currently doing partial validation using the inline code below (requires Data Binding patch comment #7 on Bug 180392 too): /** * Apply partial validation behaviour (disallow invalid characters from being entered) * to a Text control for numeric fields. * @param text Text control * @param type Type being bound (e.g. int.class) */ private static void applyPartialValidation(final Text text, final Class type) { if(type == int.class || type == Integer.class) new PartialValidator(text, intConverter != null ? intConverter : (intConverter = StringToNumberConverter.toInteger(false))); else if(type == long.class || type == Long.class) new PartialValidator(text, longConverter != null ? longConverter : (longConverter = StringToNumberConverter.toLong(false))); else if(type == double.class || type == Double.class) new PartialValidator(text, doubleConverter != null ? doubleConverter : (doubleConverter = StringToNumberConverter.toDouble(false))); else if(type == byte.class || type == Byte.class) new PartialValidator(text, byteConverter != null ? byteConverter : (byteConverter = StringToNumberConverter.toByte(false))); else if(type == short.class || type == Short.class) new PartialValidator(text, shortConverter != null ? shortConverter : (shortConverter = StringToNumberConverter.toShort(false))); else if(type == char.class || type == Character.class) text.setTextLimit(1); } /* Re-used partial validation converters */ private static StringToNumberConverter intConverter = null; private static StringToNumberConverter longConverter = null; private static StringToNumberConverter doubleConverter = null; private static StringToNumberConverter byteConverter = null; private static StringToNumberConverter shortConverter = null; /** * Apply partial validation (disallow invalid characters from being entered) * to a Text control for numeric fields of the given type. * @param text Text or Combo control * @param type Type being bound (e.g. int.class) */ private static class PartialValidator implements VerifyListener { final Text text; final StringToNumberConverter converter; public PartialValidator(final Text text, StringToNumberConverter converter) { this.text = text; this.converter = converter; text.addVerifyListener(this); } public void verifyText(VerifyEvent e) { // Ensure valid number String currentText = text.getText(); String newText = currentText.substring(0, e.start) + e.text + currentText.substring(e.end); if(newText.equals("")) return; // Try parsing try { Object out = converter.convert(newText); if(out == null) { // Disallow bad number e.doit = false; } } catch(Exception ex) { e.doit = false; } } }
I think most of this bug is satisfied by the Nebula FormattedText. Ok to resolve worksforme?
+1
Ok, closing
