Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.

Bug 349590

Summary:

Introduce quick fixes in hovers for Codan problems

Product:

[Tools] CDT

Reporter:

Tomasz Wesolowski <kosashi>

Component:

cdt-codan

Assignee:

CDT Codan Inbox <cdt-codan-inbox>

Status:

NEW ---

QA Contact:

Elena Laskavaia <elaskavaia.cdt>

Severity:

enhancement

Priority:

CC:

cdtdoug, elaskavaia.cdt, malaperle, yevshif

Version:

8.0

Target Milestone:

---

Hardware:

OS:

Windows 7

Whiteboard:

Attachments:

Description	Flags
Fix hardcoded limit for quick fix types	none

Description Tomasz Wesolowski

2011-06-16 12:14:00 EDT

Java's quick fixes are available in the hover balloon itself and I believe that's how most people use it, and most importantly- find it. CDT requires the user to manually twiddle with ctrl+1 or clicking weird places like the left margin. We should make the behaviour same as in JDT.

Without the quick fixes directly available in the hover, many CDT users won't ever find them or just will forget about them (hence I'm marking this a high priority for Codan).

Continued from Bug 309603 , see comments there

Comment 1 Tomasz Wesolowski

2011-07-05 15:18:32 EDT

If I'm correct, the quick fixes are not shown in hovers for the following reasons:

a) The method:
> org.eclipse.cdt.internal.ui.text.c.hover.ProblemHover.ProblemInfo.getCAnnotationFixes(ICAnnotation)
seems to deliberately ignore all annotations which are not SpellingAnnotations:
>82			if (!SpellingAnnotation.TYPE.equals(cAnnotation.getType()))
>83				return NO_PROPOSALS;
 Any idea why?

b) If we remove the above, nothing is read from the annotation, because the method:
> org.eclipse.cdt.internal.ui.text.correction.CCorrectionProcessor.getProcessorDescriptors(String, boolean)
which is supposed to return ContributedProcessorDescriptors, doesn't aparrently return anything codan-related. 

I'll continue on b) and I'd appreciate any comments on a) (is it safe to remove those lines?).

Comment 2 Elena Laskavaia

2011-07-05 21:25:53 EDT

ask on forum not sure people watching these prs

Comment 3 Tomasz Wesolowski

2011-07-07 08:03:08 EDT

About b) - What's the status on AbstractCodanCQuickFixProcessor ? It seems that it is never used and possibly incomplete, but looks like this is the extension point to contribute quick fixes to CDT

Right now Codan's fixes are contributed using MarkerResolutions, not Quick Fixes, right? I'm examining CDT code because it might be an issue with CDT itself that these marker resolutions are not shown on hover. Either this or we'll need to create a quickFixProvider too.

Comment 4 Tomasz Wesolowski

2011-07-07 08:03:37 EDT

s/quickFixProvider/quickFixProcessor/

Comment 5 Tomasz Wesolowski

2011-08-21 18:19:19 EDT

Created attachment 201882 [details]
Fix hardcoded limit for quick fix types

It's confirmed - using an IQuickFixProcessor makes fixes appear in the hover. Since using IQuickFixProcessors has several other advantages (see bug 355229), I strongly suggest we go for this.

This issue is going to fixed alongside with bug 355229, but also the hard-coded limitation mentioned in comment 1 part a) needs to be removed from CDT code.

Comment 6 Tomasz Wesolowski

2011-08-21 18:32:37 EDT

OK, let's not hurry. I've checked that in JDT and these two lines from CDT:

> if (!SpellingAnnotation.TYPE.equals(cAnnotation.getType()))
> 	return NO_PROPOSALS;

look like:

> if (!SpellingAnnotation.TYPE.equals(javaAnnotation.getType()) && !hasProblem(context.getASTRoot().getProblems(), location))
> 	return NO_PROPOSALS;

CDT doesn't have context.getASTRoot() but it has context.getTranslationUnit(). Can we *quickly* check whether the translation unit has any problems? This early check would make getting the hover a bit faster...

But OTOH, should we care? After all, it's just a mechanism to prohibit iterating over contributed QuickFixProcessors if there are no problems in the document. Definitely not an important bottleneck. So if there's no quick way to port it to CDT, I'd say we could just as well remove these lines (as in attachment 201882 [details]).