Regarding option 1:
It's not possible to "extend" an extension point - http://stackoverflow.com/questions/28450349/is-it-possible-to-extend-or-re-use-an-eclipse-extension-point. What we can do to "extend" it is we can copy the exsd files definitions from org.eclipse.core.expressions (definitions.exsd and expressionLanguage.exsd) and modify them to suit our needs (add 'fail' attribute). We'll need however to also modify the code in the o.e.c.expressions plugin that's responsible for parsing and evaluating the expression, so that we provide the right 'fail' message during expression evaluation.

That's a serious bit of work, compared to option 2 - building a library of messages. If I understand you correctly for example for "sampleModel.allNumeric" expression definition in the sampleModel/plugin.xml we shall define a human readable display message, such as "Selection should only contain numeric values"? You mentioned that

the library text may not always be correct for the specific context of the test

Could you explain or give an example? I imagine that ideally with a well structured reusable expression definitions, context won't matter that much.

from limpet_ops.

Hmm, something like "A frequency dataset is required".

Or, whilst the test may be an "AND" for "has location", "has course", "has speed", our user-oriented message for the AND failure could be "Requires a composite track".

from limpet_ops.

Hmm, I've had a play.

Whilst I can change the schema to include documentation elements, and the XML file still gets read in, the content is ignored by Eclipse (which I'm pretty sure is what you said).

This may offer an alternative strategy. What if we also read the XML code into a DOM object. Then, use XPath (or similar) to navigate to the comment/supporting content for a particular entry.

Is this worth further consideration?

from limpet_ops.

I think with this we'll avoid trying to extend the EXSD schema. I had some doubts that the Eclipse Plugin editor might drop elements not contained in the schema when you do save the plugin.xml, but it's not the case - I also tested with custom "fail" documentation elements/attributes, and those are preserved.
The expression evaluation however does not provide information about the expression node that evaluated to false. We would still need to somehow extend/modify the framework to handle that.

Regarding the "Requires a composite track" user message for the AND(hasLocation, hasCourse, hasSpeed) expression. Could this expression have different user message in a different context? If not, we can follow approach 2 for it. This means it will be contained in a separate definition like "requiresCompositeTrack", and then we could have a property file containing the message
requiresCompositeTrack=Requires a composite track

from limpet_ops.

On 28th Feb Dinko said:

I can do some more investigation on this option: "...What if we also read the
XML code into a DOM object. Then, use XPath (or similar) to navigate to the
comment/supporting content for a particular entry..." Reading and navigating the 
XML is easy. The challenge would be to determine which expression node failed
during evaluation. We can do that by wrapping the expression syntax tree in our
wrapper objects that delegate evaluation, but store local results (test failure), so 
that after the evaluation, we can query our nodes and see what failed.

That strategy sounds just like what I was hoping for. Having the user feedback in there with the actual content will make things much easier to maintain & understand.

@dinkoivanov - I'd appreciate you exploring this further.

from limpet_ops.

How will those failure messages be presented to the user? Imagine the following applicability expression:

<and failMessage="Needs multiple composite tracks">
    <and failMessage="Requires a composite track">
        <hasLocation/>
        <hasCourse/>
        <hasSpeed/>
    </and>
    <and failMessage="Requires multiple elements">
        <sizeBiggerThanZero/>
    </and>
</and>

In case one of the inner ANDs fail, the outer will fail as well - what should be the message then? Shall we show only the outermost message? Showing all the fail messages will make the task harder, since we'll need to also convey the expression data somehow - whether it's an AND/OR node, etc.

from limpet_ops.

Ok, we'll imagine that the track is missing a "course" attribute.

Somewhere, we'd declare that failing "hasCourse" is presented as:
"Needs a course field (an angle, in degrees or radians)".

This could be declared as a "failMessage" for , or somewhere centrally.

So the error message for a track that omits both course and speed would be:

Selection invalid, reasons as follows:

• Needs multiple composite tracks failed because of:
• Requires a composite track failed because of:
• Needs a course field (an angle, in degrees or radians)
• Needs a speed field (a speed, in m/sec or knots)

from limpet_ops.

I did some exploratory work in a separate branch here:
https://github.com/debrief/limpet_ops/tree/issue3__document_test_failure_reason
To demonstrate this, launch the Limpet POC View, select some elements and right click - in the IDE console you'll see some logging - not applicable operations would dump some messages (failure messages if specified).
To see how failure messages are specified, open the plugin.xml of the sampleModel and check the XML. I've provided fail messages for some of the conditions.
There are now few Custom* classes in the samplemodel package that carry out loading the fail message from the XML and noting the failure during expression node evaluation. It's not quite clean, since the expression framework was not quite designed to be extensible, but I'd say it's not that bad (no reflection was needed for example).
In essence, we provide a CustomElementHandler that will wrap expression nodes in our CustomExpression (wrapper), which will take care of the fail message during evaluation.

from limpet_ops.

Selection invalid, reasons as follows:

Needs multiple composite tracks failed because of:
Requires a composite track failed because of:
Needs a course field (an angle, in degrees or radians)
Needs a speed field (a speed, in m/sec or knots)

That's clear, however the "because of" relationship might not be clear. For example if sizeBiggerThanZero failed too, there'll be a "Requires multiple elements" message on bottom and it won't be clear whether "Needs multiple..." or "Requires a composite ..." failed due to sizeBiggerThanZero.
Perhaps we can use indentation to convey this?

from limpet_ops.

Yes, maybe something like this (I've used characters to force the indent).

Selection invalid, reasons as follows:

• Needs multiple composite tracks failed because of:
• == Requires a composite track failed because of:
• == == Needs a course field (an angle, in degrees or radians)
• == == Needs a speed field (a speed, in m/sec or knots)
• == Requires multiple elements

from limpet_ops.

The presentation is checked in. Failure reason related classes are moved to a separate package - samplemodel.failurelog.
The functionality can be tested with the "Concatenate Strings" operation defined in the sampleOperation1 plugin. The operation is only applicable if all selected elements are SampleModel instance and non-numeric.
Here is what is logged in the console, if a String and a Number are selected:

[Concatenate Strings] failed because of:
  [All should be instanceof sample model and not be numeric] failed because of:
    [Should not be numeric]

And here is the result when a String and a Date are selected:

[Concatenate Strings] failed because of:
  [All should be instanceof sample model and not be numeric] failed because of:
    [Should be instanceof sample model]

In essence there is an ExpressionLogger class, that is invoked during expression evaluation (In the CustomExpression class). It creates a hierarchical structure of LoggerNode instances, to represent the failure messages at specific parts of the expression.

from limpet_ops.

Yes, I saw the screenshot of your code earlier on, it looks great (the indent-level code is clever).

I may not get chance to test this until later this week.

from limpet_ops.