Re: [e4-dev] Feedback from a frustrated user on the @(Can)Execute API

[Tom Schindl <tom.schindl@xxxxxxxxxxxxxxx>]

Hi,

I'm sorry if the next few line will add even more to your frustration
because what you are doing here is just plain wrong :-(.

@CanExecute not called if things change
---------------------------------------
Let's start with a false assumption on @CanExecute. Unfortuantely
@CanExecute is not called if values accessed in the signature are
changing in the context. In general such a thing should be handled with
RATs for CIF#invoke this RAT thing does NOT work!

Hence we require you to post events on the EventBus to force a
revalidation of those (some special things we already track ourselves
like active-part changes, ...)


Abuse of @CanExecute
--------------------
You are abusing @CanExecute for something it is not designed for. First
of all @CanExecute sole purpose is to control the enablement/disablement
of a command - point.

You further assume that the command is bound to a MMenuItem but it might
as well get bound to a keybinding, toolbar item.


The correct solution:
---------------------
The correct solution is to use an Addon who listens to the preference
change, querys the model to find the required MMenuItems, ... and
setting the check-state of the MMenuItem. Your handler should never ever
access the MMenuItem, MToolBarItem it should just flip the preference
(as I said it might get triggered by a keybinding!)

Look at [1] where I do something like that in efxclipse application, it
uses our preference story and does some more complex things but i think
you get the idea.

Tom

[1]https://github.com/BestSolution-at/dartedit/blob/master/bundles/at.bestsolution.dart.app/src/at/bestsolution/dart/app/addons/EditorFeatureAddon.java

On 03.03.17 15:36, Andreas Sewe wrote:
> Dear e4 Developers,
> 
> I hope this is the right venue for this (as opposed to the Eclipse 4
> forum or Bugzilla).
> 
> I just like to share a very frustrating experience I had trying to
> accomplish a seemingly simple task: Implementing a check-type menu item
> whose value is synchronized with a preference. For simplicity, I choose
> a Direct Menu Item with the following "handler":
> 
>   class MyHandler {
> 
>     @PostConstruct
>     void init(MMenuItem item, @Preference(MY_PREF) boolean selected) {
>       item.setSelected(selected);
>     }
> 
>     @Execute
>     void exec(MMenuItem item, @Preference IEclipsePreferences prefs) {
>       prefs.putBoolean(MY_PREF, item.isSelected());
>     }
>   }
> 
> The above looks downright *elegant*, with the @Preference annotation and
> all. Unfortunately, it doesn't work; the @PostConstruct method cannot be
> called since the context does not *yet* contain the MMenuItem for my
> handler.
> 
> One StackOverflow question [1] later, I've converted the above to a
> "solution" using @CanExecute:
> 
>   class MyHandler {
> 
>     @CanExecute
>     void can(MMenuItem item, @Preference(MY_PREF) boolean selected) {
>       item.setSelected(selected);
>       return true;
>     }
> 
>     @Execute
>     void exec(MMenuItem item, @Preference IEclipsePreferences prefs) {
>       prefs.putBoolean(MY_PREF, item.isSelected());
>     }
>   }
> 
> Using @CanExecute for this seems like a hack, as I don't really
> determine the handler's executability; I merely want to synchronize the
> MMenuItem with the preference and, unlike in @PostConstruct, now the
> context contains my handler's associated MMenuItem.
> 
> Unfortunately, the above still doesn't work -- and it's not obvious (to
> me, at least) why!
> 
> The problem with the second "solution" is that the menu item's selection
> state is seemingly "stuck" at the preference's initial value. The root
> cause is that the @CanExecute is executed twice: First when the menu is
> created. Then after the user has clicked on the item, at which point in
> time the selection state has already changed -- and gets reset in
> @CanExecute.
> 
> My final solution looks hence like this:
> 
>   class MyHandler {
> 
>     boolean prefSynced = false;
> 
>     @CanExecute
>     void can(MMenuItem item, @Preference(MY_PREF) boolean selected) {
>       if (!prefSynced) {
>         item.setSelected(selected);
>         prefSynced = true;
>       }
>       return true;
>     }
> 
>     @Execute
>     void exec(MMenuItem item, @Preference IEclipsePreferences prefs) {
>       prefs.putBoolean(MY_PREF, item.isSelected());
>     }
>   }
> 
> Granted, that's still not a lot of code, but how to get there was far
> from obvious.
> 
> I hope this (lengthy) story helps you to further improve the Eclipse 4
> API; despite episodes like the above, it has generally been a pleasant
> experience.
> 
> Best wishes,
> 
> Andreas
> 
> [1] <stackoverflow.com/questions/42575661/>
> 
> 
> 
> _______________________________________________
> e4-dev mailing list
> e4-dev@xxxxxxxxxxx
> To change your delivery options, retrieve your password, or unsubscribe from this list, visit
> https://dev.eclipse.org/mailman/listinfo/e4-dev
> 


-- 
Thomas Schindl, CTO
BestSolution.at EDV Systemhaus GmbH
Eduard-Bodem-Gasse 5-7, A-6020 Innsbruck
http://www.bestsolution.at/
Reg. Nr. FN 222302s am Firmenbuchgericht Innsbruck