Skip to content

Added bracket highlighting for generic angle brackets #8500

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Added bracket highlighting for generic angle brackets #8500

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
108 changes: 100 additions & 8 deletions java/java.editor/src/org/netbeans/modules/editor/java/JavaBracesMatcher.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,11 +50,12 @@
*/
public final class JavaBracesMatcher implements BracesMatcher, BracesMatcherFactory, BracesMatcher.ContextLocator {

private static final char [] PAIRS = new char [] { '(', ')', '[', ']', '{', '}' }; //NOI18N
private static final char [] PAIRS = new char [] { '(', ')', '[', ']', '{', '}', '<', '>' }; //NOI18N
private static final JavaTokenId [] PAIR_TOKEN_IDS = new JavaTokenId [] {
JavaTokenId.LPAREN, JavaTokenId.RPAREN,
JavaTokenId.LBRACKET, JavaTokenId.RBRACKET,
JavaTokenId.LBRACE, JavaTokenId.RBRACE
JavaTokenId.LBRACE, JavaTokenId.RBRACE,
JavaTokenId.LT, JavaTokenId.GT
};

private final MatcherContext context;
Expand Down Expand Up @@ -98,13 +99,13 @@ public int[] findOrigin() throws BadLocationException, InterruptedException {
context.getLimitOffset(),
PAIRS
);

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please remove space at end of lines (see lines 102, 108, 150, 154, 160)

if (origin != null) {
originOffset = origin[0];
originChar = PAIRS[origin[1]];
matchingChar = PAIRS[origin[1] + origin[2]];
backward = origin[2] < 0;

// Filter out block and line comments
TokenHierarchy<Document> th = TokenHierarchy.get(context.getDocument());
sequences = getEmbeddedTokenSequences(th, originOffset, backward, JavaTokenId.language());
Expand Down Expand Up @@ -146,17 +147,17 @@ public int[] findMatches() throws InterruptedException, BadLocationException {
list = th.tokenSequenceList(seq.languagePath(), originOffset + 1, context.getDocument().getLength());
}
int counter = 0;

seq.move(originOffset);
if (seq.moveNext()) {
Token<?> token = seq.token();

if (token.id() == JavaTokenId.STRING_LITERAL ||
token.id() == JavaTokenId.MULTILINE_STRING_LITERAL) {

for(TokenSequenceIterator tsi = new TokenSequenceIterator(list, backward); tsi.hasMore(); ) {
TokenSequence<?> sq = tsi.getSequence();

if (sq.token().id() == JavaTokenId.STRING_LITERAL ||
sq.token().id() == JavaTokenId.MULTILINE_STRING_LITERAL) {

Expand Down Expand Up @@ -249,6 +250,97 @@ public int[] findMatches() throws InterruptedException, BadLocationException {

JavaTokenId originId = getTokenId(originChar);
JavaTokenId lookingForId = getTokenId(matchingChar);

/* The lexer tokenizes << and >> because they are bit shift
** operators. Special handler is required to bracematch angle
** brackets in generic types so that generic angles are
** differentiated from bitwise operators.*/

/* One idea was to modify the input TokenHierachy to split
** GTGT tokens into 2 separate GT and GT tokens. However,
** it seems like almost all token related classes are immutable
** or inaccessible due to private constructors. Leading me to
** write this. */
Comment on lines +259 to +263
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The TokenHierarchy/-sequence represents the lexing result and yes, for analysis they are read-only. This comment gives reasoning for the current implementation, but reads more like "note to self", then intentional for others.

if (originId == JavaTokenId.LT || originId == JavaTokenId.GT) {

for(TokenSequenceIterator tsi = new TokenSequenceIterator(list, backward); tsi.hasMore();) {
TokenSequence<?> sq = tsi.getSequence();

if (sq.token().id() == JavaTokenId.GTGT) {
if (originId == JavaTokenId.GT) {
if (originOffset != sq.offset()) {
counter += (originOffset - sq.offset());
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't understand the idea here. Why is the originOffset used here? Shouldn't count just count the number of unmatched braces?

} else {
counter++;
}
}

if (lookingForId == JavaTokenId.GT) {
if (counter <= 1) {
return new int [] {sq.offset() + counter, sq.offset() + 1 + counter};
} else {
counter -= 2;
}
}
}

if (sq.token().id() == JavaTokenId.GTGTGT) {
if (originId == JavaTokenId.GT) {
if (originOffset != sq.offset()) {
int test = sq.offset();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

stray test code?

counter += (originOffset - sq.offset());
} else {
counter++;
}
}

if (lookingForId == JavaTokenId.GT) {
if (counter <= 2) {
return new int [] {sq.offset() + counter, sq.offset() + 1 + counter};
} else {
counter -= 3;
}
}
}

if (sq.token().id() == JavaTokenId.LTLT) {
if (originId == JavaTokenId.LT) {
if (originOffset != sq.offset()) {
counter += 2;
} else {
counter++;
}
}

//Shouldn't happen, but added it anyways.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I assume that this refers to the fact, that after each opening brace a type must be placed? I mean List<TYPE?

if (lookingForId == JavaTokenId.LT) {
switch (counter) {
case 1:
return new int [] {sq.offset(), sq.offset() + 1 + counter};
case 0:
return new int [] {sq.offset() + 1, sq.offset() + 1 + counter};
default:
counter -= 2;
break;
}
}
}

if (originId == sq.token().id()) {
counter++;
}

if (lookingForId == sq.token().id()) {
if (counter == 0) {
return new int [] {sq.offset(), sq.offset() + sq.token().length()};
} else {
counter--;
}
}
}

return null;
}

for(TokenSequenceIterator tsi = new TokenSequenceIterator(list, backward); tsi.hasMore(); ) {
TokenSequence<?> sq = tsi.getSequence();
Expand Down Expand Up @@ -428,7 +520,7 @@ public static List<TokenSequence<?>> getEmbeddedTokenSequences(
for(int i = sequences.size() - 1; i >= 0; i--) {
TokenSequence<?> seq = sequences.get(i);
if (seq.language() == language) {
break;
break;
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wrong indentation

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change is unnessary and wrong as already indicated by @essien. Please remove.

} else {
sequences.remove(i);
}
Expand Down
7 changes: 7 additions & 0 deletions java/java.editor/test/unit/src/org/netbeans/modules/editor/java/JavaBracesMatcherTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,13 @@ public void testMultilineStringBrackets() throws Exception {
+ "^)\n"
+ "\"\"\"");
}

public void testAngleBrackets() throws Exception {
assertMatches2("^<test^>");
assertMatches2("Map^<Class<? extends AbstractStudent>, Map<CourseTime, List<? extends AbstractCourse>>^>");
assertMatches2("Map<Class<? extends AbstractStudent>, Map^<CourseTime, List<? extends AbstractCourse>^>>");
assertMatches2("Map x = new HashMap<String, List^<String^>>()");
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This needs more test cases. I added some and hit a bug:

        assertMatches2("^<test^>");
        assertMatches2("Map^<Class<? extends AbstractStudent>,  Map<CourseTime, List<? extends AbstractCourse>>^>");
        assertMatches2("Map<Class<? extends AbstractStudent>,  Map^<CourseTime, List<? extends AbstractCourse>^>>");
        assertMatches2("Map<Class<? extends AbstractStudent>,  Map<CourseTime, List^<? extends AbstractCourse^>>>");
        assertMatches2("Map<Class^<? extends AbstractStudent^>,  Map<CourseTime, List<? extends AbstractCourse>>>");
        assertMatches2("Map^<Map<CourseTime, List<? extends AbstractCourse>>, Class<? extends AbstractStudent>^>");
        assertMatches2("Map<Map^<CourseTime, List<? extends AbstractCourse>^>, Class<? extends AbstractStudent>>");
        assertMatches2("Map<Map<CourseTime, List^<? extends AbstractCourse^>>, Class<? extends AbstractStudent>>");
        assertMatches2("Map<Map<CourseTime, List<? extends AbstractCourse>>, Class^<? extends AbstractStudent^>>");
        assertMatches2("Map x = new HashMap<String, List^<String^>>()");
        assertMatches2("Map x = new HashMap^<String, List<String>^>()");

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

May I also add that <> should match in the below:

class LinkedList < T extends Object & Serializable > {
}

but <> should not match in the below:

int Object = 2;
int Serializable = 3;
boolean result = 1 < Object & Serializable > 4;

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, there needs to be some way to differentiate generic brackets from comparison/operator brackets.


//from CslTestBase:
protected void assertMatches2(String original) throws Exception {
Expand Down