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

Rename PluginManager setting #48

Closed
wants to merge 2 commits into from
Closed

Rename PluginManager setting #48

wants to merge 2 commits into from

Conversation

ghost
Copy link

@ghost ghost commented Oct 24, 2020

fixes #35

@ghost ghost marked this pull request as draft October 24, 2020 03:19
Copy link
Member

@RussKie RussKie left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@ghost
Copy link
Author

ghost commented Nov 3, 2020

Not sure if we can restart processes gracefully.

@ghost ghost marked this pull request as ready for review November 3, 2020 22:08
@mast-eu mast-eu self-requested a review November 4, 2020 18:03
Copy link
Member

@mast-eu mast-eu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, I went through the use cases and I think this PR is not yet working as it should.

Use case 1: Set Automatically close Git Extensions when installing plugins = FALSE and try to uninstall a plugin which is used by a second GE instance.

What happens:

  • The PM does not show any dialog
  • All GE instances remain open
  • Plugin not removed (plus, I get a popup from Windows User Account Control)

What should happen:

  • The PM shows the dialog image
  • According to the selection all GE instances are either closed or remain open
  • Plugin removed (if select to close GE)

Use case 2: Set Automatically close Git Extensions when installing plugins = TRUE and try to uninstall a plugin which is used by a second GE instance.

What happens:

  • The PM does show the dialog
    image
  • According to the selection all GE instances are either closed or remain open
  • Plugin removed (if select to close GE)

What should happen:

  • No dialog is shown
  • All Git Extensions instances are closed
  • Plugin removed

@ghost
Copy link
Author

ghost commented Nov 4, 2020

Yes, only works for installations.

@mast-eu
Copy link
Member

mast-eu commented Nov 4, 2020

Not sure if we can restart processes gracefully.

I think we discussed this some time ago (don't remember where and when). Restarting gracefully would at least include to reopen the repos I had opened (might be more than just 1 instance of GE), potentially also restore the window positions and other stuff.
Some internal states (selected branch/commit, filters, whatever) would probably be lost anyhow. Therefore I wouldn't bother with trying to restore something that would likely be incomplete.

@ghost
Copy link
Author

ghost commented Nov 4, 2020

This requires a full session save function, if there is none.

@ghost
Copy link
Author

ghost commented Nov 4, 2020

GitExtensions.exe --restore-last-session

@RussKie
Copy link
Member

RussKie commented Nov 5, 2020 via email

@ghost
Copy link
Author

ghost commented Nov 5, 2020

By process number in the order of the launches.

@ghost
Copy link
Author

ghost commented Nov 5, 2020

LIFO

Open 1, 2, 3;
Close 2, 1, 3.
Reopen 3, 1, 2.

Like a stack. Looks like this.
Updating the top of the stack when closing.

@ghost
Copy link
Author

ghost commented Nov 6, 2020

Use case 1: Set Automatically close Git Extensions when installing plugins = FALSE.
The PM shows the dialog

Set Automatically close Git Extensions when installing plugins = TRUE
No dialog is shown

It is correct?

image

I think it works.

@mast-eu
Copy link
Member

mast-eu commented Nov 7, 2020

Use case 1: Set Automatically close Git Extensions when installing plugins = FALSE.
The PM shows the dialog

Set Automatically close Git Extensions when installing plugins = TRUE
No dialog is shown

It is correct?

I'm not sure I understood you correctly. Is your doubt about when the close dialog should be shown and when not?

When Set Automatically close Git Extensions when installing plugins is not checked, I expect GE to not close automatically. Instead, it should ask me (i.e. showing the dialog):

When Set Automatically close Git Extensions when installing plugins is checked, I expect GE to close automatically. Hence, the dialog is not needed, since I have nothing to choose/confirm.

Yes, only works for installations.

Maybe we should not focus primarily on the install part. IMHO, uninstall is more critical, since it can easily fail.
Just open 2 instances of GE, run a plugin in one instance and try to uninstall the same plugin from the second instance. This will fail, since the DLL cannot be deleted while it is used.
That's why all instances need to be closed before modifying the plugins via Plugin Manager.

@ghost
Copy link
Author

ghost commented Nov 7, 2020

I was confused by "Confirmation".
If this is a confirmation, then nothing should happen after the cancellation, but it is not.
Because closing GE instances is a necessity.

@ghost
Copy link
Author

ghost commented Nov 8, 2020

Only confused by the behavior after canceling the confirmation.

@mast-eu
Copy link
Member

mast-eu commented Nov 8, 2020

behavior after canceling the confirmation.

All GE instances remain open, plugin is not installed/updated/removed.

@ghost ghost closed this Jan 13, 2021
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Modify setting to close GE
3 participants