-
Notifications
You must be signed in to change notification settings - Fork 150
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
Remove cancel dialog during Eclipse shutdown #1290
base: master
Are you sure you want to change the base?
Conversation
Removes the dialog during Eclipse shutdown which only flashes up in my Eclipse SDK build without any option to read it or interact with it. The currently dialog behavior is also highly inconsistent or non-functional. The cancel dialog during Eclipse shutdown, does not allow to cancel the shutdown, it allow affects the history pruning in the hard-code assumption that pruning happens in (Math.abs(total - 4.0)). Fixes #1269
5fb4762
to
9497226
Compare
I will have a look once we have agreed on the solution concept in #1269 to not have separate discussions until we have agreed on an approach. |
Is not this the only way to resolve a live lock between hanged jobs? I remember this dialog saving me a couple of times, because each job has its own cancel button (red square) and plugins often fight each other for workspace (and other) resources. I could stop less important jobs and allow more important jobs (like saving resources) to complete without waiting for their opponents. |
This is only about a progress dialog for the save operation performed during workbench shutdown, so it will not affect any other job progress dialog shown. @basilebs You probably refer to progress dialogs popping up while using the IDE and not during shutdown, right? |
Nope, a dialog with a list of jobs pops up on shutdown, if jobs are long running. |
I see, but then you probably refer to the "ordinary" progress dialog that shows all the jobs that are still running during shutdown? The one in this PR is really only a single progress bar for the save operation in which you can currently cancel one subtask of the save operation. It does not allow to cancel any other jobs. |
As long as there is only a single progress bar, it is indeed different from the one I've thought about. Thanks and sorry for the confusion. |
Thanks for the feedback, then the usecase you mention should not be affected. And no problem, thanks for asking! Could have also been that we missed some use case of the removed functionality. |
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.
I like this simplification and the removal of the broken opportunity to cancel the history cleaning step. I would only propose a change to avoid a freezing UI in case the save operation takes rather long by using a busy cursor, which automatically opens a progress dialog in case the operation takes too long.
try { | ||
final ProgressMonitorJobsDialog p = new CancelableProgressMonitorJobsDialog( | ||
null); | ||
|
||
final boolean applyPolicy = ResourcesPlugin.getWorkspace() | ||
.getDescription().isApplyFileStatePolicy(); | ||
|
||
IRunnableWithProgress runnable = monitor -> { | ||
try { | ||
if (applyPolicy) | ||
monitor = new CancelableProgressMonitorWrapper(monitor, p); | ||
|
||
status.merge(((Workspace) ResourcesPlugin.getWorkspace()).save(true, true, monitor)); | ||
} catch (CoreException e) { | ||
status.merge(e.getStatus()); | ||
} | ||
}; | ||
status.merge(((Workspace) ResourcesPlugin.getWorkspace()).save(true, true, new NullProgressMonitor())); | ||
|
||
p.run(true, false, runnable); | ||
} catch (InvocationTargetException e) { | ||
status | ||
.merge(new Status(IStatus.ERROR, | ||
IDEWorkbenchPlugin.IDE_WORKBENCH, 1, | ||
IDEWorkbenchMessages.InternalError, e | ||
.getTargetException())); | ||
} catch (InterruptedException e) { | ||
} catch (CoreException e) { | ||
status.merge(new Status(IStatus.ERROR, | ||
IDEWorkbenchPlugin.IDE_WORKBENCH, 1, | ||
IDEWorkbenchMessages.InternalError, e)); |
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.
As mentioned here, I would propose to execute the save operation in a busyCursorWhile
of the ProgressManager
, so that the UI does not freeze in case saving takes long. Instead a busy cursor will be shown first and when it takes a "long time" (currently more than 800ms), it will open a progress dialog.
ProgressManager.getInstance().busyCursorWhile(monitor -> {
try {
status.merge(((Workspace) ResourcesPlugin.getWorkspace()).save(true, true, monitor));
} catch (CoreException e) {
status.merge(new Status(IStatus.ERROR,
IDEWorkbenchPlugin.IDE_WORKBENCH, 1,
IDEWorkbenchMessages.InternalError, e));
}
});
Removes the dialog during Eclipse shutdown which only flashes up in my Eclipse SDK build without any option to read it or interact with it.
The currently dialog behavior is also highly inconsistent or non-functional.
The cancel dialog during Eclipse shutdown, does not allow to cancel the shutdown, it allow affects the history pruning in the hard-code assumption that pruning happens in (Math.abs(total - 4.0)).
Fixes #1269