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

IAction must not be disposable. #158307

Merged
merged 3 commits into from Aug 16, 2022
Merged

IAction must not be disposable. #158307

merged 3 commits into from Aug 16, 2022

Conversation

jrieken
Copy link
Member

@jrieken jrieken commented Aug 16, 2022

Fixes #154938

The IAction interface is currently marked as disposable but that's for no good reason. Because of the MenuItemAction and SubmenuItemAction are also "fake disposable" which has a ripple where various places assume and collect something to dispose just for the sake of calling empty methods. The "empty dispose" function for IAction can be seen in various other places and all of that is cleaned-up with this PR.

Another good consequence is that createAndFillInActionBarActions and createAndFillInContextMenuActions don't return disposables anymore. This makes using menus simpler but #154939 plans even more simplifications.

There is a few unfortunate places where we don't know if we are dealing with simple IAction-instances or disposable Action-instances. For those cases I have introduced the disposeIfDisposable-util. The hope is that further clean-up removes the need for this

…mentations and unneeded dispose chains. Use `Action` over `IAction` type if needed
@jrieken jrieken self-assigned this Aug 16, 2022
@jrieken jrieken added this to the August 2022 milestone Aug 16, 2022
@jrieken jrieken marked this pull request as ready for review August 16, 2022 18:59
@jrieken jrieken enabled auto-merge August 16, 2022 19:09
@jrieken jrieken merged commit 1f86576 into main Aug 16, 2022
@jrieken jrieken deleted the joh/calm-alligator branch August 16, 2022 19:10
@github-actions github-actions bot locked and limited conversation to collaborators Sep 30, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

IAction shouldn't be disposable, only Action is
2 participants