Bugzilla – Full Text Bug Listing
| Summary: | Improved support for Diagnostic to Issue conversion for code | ||
|---|---|---|---|
| Product: | [Modeling] TMF | Reporter: | Ed Merks <Ed.Merks> |
| Component: | Xtext | Assignee: | Project Inbox <tmf.xtext-inbox> |
| Status: | CLOSED FIXED | QA Contact: | |
| Severity: | enhancement | ||
| Priority: | P3 | CC: | jan, sebastian.zarnekow, sven.efftinge |
| Version: | 2.3.0 | Flags: | sebastian.zarnekow:
juno+
|
| Target Milestone: | M7 | ||
| Hardware: | PC | ||
| OS: | Windows 7 | ||
| Whiteboard: | |||
I don't that we can impose users to define their fixes like this in a backwards compatible manner without querying twice for the pure issue code and for the qualified issue code. Thus I'm inclined to close this as won't fix. Any opinions? Can you refactor the code so I can avoid copying the entire convertValidatorDiagnostic method?
Also, right now you don't set the code at all
if (diagnostic instanceof AbstractValidationDiagnostic) {
AbstractValidationDiagnostic diagnosticImpl = (AbstractValidationDiagnostic) diagnostic;
issue.setType(diagnosticImpl.getCheckType());
issue.setCode(diagnosticImpl.getIssueCode());
issue.setData(diagnosticImpl.getIssueData());
} else {
// default to FAST
issue.setType(CheckType.FAST);
}
so I'm not sure how setting it in the else case will break clients. Isn't it the case that without a code you can't even use it? And if clients have figured out how to use it, they must be setting the code some other way and will be able to continue doing that...
Setting the code for non AbstractValidationDiagnostic seems to be ok. (In reply to comment #3) > Setting the code for non AbstractValidationDiagnostic seems to be ok. +1 I misunderstood Ed's initial problem description. Ed, could you please provide a patch? I ran into problems creating a patch. I just want to add this one line here:
} else {
// default to FAST
issue.setType(CheckType.FAST);
issue.setCode(diagnostic.getSource() + "." + diagnostic.getCode());
}
I pushed the missing line. Should we close this ticket now? yes :-) Requested via bug 522520. -M. |
I subclassed the entire DiagnosticConverterImpl.convertValidatorDiagnostic method just to be able to set the Issue's code. I set it like this: issue.setCode(diagnostic.getSource() + "." + diagnostic.getCode()); The combination of diagnostic source and the integer code nicely identifies the issue. It's nice because then I can define quick fixes like this: public class XcoreQuickfixProvider extends DefaultQuickfixProvider { @Fix(EcoreValidator.DIAGNOSTIC_SOURCE + '.' + EcoreValidator.CONSISTENT_TYPE_CLASS_NOT_PERMITTED) public void convertToReference(final Issue issue, final IssueResolutionAcceptor acceptor) { See how it's still nicely symbolic and human readable? It seems there'd be no harm in using this same approach in the base Xtext framework...