Skip to content

Show ProgressMonitorJobsDialog after delay in ProgressManager.run #3147

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

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

fedejeanne
Copy link
Contributor

Avoids a "flickering" produced by showing the progress dialog and closing it immediately for short-running tasks, improving the UX.

Before this PR, calling either one of these 2 methods ...

  • PlatformUI.getWorkbench().getProgressService().run(...)
  • PlatformUI.getWorkbench().getActiveWorkbenchWindow().getActivePage().getActivePart().getSite().getAdapter(IWorkbenchSiteProgressService.class).run(...)

... and passing false as the 1st and/or the 2nd parameter (fork / cancelable) ended up showing a ProgressMonitorJobsDialog immediately. On the other hand passing true to both parameters showed the dialog after a short delay of 800 ms.

After this PR, the dialog is always shown after a delay of 800 ms.

Even for the backwards compatible mode (when the runnable is not forked
or non-cancelable).
@iloveeclipse
Copy link
Member

... and passing false as the 1st and/or the 2nd parameter (fork / cancelable) ended up showing a ProgressMonitorJobsDialog immediately

Which probably was expected / documented in javadocs? Have you checked that?
Behavior change may also break existing code / tests expecting new shell to appear immediately.

@fedejeanne
Copy link
Contributor Author

Which probably was expected / documented in javadocs? Have you checked that?

The javadocs make no promises about whether or not the code runs in the UI thread.

* If <code>fork</code> is <code>false</code>, the current thread is used
* to run the runnable. Note that if <code>fork</code> is <code>true</code>,
* it is unspecified whether or not this method blocks until the runnable
* has been run. Implementers should document whether the runnable is run
* synchronously (blocking) or asynchronously (non-blocking), or if no
* assumption can be made about the blocking behaviour.

In fact, in this specialization, the javadoc says that it might run the runnable asynchronously if fork is true

* IRunnableWithProgress) might run the runnable asynchronously if
* <code>fork</code> is <code>true</code>.

Behavior change may also break existing code / tests expecting new shell to appear immediately.

The "backwards compatible code" was added in 2004 (28429cf) and it provides a bad UX (flickering). I'd very much rather improve the UX if possible, even at the cost of letting clients adapt after 20+ years.
But let's wait for the tests before making decissions :-)

@fedejeanne
Copy link
Contributor Author

Scratch my last comment, the JavaDocs talk about whether or not it runs in another thread but not about a delay. There are no promises about when is the dialog shown so this PR doesn't break any contract.

Side note: I'm not targeting this one for M3. I'll draft it as soon as I get to my computer.

Copy link
Contributor

github-actions bot commented Aug 6, 2025

Test Results

 2 778 files  ±0   2 778 suites  ±0   1h 44m 1s ⏱️ + 7m 43s
 7 932 tests ±0   7 703 ✅  - 1  228 💤 ±0  1 ❌ +1 
23 351 runs  ±0  22 604 ✅  - 1  746 💤 ±0  1 ❌ +1 

For more details on these failures, see this check.

Results for commit f3f8ff5. ± Comparison against base commit 4a2727c.

@fedejeanne fedejeanne marked this pull request as draft August 6, 2025 16:34
@fedejeanne
Copy link
Contributor Author

Failing test is unrelated #3042

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.

2 participants