Community
Participate
Working Groups
initial DnD allows to retrieve only one command per strategy. some strategies could retrieve multiple possible commands
*** Bug 469719 has been marked as a duplicate of this bug. ***
Gerrit change https://git.eclipse.org/r/49990 was merged to [master]. Commit: http://git.eclipse.org/c/papyrus/org.eclipse.papyrus.git/commit/?id=f386adf5bd9cb4b8e55107f9ffc82d9f8020edc7
> Gerrit change https://git.eclipse.org/r/49990 was merged to [master]. This commit causes UI Freeze by opening the Drag/Drop dialog during test execution The following jobs are affected: https://hudson.eclipse.org/papyrus/job/Papyrus-Master-Tests-Failures/ (Fails since this commit) https://hudson.eclipse.org/papyrus/job/Papyrus-Master-Tests/ (Was already failing before, but now fails on this issue)
Created attachment 255339 [details] Test execution screenshot Screenshot from the frozen Hudson job (Master-Tests-Failure)
The current status of this task is definitely not satisfying: - The CustomizableDropStrategy relies on a lot of highly restrictive "instanceof". The contract for the extension point is the "DropStrategy" interface. Most methods from CustomizableDropStrategy only handle "TransactionalCommandDropStrategy". I don't understand the various "return null" instructions for TransactionalCommandsDropStrategy, and implementations that are not transactional are simply ignored - TransactionalCommandsDropStrategy offers a new API for handling multiple commands, but it is not an Interface. Developers cannot implement non-transactional drop strategies using multiple commands - I couldn't find any example implementation of TransactionalCommandsDropStrategy in Papyrus, so this new feature that introduces so many regressions isn't even used In the current state, I would rather revert these changes entirely, until a complete, clean and regression-free feature can be proposed
These code modifications are used in an extra plugins here: https://git.eclipse.org/c/papyrus/org.eclipse.papyrus.git/tree/extraplugins/uml/org.eclipse.papyrus.uml.diagram.dnd.smart Perhaps I miss understand something because I have used "instanceof" in CustomizableDropEditPolicy after a meeting we had together where you have highlight that my previous implementation break the initial API. There are only a few modifications (3 files), so perhaps you can create an additional bug with the additional requirements? May we make a small meeting on the subject? Francois
> Perhaps I miss understand something because I have used "instanceof" in CustomizableDropEditPolicy after a meeting we had together where you have highlight that my previous implementation break the initial API. Indeed; the issue is not the usage of "instanceof" as such; it is that it has been used in a highly restrictive way, allowing only two specific implementation classes to be used (While the contract from the extension point is that any class implementing DropStrategy is valid) The right pattern would look like: public interface MultipleDropStrategy extends DropStrategy { //new methods } Then in the runtime: if (contribution instanceof MultipleDropStrategy) { //New behavior } else { //Standard behavior } Which would work for any implementation of DropStrategy or MultipleDropStrategy. And in this case, we never rely on implementation-specific code (TransactionalDropStrategies are only provided as a helper implementation to simplify usage of transactions within Papyrus; they are absolutely not mandatory, and shouldn't be).
A patch has been submitted here [1] [1] https://git.eclipse.org/c/papyrus/org.eclipse.papyrus.git/commit/?id=4b1361f7269cf0db9ce94494de959859a64a1435
