Bugzilla – Full Text Bug Listing
| Summary: | ProgressManager should show MultiStatus with CANCEL and ERROR statuses as error. | ||
|---|---|---|---|
| Product: | [Eclipse Project] Platform | Reporter: | Malgorzata Janczarska <malgorzata.tomczyk> |
| Component: | UI | Assignee: | Platform-UI-Inbox <Platform-UI-Inbox> |
| Status: | CLOSED WONTFIX | QA Contact: | |
| Severity: | normal | ||
| Priority: | P3 | CC: | daniel_megert, john.arthorne, pwebster, remy.suen, Szymon.Brandys, tomasz.zarna |
| Version: | 3.5 | ||
| Target Milestone: | --- | ||
| Hardware: | All | ||
| OS: | All | ||
| Whiteboard: | |||
| Bug Depends on: | 363505 | ||
| Bug Blocks: | 263919 | ||
Paul, any comment on this? (In reply to comment #0) > > Instead MultiStatus should override matches(int) method to check if any of the > underlying statuses severity matches given mask and > org.eclipse.ui.internal.progress.ProgressManager.createChangeListener().new > JobChangeAdapter() {...}.done(IJobChangeEvent) should verify severity by > ".matches(IStatus.ERROR)" instead of "== IStatus.ERROR". This way an error is > reported even if parts of the running job where canceled. it seems reasonable, although IStatus is vague on how it would deal with multiple status' I don't see a problem there. PW I'm not sure about this change. If the user canceled the operation, will they really care if part of that operation prior to cancelation had failures? If I cancel it usually means I'll be trying the operation again later and I'll eventually find out the failure. It's hard to be sure the error is still relevant if the operation never completed. I can see cases in either direction though. I agree with John now, what it more, this is javadoc do IStatus#matches
> Returns whether the severity of this status matches the given
> severity mask.
I understand from it that matches reflects only the severity of this status and invocation of matches(ERROR) and getSeverity()==ERROR should return the same. I think changing matches to my request would be inconsistent with javadoc.
This change doesn't make sense than.
(In reply to comment #4) I agree that this change could affect many callers of this API in a breaking way. While looking at the issue I thought that 'CANCEL' as different from other severities should be perhaps ORed for multistatuses. So if children have CANCEL and other severities, the multistatus' severity would return CANCEL, but matches would return true for both #matches(CANCEL) and #matches(the most severe severity). Anyway, we have a fix on the CVS client side, so I'm also fine with closing it as WONTFIX. |
Currently if job finishes with a MultiStatus that is a mixture of ERROR and CANCEL severity statuses it is not reported as an error, because in this case MultiStatus.getSeverity() returns CANCEL. Instead MultiStatus should override matches(int) method to check if any of the underlying statuses severity matches given mask and org.eclipse.ui.internal.progress.ProgressManager.createChangeListener().new JobChangeAdapter() {...}.done(IJobChangeEvent) should verify severity by ".matches(IStatus.ERROR)" instead of "== IStatus.ERROR". This way an error is reported even if parts of the running job where canceled. For further justification see bug 263919 comment 31.