Use tokenrange to construct lex pres data #1034
Conversation
matozoid
changed the title
| @@ -65,6 +67,7 @@ | |||
|
|
|||
| /** | |||
| * Parse the code and setup the LexicalPreservingPrinter. | |||
| * @deprecated just use the other constructor. | |||
There was a problem hiding this comment.
@ftomassetti - I can't see a good reason to have this constructor, is it okay to deprecate?
There was a problem hiding this comment.
Well, until now it was the easiest way to setup LPP. The user would call this method and get the AST with LPP enabled. How the user should get the same result? Should he parse the code and then call new LexicalPreservingPrinter(parseResult)?
There was a problem hiding this comment.
Oh, ok, with the new changes the Node is enough to start LPP, right
There was a problem hiding this comment.
Uhuh, we might even refactor this code into the node constructors ("I know my start token, my end token, and the start and end token of my children, so I can deduce which tokens are mine")
| @@ -201,7 +205,7 @@ public void process(Node node) { | |||
|
|
|||
| // We go over tokens and find to which nodes belong. Note that we start from the most specific nodes | |||
| // and we move up to more general nodes | |||
| for (JavaToken token : documentTokens) { | |||
| for (JavaToken token : root.getTokenRange().get()) { | |||
There was a problem hiding this comment.
@ftomassetti Well, having token ranges didn't help much to reduce the code here :-)
There was a problem hiding this comment.
Well, now we do not need a ParseResult anymore, just the AST is enough to enable LPP, right?
There was a problem hiding this comment.
Yes, that is an advantage.
…kenrange # Conflicts: # javaparser-core/src/main/java/com/github/javaparser/printer/lexicalpreservation/LexicalPreservingPrinter.java
| public static TokenTextElement newLine() { | ||
| return new TokenTextElement(TokenTypes.eolTokenKind(), EOL); | ||
| } | ||
| private final JavaToken token; |
There was a problem hiding this comment.
I'll directly wrap a JavaToken here, so the lex pres code gets a little more connected to the rest.
There was a problem hiding this comment.
"Ok, that's great!" or "ok, if you insist..." ?
There was a problem hiding this comment.
The first one: it is great!
| return "TokenTextElement(" + tokenKind + | ||
| ") {" + text + '}'; | ||
| return "TokenTextElement(" + token.getKind() + | ||
| ") {" + getText() + '}'; |
There was a problem hiding this comment.
@ftomassetti I could use JavaToken's toString here - are you very attached to this representation?
There was a problem hiding this comment.
I am ok with changing it, if it shows the token kind
| import java.util.Optional; | ||
|
|
||
| import static com.github.javaparser.utils.Utils.assertNotNull; | ||
|
|
||
| /** | ||
| * The range of tokens covered by this node. | ||
| */ | ||
| public class TokenRange { | ||
| public class TokenRange implements Iterable<JavaToken> { |
| @@ -65,6 +67,7 @@ | |||
|
|
|||
| /** | |||
| * Parse the code and setup the LexicalPreservingPrinter. | |||
| * @deprecated just use the other constructor. | |||
There was a problem hiding this comment.
Oh, ok, with the new changes the Node is enough to start LPP, right
| public static TokenTextElement newLine() { | ||
| return new TokenTextElement(TokenTypes.eolTokenKind(), EOL); | ||
| } | ||
| private final JavaToken token; |
| return "TokenTextElement(" + tokenKind + | ||
| ") {" + text + '}'; | ||
| return "TokenTextElement(" + token.getKind() + | ||
| ") {" + getText() + '}'; |
There was a problem hiding this comment.
I am ok with changing it, if it shows the token kind
matozoid
changed the title
No description provided.