What is the best way to override/decorate default components?

I made a plugin where I found that I had to override classes like FileDocumentManager and EditorSettingsExternalizable to add extra events to subscribe to within my plugin (I posted another topic about the additional events I needed). Currently, I am subclassing FileDocumentManagerImpl and EditorSettingsExternalizable, overriding a couple methods in each, and then registering my alternative implementations with the component manager. However, I suppose this could cause me to stomp on other plugins doing the same thing. Would it be better to use getInstance to get the current instance of each of those components, then wrap/decorate it in my class, and then register that instance with the component manager/picocontainer? The picocontainer docs discourage using the "registerInstance" methods, but is this a case where it is good to do so?

7 comments

Trying to replace IDEA's core components with your custom implementations is something that we consider a very bad idea, and it's pretty much guaranteed to break in future versions of IntelliJ IDEA. I certainly hope that you won't stomp on any other plugins doing that, because no one else is doing it. It would be better to find another approach to solving your problem.

0

Hi Dmitry,

Of course I started by looking for less intrusive ways of doing what I am, but I couldn't find one. I followed the example of Eclipse Code Formatter, which overrides componenents using picocontainer. The auther of that plugin wrapped an instance of an IntelliJ component with his own class, and then registered that using picocontainer.registerinstance. Is that "less bad" than what I'm doing?

Also, the primary reason I feel it's been necessary to override intellij components is that there are certain settings that are global in intellij, but may be applied per-file in the editorconfig specification (settings like stripTrailingWhitespace, ensureEOF, etc.) I want to override these by simply wrappig file saves like this:

store old setting
apply editorconfig setting
[save happens]
revert to old setting

But I can't do that because there is no event fired after each file is saved, right?

0

No, it's not "less bad", it's just as bad. :) But in the case of Eclipse Code Formatter, there is really no better solution.

In this case, I would solve the problem by adding an extension point to TrailingSpacesStripper that would allow replacing the global settings on a per-file basis.

0

Hey Again,

I tried looking into working through TrailingSpacesStripper, but I don't see how I can "add an extension point" to it...

"myTrailingSpacesStripper" is a private field of FileDocumentManagerImpl, so I can't replace it with my own alternative implementation, and FileDocumentManagerImpl gets its TrailingSpacesStripper instance by calling "new" directly on TrailingSpacesStripper, so I can't replace it using the picocontainer dependency injection. TrailingSpacesStripper also doesn't appear to fire any events or provide anything else that I can hook into.

Can you give an example of what you had in mind?

Thanks!

0

I meant sending us a pull request that will add the necessary extension point to the implementation of TrailingSpacesStripper.

0

Hey Dmitry,

I have been very busy with work lately, so I haven't had a chance to look at this in a long time. I think I can give the plugin some long-overdue TLC now though. My tentative plan is:

1. Remove the plugin's attempts to enforce editorconfig newline settings, because JetBrains is already pretty smart about newlines
2. Remove the enforcement of editorconfig's stripping trailing whitespace/ensure final newline options (together, 1 and 2 should get rid of all of the picocontainer stuff)
3. Put the exension in TrailingSpacesStripper as you described, to re-enable (2)

Does that make sense? Also, is there a method that you would recommend for extending TrailingSpacesStripper? Does making it fire "willStripSpaces(Document)" and "doneStrippingSpaces(Document)" make sense? Or would you do something else? Like use a strategy or something?

Thanks,

Kevin

0

Actually, I went ahead and tried implementing it using UserData, kind of like how LoadTextUtil handles newline settings. The pull request is here:
https://github.com/JetBrains/intellij-community/pull/139

Also, I released an update to this plugin (v0.2.0), which gets rid of all the nasty overriding :)

0

Please sign in to leave a comment.