matthiasblaesing commented on code in PR #8500:
URL: https://github.com/apache/netbeans/pull/8500#discussion_r2114355884
##########
java/java.editor/src/org/netbeans/modules/editor/java/JavaBracesMatcher.java:
##########
@@ -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;
Review Comment:
This change is unnessary and wrong as already indicated by @essien. Please
remove.
##########
java/java.editor/test/unit/src/org/netbeans/modules/editor/java/JavaBracesMatcherTest.java:
##########
@@ -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^>>()");
+ }
Review Comment:
This needs more test cases. I added some and hit a bug:
```java
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>^>()");
```
##########
java/java.editor/src/org/netbeans/modules/editor/java/JavaBracesMatcher.java:
##########
@@ -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. */
+ 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());
+ } 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();
+ 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.
Review Comment:
I assume that this refers to the fact, that after each opening brace a type
must be placed? I mean `List<TYPE`?
##########
java/java.editor/src/org/netbeans/modules/editor/java/JavaBracesMatcher.java:
##########
@@ -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. */
+ 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());
+ } 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();
Review Comment:
stray test code?
##########
java/java.editor/src/org/netbeans/modules/editor/java/JavaBracesMatcher.java:
##########
@@ -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. */
+ 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());
Review Comment:
I don't understand the idea here. Why is the `originOffset` used here?
Shouldn't `count` just count the number of unmatched braces?
##########
java/java.editor/src/org/netbeans/modules/editor/java/JavaBracesMatcher.java:
##########
@@ -98,13 +99,13 @@ public int[] findOrigin() throws BadLocationException,
InterruptedException {
context.getLimitOffset(),
PAIRS
);
-
+
Review Comment:
Please remove space at end of lines (see lines 102, 108, 150, 154, 160)
##########
java/java.editor/src/org/netbeans/modules/editor/java/JavaBracesMatcher.java:
##########
@@ -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. */
Review Comment:
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.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscr...@netbeans.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscr...@netbeans.apache.org
For additional commands, e-mail: notifications-h...@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
