-
-
Notifications
You must be signed in to change notification settings - Fork 2.1k
Provide IServiceProvider to FormStatus #11320
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
Conversation
GitUI/HelperDialogs/FormProcess.cs
Outdated
private FormProcess(IServiceProvider serviceProvider, | ||
GitUICommands? commands, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why are we doing this? GitUICommands
is a service provider
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
commands
is optional (can be null
), serviceProvider
is mandatory.
This ctor is private
.
Then we just need to make mandatory
|
For making UICommands mandatory, we would need to switch to IGitUICommands instead of GitUICommands... |
Then we need to prioritise #11269 as it would likely unblock the use of the
interface.
I wonder what these "some cases"... Still, we can construct and pass an
instance of the commands.
|
Ask your self from 4 years ago ;-) I do not see why FormStatus and all its descendants need to be a GitModuleForm. They have never used GitUICommands and should never do so except for IServiceProvider. Though since an IGitUICommands - and thereby IServiceProvider - instance is already provided everywhere and since a refactoring of the grown inheritance chain will be even more difficult, |
I agree we should always pass an instance of IGitUICommands right now. In
the target state I don't want any UI elements to depend on that explicitly,
and instead get the required dependencies via the service provider.
I'm currently dealing with health issues, when I feel better I'll have a
look in more detail.
|
when running scripts by downcasting from IGitUICommands to GitUICommands Make GitUICommands instance mandatory for FormStatus & FormProcess
I have tried the naive approach to use the completed So I wait for #11269 and the follow-ups. For the meantime, I am downcasting |
For the meantime, I am downcasting IGitUICommands to GitUICommands for
calling FormProcess.ShowDialog in RunScriptInternal.
Sounds good, just add a tracking comment.
…On Sun, 5 Nov 2023, 8:06 am Michael Seibt, ***@***.***> wrote:
I have tried the naive approach to use the completed IGitUICommands
instead of GitUICommands - as a quick first step ... I thought.
It will not work without completing IGitModule, too.
Raise event functions would need to be declared public: db5c181
<db5c181>
.
Several types would need to be moved to GitUIPluginInterfaces: 796cc11
<796cc11>
.
So I wait for #11269
<#11269> and the
follow-ups.
For the meantime, I am downcasting IGitUICommands to GitUICommands for
calling FormProcess.ShowDialog in RunScriptInternal.
—
Reply to this email directly, view it on GitHub
<#11320 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ABBTEXTEQGZFDY3PQYKHEKTYC2U5NAVCNFSM6AAAAAA62APRUCVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTOOJTGU2TKMBSHE>
.
You are receiving this because you commented.Message ID:
***@***.***>
|
Fixes: preparation for git output / history control
Proposed changes
FormStatus
has aGitUICommands
instance.Screenshots
N/A
Test methodology
Merge strategy
I agree that the maintainer squash merge this PR (if the commit message is clear).
✒️ I contribute this code under The Developer Certificate of Origin.