Skip to content

Fix SubMonitor misuse in UndoablePackageDeleteChange#2985

Merged
jjohnstn merged 1 commit into
eclipse-jdt:masterfrom
vogella:fix-undoable-package-delete-progress
May 14, 2026
Merged

Fix SubMonitor misuse in UndoablePackageDeleteChange#2985
jjohnstn merged 1 commit into
eclipse-jdt:masterfrom
vogella:fix-undoable-package-delete-progress

Conversation

@vogella
Copy link
Copy Markdown
Contributor

@vogella vogella commented May 14, 2026

The perform() method mixed pm.worked() with SubMonitor.convert() on the same monitor, and called SubMonitor.convert(pm, 1) inside a loop. Both patterns corrupt progress reporting: convert() resets the monitor's remaining work, so repeated calls inside a loop or after worked() discard already-reported progress.

The fix replaces beginTask/worked and the in-loop SubMonitor.convert() calls with a single SubMonitor.convert() up front, then split() for each unit of work. The loops are also simplified to enhanced for-each since the index is no longer needed.

Author checklist

The perform() method mixed pm.worked() with SubMonitor.convert() on the
same monitor, and called SubMonitor.convert(pm, 1) inside a loop. Both
patterns corrupt progress reporting: convert() resets the monitor's
remaining work, so repeated calls in a loop or after worked() discard
already-reported progress.

Replace pm.beginTask/worked/done and the in-loop SubMonitor.convert()
calls with a single SubMonitor.convert() up front, then split() for
each unit of work.
@vogella vogella force-pushed the fix-undoable-package-delete-progress branch from 78d710e to bd8fbb6 Compare May 14, 2026 04:48
@vogella vogella marked this pull request as ready for review May 14, 2026 04:48
Copy link
Copy Markdown
Contributor

@jjohnstn jjohnstn left a comment

Choose a reason for hiding this comment

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

Looks good.

@jjohnstn jjohnstn merged commit a104ded into eclipse-jdt:master May 14, 2026
13 checks passed
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