[NETBEANS-791] Highlights for the top line of JavaDoc are done on every line. #550
Conversation
| @@ -55,21 +57,26 @@ | |||
| public class Highlighting extends AbstractHighlightsContainer implements TokenHierarchyListener { | |||
|
|
|||
| private static final Logger LOG = Logger.getLogger(Highlighting.class.getName()); | |||
|
|
|||
| private static final String WS = " \t\n"; // NOI18N | |||
| private static final String JAPANESE_PERIOD = "。"; // NOI18N | |||
There was a problem hiding this comment.
If the character does not fall into ANSI/ISO-8859-1 charset, better encode it as unicode escape in the java source.
There was a problem hiding this comment.
Instead of "。", use "\u3002" i.e. private static final String JAPANESE_PERIOD = "\u3002"; Is this OK?
| String period = null; | ||
| int indexOfPeriod = -1; | ||
| for (String p : PERIODS) { | ||
| int index = TokenUtilities.indexOf(seq.token().text(), p); |
There was a problem hiding this comment.
Wouldn't it be more appropriate if Javadoc lexer returned some specific tokenID japanese period char (and ordinary period char), since the period after the 1st sentence is a lexical element. But I fear that a new token ID could break existing lexer clients.
There was a problem hiding this comment.
I fear that a new token ID could break existing lexer clients.
Me, too...
There was a problem hiding this comment.
@sdedic So, I would like to avoid changing the JavadocLexer(java.lexer) because a new tokenID(e.g. JavadocTokenId.JAPANESE_DOT) is needed only for this feature at the moment. Even if someone adds the new tokenID, probably, we can notice that because I added unit tests to this feature.
However, if you prefer adding the tokenID, I'll try it :)
| if (seq.token().id() == JavadocTokenId.DOT) { | ||
| if (seq.moveNext()) { | ||
| if (isWhiteSpace(seq.token())) { | ||
| return new int [] { start, seq.offset()}; | ||
| } | ||
| seq.movePrevious(); | ||
| } | ||
| } else if (period != null && indexOfPeriod != -1) { | ||
| // NETBEANS-791 | ||
| int offset = TokenUtilities.indexOf(seq.token().text(), period) + 1; |
There was a problem hiding this comment.
Isn't TokenUtilities.indexOf(seq.token().text(), period) the same value as indexOfPeriod ?
There was a problem hiding this comment.
You are right. Thank you for catching it.
|
@sdedic Should I add the tokenID? or Can I merge this? |
|
I didn't study this in detail, but should JavadocTI.DOT be also used for
the Japanese dot? I.e. there would be a single token ID returned for both
(all) kinds of dots. In any case, I'd keep the test for highlighting, that
seems useful to ensure the functionality does not regress.
|
|
Possibly. But |
|
I'm not sure where impact area of that change is because I'm not an expert of Java area... |
|
My Proposal:
That way, someone other than me also may be able to work on it. I suppose that the person can do it even if the person doesn't know the Japanese language because I added test cases for it. |
|
@junichi11 OK, let's do gradual improvements. |
|
@sdedic Thank you! Will merge this. |
I added unit tests.
https://issues.apache.org/jira/browse/NETBEANS-791