Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Commands & Operations #8

Closed
IanMayo opened this issue Feb 1, 2016 · 10 comments
Closed

Commands & Operations #8

IanMayo opened this issue Feb 1, 2016 · 10 comments
Assignees

Comments

@IanMayo
Copy link
Member

IanMayo commented Feb 1, 2016

Dinko said:

   The sole responsibility of the operation is checking applicability and 
   providing the commands for certain context. My feeling is that Limpet 
   operations should be dropped, applicability will be provided in a declarative 
   way and the Commands should be renamed to Operations. That will 
   simplify the design.

This is good logic, good suggestion. There is an exception to the processing, where the outer class (the operation) does some more complex processing, including deciding the order in which to pass the items to the command, or offering the user multiple options for running the command.

For example, the subtraction isn't commutative. So, if two applicable datasets are selected we need to offer the user the choice of which one is subtracted from the other:
https://github.com/debrief/limpet/blob/master/info.limpet/src/info/limpet/data/operations/arithmetic/SubtractQuantityOperation.java#L85

Similarly, for the "Doppler" calculation, we can create an operation that could take any of the selection as the "Primary" object, and the other selected items as the "secondary track".

If we can handle instances like this declaratively, then we can certainly drop Operations.

Or, I guess we can remove the Operation wrapper for most commands, just keeping a programmatic solution when necessary.

@dinkoivanov
Copy link
Contributor

Given the subtract operation example, I think we can use the selection order to define which item is 1 and which is 2. For example click A then B and the operation offered will be subtract B from A, click B then A, the operation will be subtract A from B. In this way there will be no need to offer multiple operations based on selection order.
Regarding

...offering the user multiple options for running the command...

the subtract operation should be split in two - SubtractIndexedQuantityValues and SubtractInterpolatedQuantityValues and both operations are declared. If needed, implementing classes can have common superclass.

@IanMayo
Copy link
Member Author

IanMayo commented Feb 6, 2016

RCP doesn't give us the selection order - we just get a structured selection object. I think the items are provided in their comparable order from the source - the tree in our case.

Yes, it's fine to separate them into indexed & interpolated commands.

@dinkoivanov
Copy link
Contributor

You are right, the JFace treeviewer does not track selection order. I got confused with GEF (https://eclipse.org/gef/) viewers, which preserve the selection order.
In that case we can either (1) again split each Indexed and Interpolated subtract operation in two - Subtract A from B and Subtract B from A. That means there'll be four subtract operations declared overall, each with own class (probably common parent). Or (2) we can extend the viewer used in the DataManagerEditor with some logic to preserve the selection order (http://stackoverflow.com/a/12978563). Then use this preserved selection when creating the context menu here: https://github.com/debrief/limpet/blob/master/info.limpet.ui/src/info/limpet/ui/editors/DataManagerEditor.java#L541

@IanMayo
Copy link
Member Author

IanMayo commented Feb 6, 2016

Hmm, maybe we define some "non commutative" flag, and our logic processing creates multiple operations, for any permissible permutations.

@dinkoivanov
Copy link
Contributor

That makes sense. We can even parameterize the operation name, like "Subtract {0} from {1}", then replace with selection on runtime.
I'm not sure how to define the permissible permutations though. Can you give an example for a such operation?
Perhaps assuming there are 3 elements in the selection and 3! permutations, we can just list the numbers of the allowed permutations, like allowedPermutations="1;2;6".

@IanMayo
Copy link
Member Author

IanMayo commented Feb 10, 2016

The subtract operation is typical for collections that are either dimensionless, or have the same dimension.

A similar operation is available for two location datasets: "bearing from" could be from the perspective of either of the operands.

If there are more than two operands, we should invite the user to choose "bearing from" any of the operands. If there are 5 in the selection, then 4 new collections will be produced: with each one being the "bearing from" whichever one the user selected.

[Note: the above "5 from 4" situation isn't just this particular to this commutative situation. Range from isn't commutative. So, it's the same distance whichever of the pair comes first. But, a user may select 5 elements, choose which one it's "range from", then 4 new collections will be produced].

@dinkoivanov
Copy link
Contributor

I've extended the PoC with support for declaration of non-commutative operations. The commit is f46f7f2. If you launch the PoC view now, you'll see two new numerical operations: Subtract and DistanceFrom. In the example below, user selects two numbers and sees two combination for subtracting the numbers:
capture
DistanceFrom operation is applicable for 2 or more numbers:
capture2
I've introduced an optional Java class-typed attribute in the extension point schema, called inputPermutator. Normally non-commutative operations will use this attribute. This class will produce the different permutations of the input. This will be taken in account when building the context menu so that multiple entries will be added for an operation. Possible enhancement would be to introduce a submenu (menu nesting), so that there is a "Subtract >" menu and then "1 from 12", "12 from 1" for example.
Operation name now can be parameterized using Java MessageFormat syntax (https://docs.oracle.com/javase/7/docs/api/java/text/MessageFormat.html), such as "Subtract {1} from {0}".
Important note is that it's best to define the concrete permutators in the base plugin (sampleModel in this case), since it's loaded during building the context menu and before operation execution.

@IanMayo
Copy link
Member Author

IanMayo commented Feb 14, 2016

Hi Dinko, that looks just great - nicely done. The submenu will certainly make tidier. Could you develop some submenu thoughts in the demonstrator?

Cheers,
Ian

@dinkoivanov
Copy link
Contributor

Non-commutative operations (those that specify an inputPermutator in the schema) are now grouped via submenu: 2dd6dfb. Note the format of the operation name:
Subtract>{1} from {0}

In addition, with commit cf7bb42 I address your concern:

Hopefully the sub-menu won't be too RCP/JFace-centric

There is now a SampleModelOperationRegistry singleton, that resides in the model plugin and handles building the operation library together with an IOperationLibraryBuilder. The latter decouples operation traversal from building the library. The registry does not know anything about the UI/JFace except the org.eclipse.jface.IStructuredSelection. In the sampleView plugin there's a concrete OperationLibraryMenuBuilder that will actually use JFace menu manager to build the menu.

@IanMayo
Copy link
Member Author

IanMayo commented Mar 3, 2016

Issue complete - non-commutative operations are now supported.

@IanMayo IanMayo closed this as completed Mar 3, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants