[fedejeanne]

busyCursorWhile always showed an enabled Cancel button. The new overload disables it from the start for operations that can't be canceled while keeping the input blocking of busyCursorWhile.

How to test

Use the newly added checkbox in the Job Factory view: if you uncheck it then the Cancel button is grayed out.

[github-actions]

Test Results

   867 files  ±0     867 suites  ±0   50m 44s ⏱️ - 3m 33s
 8 466 tests ±0   8 226 ✅ ±0  240 💤 ±0  0 ❌ ±0 
21 270 runs  ±0  20 593 ✅ ±0  677 💤 ±0  0 ❌ ±0 

Results for commit 1ea502b. ± Comparison against base commit 1715ff2.

♻️ This comment has been updated with latest results.

[Copilot]

🟡 Changes recommended

The new tests can pass without displaying the dialog, leaving the user-visible Cancel-button behavior unverified.

1 open issue

• Tests accept hidden dialog without verifying visible Cancel button · New

What changed in this PR

Adds optional cancellation control to workbench and E4 busyCursorWhile, preserving existing behavior for callers of the original overload.

Changes:

• Adds and propagates a cancelable parameter.
• Adds cancellation checkboxes to both Job Factory examples.
• Adds Cancel-button tests and bumps the E4 progress bundle version.

File	Description
tests/​org.eclipse.ui.tests/​Eclipse UI Tests/​org/​eclipse/​ui/​tests/​progress/​ProgressServiceTest.java	Tests Cancel-button enablement.
examples/​org.eclipse.ui.examples.job/​src/​org/​eclipse/​ui/​examples/​jobs/​views/​JobsView.java	Adds cancellation checkbox.
examples/​org.eclipse.e4.ui.examples.job/​src/​org/​eclipse/​e4/​ui/​examples/​jobs/​views/​JobsView.java	Adds E4 cancellation checkbox.
bundles/​org.eclipse.ui.workbench/​eclipseui/​org/​eclipse/​ui/​progress/​IProgressService.java	Declares workbench overload.
bundles/​org.eclipse.ui.workbench/​eclipseui/​org/​eclipse/​ui/​internal/​progress/​WorkbenchSiteProgressService.java	Delegates new overload.
bundles/​org.eclipse.ui.workbench/​eclipseui/​org/​eclipse/​ui/​internal/​progress/​ProgressManager.java	Applies cancellation option.
bundles/​org.eclipse.e4.ui.progress/​src/​org/​eclipse/​e4/​ui/​progress/​IProgressService.java	Declares E4 overload.
bundles/​org.eclipse.e4.ui.progress/​src/​org/​eclipse/​e4/​ui/​progress/​internal/​ProgressServiceImpl.java	Applies E4 cancellation option.
bundles/​org.eclipse.e4.ui.progress/​META-INF/​MANIFEST.MF	Bumps bundle version to 0.6.0.

_{Review effort: Balanced}

[Copilot]

🔵 Needs a closer look

Cross-platform SWT close and Escape behavior still needs UI validation; tests were not executed.

No open issues 🎉

1 resolved since last review

• Tests accept hidden dialog without verifying visible Cancel button

_{Review effort: Balanced}

[laeubi]

Thanks for the PR, Federico, and for the screenshot, it makes the behavior easy to see.

I have some concerns about the direction, though.
The screenshot shows a dialog with disabled Cancel and Details for a 10 second operation.
If the operation takes much longer, the user is blocked with no way out except killing the IDE.
The new overload makes this easy to do and gives it an official blessing.

I see two cases:

• Short operation: the dialog only appears after getLongOperationTime() anyway, so BusyIndicator.showWhile(display, …) from SWT already gives the busy cursor without extra API.
• Long operation: a non-cancelable blocking dialog is a poor experience; a Job (or run(true, false, …), see PlatformUI.getWorkbench().getProgressService().run(true, false, runnable) does not show progress dialog anymore #4086/Only set dialog.setOpenOnRun(false) according to preference #486 #4124) with visible progress, or making the work cancelable, seems better.

Two further points:

• dialog.setCloseCancels(true) is unconditional, so close box/ESC now cancels for all existing busyCursorWhile(runnable) callers (e.g. SaveableHelper, CachedMarkerBuilder).
  That is a behavior change independent of the new overload and, as the Copilot review notes, still unvalidated on Linux/macOS.
• Dropping SWT.MAX to hide the close box relies on a Windows side effect.

Could you share the concrete use case (a caller that must block, can't be canceled, and isn't short)?
If there is one, I'd suggest keeping this PR to the overload, and moving the close/ESC change to a separate PR.
That way each can be discussed on its own.

[fedejeanne]

Thanks Christoph, fair points.

Could you share the concrete use case (a caller that must block, can't be canceled, and isn't short)?

eclipse-jdt/eclipse.jdt.ui#2415: save actions that need an AST (e.g. Organize Imports) compute it in CleanUpPostSaveListener, during the save, which can take seconds on large files. The work must block input from the start, because typing into the editor being saved would conflict with the computed changes. It can't be cancelled midway: [why cancelling the AST computation isn't viable].

Short operation: the dialog only appears after getLongOperationTime() anyway, so BusyIndicator.showWhile(display, …) from SWT already gives the busy cursor without extra API.

BusyIndicator.showWhile runs on the UI thread without pumping events. That's the freeze users see today during save (eclipse-jdt/eclipse.jdt.ui#2414), which #2415 fixes. It also isn't always short: on large files it takes seconds.

Long operation: a non-cancelable blocking dialog is a poor experience; a Job (or run(true, false, …), see #4086/#4124) with visible progress, or making the work cancelable, seems better.

• run(true, false, …) takes the backward-compatible path: it doesn't block input until the dialog opens after getLongOperationTime(), so the user can keep typing for the first ~800 ms. Those edits make the AST stale, so the clean-ups would compute changes on outdated content and corrupt or lose the user's edits.
• Neither a Job nor a UIJob fits: the post-save listener runs inside the save, so the clean-ups must finish before it returns. A UIJob would also freeze the UI again.
• I added a hint to the Javadoc to prefer a cancelable operation or a Job for long operations.

dialog.setCloseCancels(true) is unconditional, so close box/ESC now cancels for all existing busyCursorWhile(runnable) callers (e.g. SaveableHelper, CachedMarkerBuilder).

You're right. I removed that change from this PR. Every overload now behaves as on master: closing the dialog while the operation runs is ignored. New tests pin that down.

Dropping SWT.MAX to hide the close box relies on a Windows side effect.

Removed as well.

I'd suggest keeping this PR to the overload, and moving the close/ESC change to a separate PR.

Done. The PR now only adds the overload, plus a port of b95f72783a to the e4 progress service (otherwise the delayed dialog could open after a fast operation had already finished).

[laeubi]

I looked at the source, and I'm not yet convinced the use case needs a non-cancelable blocking dialog.

Typing conflicts:
jdt.ui#2415 already adds a DocumentChangeTracker that skips the clean-ups when the document was edited meanwhile, and it notes that programmatic events may edit the document even with blocked input.
So correctness comes from detecting the edit, not from blocking.
If the work is dropped and restarted on an edit, the user does not have to wait for a result that is outdated anyway.
This is also what eclipse-platform/eclipse.platform#2948 is about: user work blocked by something that could run in the background.

Cancelation:
ASTParser.createAST takes a monitor to "request cancellation", and the PR already forwards it.
Clean-ups only produce a change that is applied afterwards, so dropping a result needs no rollback.

BusyIndicator:
showWhile(Display, Runnable) runs in the calling thread as documented, and callers can drive the UI themselves.
There is also showWhile(Future) that processes events.
Replacing one blocking UI by another doesn't fix the cause, which is the missing cancelation.

run(true, false, …):
IRunnableContext leaves blocking unspecified for fork=true, and IProgressService.run says it might run asynchronously.
The "backward compatible" branch in ProgressManager.run is an implementation detail, so I would not build the argument for new API on it.

Could the save action instead run cancelable and restart on edit?

[fedejeanne]

I looked at the source, and I'm not yet convinced the use case needs a non-cancelable blocking dialog.

I disagree, see my comment in that PR for why.

Let's keep this PR about what it is: making the dialog non-cancelable. It's a small addition that complements the existing busyCursorWhile(IRunnableWithProgress). Callers whose work can't be interrupted currently show a Cancel button that does nothing; with the overload they can say so explicitly while keeping the immediate input blocking and the delayed dialog of busyCursorWhile.

[laeubi]

Let's keep this PR about what it is: making the dialog non-cancelable.

From that my concern still holds here, offering an API for a IDE blocking never cancel-able dialog seems a no-go for me. So from an API / platform perspective I see no valid reason so offer this, the risk is high that a user needs to kill the IDE to get out of the situation where before one can at least get rid of the dialog even though there is no guarantee the task itself is canceled.

So if pressing cancel do nothing this is a bug for me and nothing we should support or promote to a first class API.