Unexpected behaviour binding custom ActionHandlers

Hi everyone,

we want to observe what RS features are used and for RS-Actions we do this by installing a logging IActionHandler on every UpdateableAction known to the IActionManager.
This worked for R#8 but since we upgraded to R#9 we had an unexpected problem:
Whenever we invoke the nextUpdate of an IExecutableAction, our handler isn't called for execution, even for actions that are clearly enabled (e.g. CodeCleanup).
Always returning true without invoking nextUpdate solves this issue but enables every IExecutableAction, which is obviously not what we want.

I've built a small extension to show this issue:
https://github.com/M8is/resharper-custom-handler

For simplification I just log every action for which our handler was called in C:\log.txt
This works for actions like TextControl but using CodeCleanup as an example we get no line in the log file.
However, when I comment out line 32 in MyCustomHandler.cs (nextUpdate.Invoke();) it works as described above.

This behaviour seems unexpected, especially since Update returns true anyways (even with invoking the next update).

Thanks in advance!

Best,
Mattis

6 comments
Comment actions Permalink

Just to elaborate on this a bit since my initial post might be a bit unclear:

I add an instance of MyCustomHandler for each action from the ActionManager in BindMyCustomHandlerToAllActions, which works fine.
In this example I just implemented MyCustomHandler as an action that just logs in "C:\log.txt" whenever the action gets executed.

The main problem seems to be the Update method in MyCustomHandler:
We would like to have our handler enabled for every action where any other handler is enabled.
Our first (intuitive) approach was just 'return nextUpdate();' as this does not change the decision. The problem we encountered with this solution is that for some reason, our handler's execute doesn't get called anymore - even for actions which are obviously enabled and executed (e.g. CodeCleanup, refactorings like ChangeSignature, menu actions like ExtensionManager). It is still called for actions like TextControl.Backspace.
Then we tried just returning true, hoping that this just always enables our handler. This works, but unfortunately it seems like nextUpdate contains things like hiding the "Why ReSharper disabled..." actions in the ReSharper menu.
This lead to the current version in my example extension. We still just return true, but we invoke nextUpdate. The result is essentially the same as 'return nextUpdate();', as actions are enabled as normal, but our handler's execute is not called anymore.

Just for completeness I tried to change the true to false, which didn't change the result.

I hope this made our problem a bit clearer. It's pretty important for us to solve this as soon as possible, since this cuts out a large portion of our extension.

Thanks in advance!

Regards,
Mattis

0
Comment actions Permalink

TL;DR: If you want to monitor action usage, use ActionEvents.AdviseExecuteAction. This will call your callback whenever an action is invoked. Alternatively, implement IActivityTracking, which will get called both when an action gets invoked, but also when workflows are invoked - navigate to, refactor this, generate, etc. It will even let you know when Visual Studio actions are called. You can see this in action in the Presentation Assistant plugin.

There isn't really any support to override every action - how would you know what to do for ALL actions? More specifically, the decorator pattern doesn't work for action handlers. If you want to override specific actions, you can create a handler that derives from OverridingActionHandler (or better yet, SimpleOverridingActionHandler, which handles the semantics for you).

So, the longer version. It looks like nextUpdate didn't get changed as part of the major overhaul that actions had in v9 - there are so many actions that maybe this bit of cruft is preferable than manually going through several hundred action handlers... But, I have to admit that I don't like nextUpdate, because it's kind of lying to you - it doesn't call the next handler's Update method, not any more.

The behaviour you're seeing is correct, and explainable, but less intuitive due to this change of behaviour in nextUpdate. The idea is this - ReSharper will skip calling Execute on any handler that doesn't support the action in the current context. If it didn't, then each handler would have to duplicate the Update logic in its Execute method, and Update would essentially be called twice, unnecessarily (while this shouldn't really be a performance issue, as Update already needs to be quick, it might help with memory allocations if the Update logic is complex. And hey, doing less work is always faster).

Actions can have multiple handlers, but usually only have one. Overriding a handler is really intended for things such as allowing Escape to undo several levels of highlighting, etc. When you hit Escape, the handlers need to say whether the action is available or not. ReSharper asks them all in turn, and the first to answer "available" or "unavailable" sets the state of the action. But what if Escape is hit in a different tool window? We wouldn't want to remove highlighting for the previous editor window, so the handler needs a third state - "unavailable in the current context". ReSharper should skip calling Execute on this handler, because it's unnecessary, and that removes the need for the handler to duplicate Update.

Ideally, a handler's Update method would return a tri-state value - "available", "unavailable", "unsupported in the current context". But, the return value is a bool, and instead, you call nextUpdate if you don't support the action. Previously, this would call the next handler in the chain, which would make a decision, or ask the next handler (so ReSharper didn't explicitly loop over all the handlers). This is an intuitive, chain of command, decorator pattern kind of thing. But it doesn't allow for tracking which handler(s) should be skipped for execution.

So the current version doesn't call the next handler when you call nextUpdate. Instead, the current implementation treats calling nextUpdate as saying "unsupported in this context" (which was always the case) and it will ignore the return value of the current handler, and flag that it doesn't support the action. ReSharper now explicitly loops over all of the handlers. It keeps going until one actually does support the action, either "available" or "unavailable". Since ReSharper now knows which is the first handler that supports the action, it will call that handler's Execute, skipping any that can before it.

It should be noted, though, that Execute still supports nextExecute, and DOES allow for using the chain of command/decorator pattern for execution (which is probably for the best - you can do something before and after the original handler).

0
Comment actions Permalink

Thank you very much for the detailed answer! Your TL;DR solved our issue very conveniently. :)

0
Comment actions Permalink

Hey Matt,

thank you very much for your detailed response, it helped me a lot in understanding the action concept of ReSharper and it was easy to get all the information we are interested in by adapting our previous solution to the approach you proposed.

My next goal is to capture some navigation information of developers, e.g.: a) when and where did developers ctrl+click on classes/variables/etc in a code editor for navigation or b) which files were opened by a developer by double-clicking in the solution explorer.

Even though I tried to subscribed to all available actions, it seems that there are no advise notification for these kind of actions. Is there any way to get this information from the ReSharper model, or do I have to instrument parts of Visual Studio myself to get access to this kind of data?

best
Sebastian

0
Comment actions Permalink

You will likely have to hook into some VS events, as some of the things you're after happen on the bounday of what's ReShaprer and what's Visual Studio.

ReSharper does hook the creation of windows and editors, but doesn't directly make that availble - it has abstractions over the top of it. It has the concept of documents and text editors, and these don't always map to open files in a project.

Also, ReSharper doesn't let you know when actions are triggered by a "mouse shortcut", like ctrl+click. It just invokes the action handler without notifying the usage statistics functionality. I've raised an issue for this: RSRP-447903.

0
Comment actions Permalink

Thank you very much, I will keep an eye on this feature request. :)

0

Please sign in to leave a comment.