| [news.eclipse.tools.emf] Re: [Validation] How to change text of validation messages |
Hi Ed,
-Will
Will,
More comments below.
Will Horn wrote:Yep.Ed,
Comments below as well.
Ed Merks wrote:This makes sense. I still like the idea of having a separate plugin where the new messages are kept (so I can reuse across the different models) so I guess this means overriding EObjectValidator.getEcoreResourceLocator in each generated Validator subclass to point to my separate plugin.Will,
Comments below.
Will Horn wrote:I'm also interested in an enhanced API - were there any proposals.Nope.You could have done it directly in your generated one?
Here is what I did:
* I created a new plugin com.example.model.validation with a dependency on com.example.model (my EMF model)
* I created a class ModelValidator that subclasses the generated one and overrides getEcoreResourceLocator.The ResourceLocator returned looks in the current plugin (plugin.properties) for strings and falls back to EcorePlugin if none are found (i.e. MissingResourceException is thrown).Note that you could modify your generated plugin so that it directly delegates to the Ecore plugin by modifying the constructor to mention the Ecore plugin as a resource locator.* I created a class ModelDiagnostician that subclasses Diagnostician and calls eValidatorRegistry.put(ModelPackage.eINSTANCE, new ModelValidator()) in the constructor to hook in the ModelValidator class.This makes me think you should have modified the base validator.There isn't a general API in the item providers for fetching feature labels, although there are methods in the implementation that do that. So there's nothing we can hook into...That's cool. I am not using the validate action since I have a live validation service and I did not notice this code. One issue though - this doesn't seem to handle the feature labels.* I overrode getFeatureLabel and getObjectLabel in my ModelDiagnostician to use the generated human friendlier names in the com.example.model.edit plugin.That's what the validate action does too, i.e., exploits the label providers when they are available.Yes, and that would kind of suck. But I could imagine something like "The object ''{0}'' as a problem." and then appending the rest of the message to that. What I did for XSD is record the actual key for the message in the XSDDiagnostic, but I don't think it would be good to extend the Diagnosic API for that....The problem, like Eric mentioned is that there are different diagnostics/messages that have the same "data". If there was one code for each, that would help. For example, have separate codes for FeatureHasTooManyValues, FeatureHasTooFewValues, etc instead of them all being EOBJECT__EVERY_MULTIPCITY_CONFORMS. But this changes the API...Isn't open source great. :-P
Now if I use ModelDiagnostician to validate EObjects in my model, I can completely customize the messages (thanks to being able to review the source code in EObjectValidator).Yes, this context issue is a bit tricky. When you have a message showing directly for the applicable object it's kind of pointless to be explicit about the context.
There are a few issues I'm still dealing with, however. First, I would like to have different messages for different situations. For example, a Problems view that lists all of the validation issues probably needs more description than a control decorator on a dialog box. In the first case, I might want to say "The 'Author' attribute is missing on Book ABC." whereas in the second case when Book ABC is being edited in a dialog box "'Author' is required." is sufficient.Are you sure that the objects in the actual diagnostic wouldn't allow you to compose contextual information (about eObject) separate from the rest of the message?
To do this, I think the best solution is to have more information on the Diagnostic so the message can be formatted however the context would like. I could create another plugin (or maybe have two properties files), but it seems I would have to use two separate validation runs on the same object.
| diagnostics.add
(*new *BasicDiagnostic
(Diagnostic.ERROR,
DIAGNOSTIC_SOURCE,
EOBJECT__EVERY_MAP_ENTRY_UNIQUE,
getEcoreResourceLocator().getString
("_UI_DuplicateMapEntry_diagnostic",
*new *Object []
{
getFeatureLabel(eReference, context),
i,
index
}),
*new *Object [] { eObject, eReference, entry, eMap.get(index) }));|
That's a good idea when you have full control within your application how it will be viewed. You might even produce a result that still has a substitution in it so it needs to have substitution applied twice to give the final result...
My latest idea is to encode both the full message and the context sensitive one on the Diagnostic (separated by some well defined character sequence like ---). Then the UI can choose which to present to the user.Right now we do getEcoreResourceLocator().getString(...) but if we did getString(...) and had a protected getString(...) method that does getEcoreResourceLocator().getString(...) you'd have a little more flexibility to redirect the lookups. And if I did that, I could perhaps even pass in "data" that will be part of the final diagnostic, so you could pick out the eObject and include it in the message as part of an override... Or perhaps better yet, instead of calling new BasicDiagnostic, I call a helper method that is passed in the severity, the source, the code, the key for the message, the substitutions for the message, and the data for the diagnostic. Then you'd have all the information you could possibly need... It certainly has some appeal...I did not completely follow these suggestions. But by using a separate plugin for the messages and updating the generated validators, I can get the new messages simply using the basic Diagnostician. The only remaining issue is how to get the custom object and feature labels in a generic way. Following the ValidateAction pattern, I could use the item providers to get the object label, but that still leaves the feature label. Maybe I will make an extension point in the validation plugin that each generated edit plugin can use to register itself as the "label provider" for its package.Perhaps this could be done external to the diagnostics themselves? A given object might have hundreds of errors so perhaps grouping by context would allow the contextual message to avoid duplication.
The second issue is that I would prefer something more generic that I could use on multiple models/packages at once. In other words to be able to pass any EObject into something like Diagnostician.validate and to get the results. I think this would require a way to change/customize the ResourceLocator without subclassing.
I suppose another approach would be to delegate through another utility method that always passes eObject, which in turns fetches the resource locator and composes the message without that eObject, i.e., a bit of refactoring.I wonder if the above proposal has value. What do you think? (When we generate the validator, we could encourage this pattern by generating the helper method and the TODO bodies in this more flexible form.)Your comments definitely helped me clean up the design. There are some things that are still not ideal, but I don't have any brilliant ideas for refactoring at this point. Hopefully at least someone down the road will read this thread and benefit like I have.Maybe some of what I said helped. If a bit of refactoring would be better, that seems reasonable too. (We just don't want to break any existing APIs is the only rule to constrain us.)
Any advice is greatly appreciated!
-Will
Ed Merks wrote:Eric,
Yes, all good ideas will be taken into consideration. We often refactor things to make overrides easier once we understand the desired usage patterns. I hadn't really thought so much about replacing messages, only about controlling the substitutions in them...
Eric Rizzo wrote:Ed Merks wrote:Eric,
I guess you can't. It's really all just one constraint with different messages to try to be more helpful to pin point in what why the multiplicity constraint is violated.
It is a bit of a hack, but I found that I can take the ESructuralFeature (from getData()) and ask it isMany() - if true I use one message, if false I assume the Diagnostic is about a missing required attribute and use a different message for that.
If I entered a feature request to provide API in EObjectValidator to more easily override the message texts, is that something that would at least be considered? I've got an idea for the actual API.
Eric
Eric Rizzo wrote:Eric Rizzo wrote:Ed Merks wrote:Specializing messages is a little like wanting to alter the messages that JDT produce. Also keep in mind that the Diagnostic itself has enough details that you could use those to produce your own message external to the validator code. I.e., you have access to the source, the constraint code within the source, and all objects involved in the problem.
Shortly after my last post, I discovered that. I am customizing MarkerHelper to modify the Marker messages instead of messing around with the Diagnostic messages.
I spoke too soon - I'm not so sure in MarkerHelper.composeMessage() that I have enough information in all cases. The example I'm facing now is how to tell the difference between a feature has too many values, a feature that has not enough values, or a required feature has no value. All of those scenarios seem to produce an identical Diagnostic, so how can I distinguish them?
Eric