Bugzilla – Full Text Bug Listing
| Summary: | Leverage new compiler cancellation support into BuildNotifier | ||||||
|---|---|---|---|---|---|---|---|
| Product: | [Eclipse Project] JDT | Reporter: | Jerome Lanneluc <jerome_lanneluc> | ||||
| Component: | Core | Assignee: | JDT-Core-Inbox <jdt-core-inbox> | ||||
| Status: | CLOSED WONTFIX | QA Contact: | |||||
| Severity: | normal | ||||||
| Priority: | P3 | ||||||
| Version: | 3.4 | ||||||
| Target Milestone: | --- | ||||||
| Hardware: | PC | ||||||
| OS: | Windows XP | ||||||
| Whiteboard: | stalebug | ||||||
| Attachments: |
|
||||||
|
Description
Jerome Lanneluc
Created attachment 97516 [details]
Proposed patch
Jerome - please take a look.
Why throw the AbortCompilation inside isCanceled() since this is done outside this call? Wouldn't it be simpler (e.g more readable) if isCanceled() would simply return whether the progress monitor was canceled? Also, the compiler progress doesn't seem to be passed to the compiler. See org.eclipse.jdt.internal.compiler.Compiler.Compiler(INameEnvironment, IErrorHandlingPolicy, CompilerOptions, ICompilerRequestor, IProblemFactory, PrintWriter, CompilationProgress) Oops - forgot to include the compiler call in the patch As for the isCanceled()... setCancelling(true) is called before the throw AbortCompilation. Would prefer to reuse the existing checkCancelWithinCompiler() than rely on the sender of isCanceled to throw the exception. Actually we do check whenever a type is requested from the NameEnvironment, in addition to accepting a compilation unit. I've been trying without the patch to find a point in the build when cancel is not responsive & it seems fine to me. Jerome - do you see a point in a full build when cancel waits a noticeable amount of time ? There is nothing more annoying than pressing cancel and wait for the cancel to really occur. So definitely yes. (In reply to comment #5) > As for the isCanceled()... setCancelling(true) is called before the throw > AbortCompilation. > > Would prefer to reuse the existing checkCancelWithinCompiler() than rely on the > sender of isCanceled to throw the exception. > What if I want to call isCanceled() to know if the build was canceled after the fact? Would I get an AbortCompilation instead? Or as your patch might imply, would it always return false? Discussed with Philippe - no rush to change our current behaviour since the IDE build is actually checking for cancel more frequently than this new API does. Will re-examine in the 3.5 timeframe. This bug hasn't had any activity in quite some time. Maybe the problem got resolved, was a duplicate of something else, or became less pressing for some reason - or maybe it's still relevant but just hasn't been looked at yet. If you have further information on the current state of the bug, please add it. The information can be, for example, that the problem still occurs, that you still want the feature, that more information is needed, or that the bug is (for whatever reason) no longer relevant. -- The automated Eclipse Genie. |