This Bugzilla instance is a read-only archive of historic NetBeans bug reports. To report a bug in NetBeans please follow the project's instructions for reporting issues.
Created attachment 164739 [details] multiple error description ranges support patch Need a way to support multiple ranges for the errors in the editor. Patch is attached. Required for CND team spec version incrementation is not included in the patch
please consider the patch attached
One way to proceed would be to polish the patch and submit for API review. First, I was thinking if converting the factory to use the builder pattern wouldn't be better, but now I incline to keep that separate.
Should I send a request to apireviews@netbeans.org ? I like the idea to use builder is good but I do think it is time to change the API. Current changes are extending existing API and do not require any changes from the client side.
(In reply to Maria Tishkova from comment #3) > Should I send a request to apireviews@netbeans.org ? http://wiki.netbeans.org/APIReviews
SD01: I'd recommend to check that starts[i] < ends[i] for each index (lines ~300, ~387), HintsController or PositionBounds do not check any constraints. Perhaps also that starts.length == ends.length. SD02: Is there a corresponding apichanges.xml entry ? @since tags in added APIs should match the entry. Increase spec version of module so your code may demand the version with updated APIs. SD03: Pls. remove commented out code. SD04: Pls. update javadoc of the added methods to describe the starts and ends parameters.
(In reply to Svata Dedic from comment #5) > SD01: I'd recommend to check that starts[i] < ends[i] for each index (lines > ~300, ~387), HintsController or PositionBounds do not check any constraints. > Perhaps also that starts.length == ends.length. > > SD02: Is there a corresponding apichanges.xml entry ? @since tags in added > APIs should match the entry. Increase spec version of module so your code > may demand the version with updated APIs. > > SD03: Pls. remove commented out code. > > SD04: Pls. update javadoc of the added methods to describe the starts and > ends parameters. Please find new patch attached (also please take a look at bz#271069)
Created attachment 164777 [details] multiple error description ranges support patch _ v2
Re SD01, I guess it might be better to check all the offsets provided, and to fail (as the other int/offset-based methods do, if I read correctly). I.e. preconditions check like: --- Parameters.notNull("starts", starts); Parameters.notNull("ends", ends); if (starts.length == 0) throw new IllegalArgumentException("No offsets provided.");//NOI18N if (ends.length != starts.length) throw IllegalArgumentException("starts.length != ends.length (" + starts.length + " != " + ends.length + ")");//NOI18N for (int i = 0; i < starts.length; i++) { if (ends[i] < starts[i]) throw new IndexOutOfBoundsException("ends[i] < starts[i] (" + ends[i] + " < " + starts + "; i=" + i + ")"); } --- JL01: could there please be some tests? At least something like: --- public void testMultiLineErrors() throws Exception { ErrorDescription ed1 = ErrorDescriptionFactory.createErrorDescription(null, Severity.ERROR, null, "1", null, ErrorDescriptionFactory.lazyListForFixes(Collections.<Fix>emptyList()), file, new int[] {33 - 30, 55 - 30 - 1}, new int[] {40 - 30, 58 - 30 -1}); ErrorDescription ed2 = ErrorDescriptionFactory.createErrorDescription(null, Severity.WARNING, null, "1", null, ErrorDescriptionFactory.lazyListForFixes(Collections.<Fix>emptyList()), file, new int[] {55 - 30 - 1, 69 - 30 - 2}, new int[] {58 - 30 - 1, 75 - 30 - 2}); List<ErrorDescription> errors = Arrays.asList(ed1, ed2); final OffsetsBag bag = AnnotationHolder.computeHighlights(doc, errors); assertHighlights("",bag, new int[] {33 - 30, 40 - 30, 55 - 30 - 1, 58 - 30 - 1, 69 - 30 -2, 75 - 30 - 2}, new AttributeSet[] {AnnotationHolder.getColoring(ERROR, doc), AnnotationHolder.getColoring(ERROR, doc), AnnotationHolder.getColoring(WARNING, doc)}); } --- Thanks!
(In reply to Jan Lahoda from comment #8) > Re SD01, I guess it might be better to check all the offsets provided, and > to fail (as the other int/offset-based methods do, if I read correctly). > I.e. preconditions check like: > --- > Parameters.notNull("starts", starts); > Parameters.notNull("ends", ends); > if (starts.length == 0) throw new IllegalArgumentException("No > offsets provided.");//NOI18N > if (ends.length != starts.length) throw > IllegalArgumentException("starts.length != ends.length (" + starts.length + > " != " + ends.length + ")");//NOI18N > > for (int i = 0; i < starts.length; i++) { > if (ends[i] < starts[i]) throw new > IndexOutOfBoundsException("ends[i] < starts[i] (" + ends[i] + " < " + starts > + "; i=" + i + ")"); > } agree, will change the code > --- > > JL01: could there please be some tests? At least something like: > --- > public void testMultiLineErrors() throws Exception { > ErrorDescription ed1 = > ErrorDescriptionFactory.createErrorDescription(null, Severity.ERROR, null, > "1", null, > ErrorDescriptionFactory.lazyListForFixes(Collections.<Fix>emptyList()), > file, new int[] {33 - 30, 55 - 30 - 1}, new int[] {40 - 30, 58 - 30 -1}); > ErrorDescription ed2 = > ErrorDescriptionFactory.createErrorDescription(null, Severity.WARNING, null, > "1", null, > ErrorDescriptionFactory.lazyListForFixes(Collections.<Fix>emptyList()), > file, new int[] {55 - 30 - 1, 69 - 30 - 2}, new int[] {58 - 30 - 1, 75 - 30 > - 2}); > > List<ErrorDescription> errors = Arrays.asList(ed1, ed2); > final OffsetsBag bag = AnnotationHolder.computeHighlights(doc, > errors); > > assertHighlights("",bag, new int[] {33 - 30, 40 - 30, 55 - 30 - 1, > 58 - 30 - 1, 69 - 30 -2, 75 - 30 - 2}, new AttributeSet[] > {AnnotationHolder.getColoring(ERROR, doc), > AnnotationHolder.getColoring(ERROR, doc), > AnnotationHolder.getColoring(WARNING, doc)}); > } > --- > Can you tell me where can I add test to? Which module contains AnnotationHolder tests? > Thanks!
(In reply to Maria Tishkova from comment #9) > Can you tell me where can I add test to? Which module contains > AnnotationHolder tests? http://hg.netbeans.org/cnd-main/file/800e881f15f1/spi.editor.hints/test/unit/src/org/netbeans/modules/editor/hints/AnnotationHolderTest.java
fixed in cnd-main changeset: 304367:cafbf016499a user: Maria Dalmatova <mromashova@netbeans.org> date: Mon Jul 24 10:52:22 2017 +0300 summary: fixed bz#271070 - Support multiple ranges for ErrorDescription
Integrated into 'main-silver', will be available in build *201707250001* on http://bits.netbeans.org/dev/nightly/ (upload may still be in progress) Changeset: http://hg.netbeans.org/main-silver/rev/cafbf016499a User: Maria Dalmatova <mromashova@netbeans.org> Log: fixed bz#271070 - Support multiple ranges for ErrorDescription