Bugzilla – Full Text Bug Listing
| Summary: | Exception logged after cancelling Editor Selection dialog | ||
|---|---|---|---|
| Product: | [Eclipse Project] Platform | Reporter: | Noopur Gupta <noopur_gupta> |
| Component: | UI | Assignee: | Dani Megert <daniel_megert> |
| Status: | RESOLVED FIXED | QA Contact: | |
| Severity: | normal | ||
| Priority: | P3 | CC: | daniel_megert, loskutov, markus.kell.r |
| Version: | 4.6 | ||
| Target Milestone: | 4.6 M5 | ||
| Hardware: | All | ||
| OS: | All | ||
| See Also: | https://git.eclipse.org/r/65253 | ||
| Whiteboard: | |||
| Bug Depends on: | 90292 | ||
| Bug Blocks: | |||
|
Description
Noopur Gupta
Would be good to have a fix today since it's a new feature advertised in the N&N. New Gerrit change created: https://git.eclipse.org/r/65253 (In reply to Dani Megert from comment #1) > Would be good to have a fix today since it's a new feature advertised in the > N&N. (In reply to Eclipse Genie from comment #2) > New Gerrit change created: https://git.eclipse.org/r/65253 Dani, should I directly merge the fix or do you want to review it first? (In reply to Andrey Loskutov from comment #3) > (In reply to Dani Megert from comment #1) > > Would be good to have a fix today since it's a new feature advertised in the > > N&N. > > (In reply to Eclipse Genie from comment #2) > > New Gerrit change created: https://git.eclipse.org/r/65253 > > Dani, should I directly merge the fix or do you want to review it first? Just reviewed it. The fix is not good. (In reply to Dani Megert from comment #4) > (In reply to Andrey Loskutov from comment #3) > > (In reply to Dani Megert from comment #1) > > > Would be good to have a fix today since it's a new feature advertised in the > > > N&N. > > > > (In reply to Eclipse Genie from comment #2) > > > New Gerrit change created: https://git.eclipse.org/r/65253 > > > > Dani, should I directly merge the fix or do you want to review it first? > > Just reviewed it. The fix is not good. I've just pushed a second attempt. The problem is that IDE.open*() methods must return something, so we can't handle cancellation there. After discussion on the patch I don't see a way to fix this issue without changing IDE.open*() semantics, so returning this one back to the team pool. (In reply to Andrey Loskutov from comment #6) > After discussion on the patch I don't see a way to fix this issue without > changing IDE.open*() semantics, so returning this one back to the team pool. Argh, this is nasty! #openEditor is not the issue, but #getEditorDescriptor which does not allow 'null'. This makes a clean fix impossible and we will have to live with the OperationCanceledException and catch the cases we know about. We should catch the exception in the #openEditor methods and then return 'null' (no one does - or could do - anything useful with 'null'). This would shield most clients (e.g. catch in OpenAndLinkWithEditorHelper would not be needed), except for those who call #getEditorDescriptor, like e.g. the OpenWithMenu. And we need document the new exception in all #getEditorDescriptor API methods. Andrey, what do you think? Would you be able to provide a patch along these lines? (In reply to Dani Megert from comment #7) > Andrey, what do you think? Would you be able to provide a patch along these > lines? Yes, but not today anymore, may be for M6? (In reply to Andrey Loskutov from comment #8) > (In reply to Dani Megert from comment #7) > > Andrey, what do you think? Would you be able to provide a patch along these > > lines? > > Yes, but not today anymore, may be for M6? This is new functionality with new "API" throwing the exception. I've fixed it with http://git.eclipse.org/c/platform/eclipse.platform.ui.git/commit/?id=b5b43711fef1645eb643ddffc40b851b6325750f Filed bug 486665 to check all callers of #getEditorDescription. (In reply to Dani Megert from comment #9) Changes look good, but the whole feature is breaking APIs, see bug 486665 comment 1. |