Skip to content
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

TOML Cleanup and Improvements #4565

Merged
merged 3 commits into from
Sep 3, 2022
Merged

TOML Cleanup and Improvements #4565

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
15f0121
TOML Support Improvement and Cleanup
lkishalmi Aug 30, 2022
987ec7d
Report SyntaxErrors from TomlParser
lkishalmi Aug 30, 2022
2931102
A slightly better CharStream implementation
lkishalmi Sep 1, 2022
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
30 changes: 16 additions & 14 deletions ide/languages.toml/nbproject/project.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,15 @@
<specification-version>2.74</specification-version>
</run-dependency>
</dependency>
<dependency>
<code-name-base>org.netbeans.modules.csl.types</code-name-base>
<build-prerequisite/>
<compile-dependency/>
<run-dependency>
<release-version>1</release-version>
<specification-version>1.17</specification-version>
</run-dependency>
</dependency>
<dependency>
<code-name-base>org.netbeans.modules.editor.completion</code-name-base>
<build-prerequisite/>
Expand Down Expand Up @@ -104,35 +113,28 @@
</run-dependency>
</dependency>
<dependency>
<code-name-base>org.openide.awt</code-name-base>
<build-prerequisite/>
<compile-dependency/>
<run-dependency>
<specification-version>7.85</specification-version>
</run-dependency>
</dependency>
<dependency>
<code-name-base>org.openide.filesystems</code-name-base>
<code-name-base>org.netbeans.modules.parsing.api</code-name-base>
<build-prerequisite/>
<compile-dependency/>
<run-dependency>
<specification-version>9.29</specification-version>
<release-version>1</release-version>
<specification-version>9.24</specification-version>
</run-dependency>
</dependency>
<dependency>
<code-name-base>org.openide.loaders</code-name-base>
<code-name-base>org.openide.awt</code-name-base>
<build-prerequisite/>
<compile-dependency/>
<run-dependency>
<specification-version>7.87</specification-version>
<specification-version>7.85</specification-version>
</run-dependency>
</dependency>
<dependency>
<code-name-base>org.openide.nodes</code-name-base>
<code-name-base>org.openide.filesystems</code-name-base>
<build-prerequisite/>
<compile-dependency/>
<run-dependency>
<specification-version>7.62</specification-version>
<specification-version>9.29</specification-version>
</run-dependency>
</dependency>
<dependency>
Expand Down
59 changes: 38 additions & 21 deletions ide/languages.toml/src/org/netbeans/modules/languages/toml/LexerInputCharStream.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@
*/
package org.netbeans.modules.languages.toml;

import java.util.Stack;
import org.antlr.v4.runtime.CharStream;
import org.antlr.v4.runtime.misc.Interval;
import org.netbeans.spi.lexer.*;
Expand All @@ -29,21 +28,27 @@
*/
public class LexerInputCharStream implements CharStream {
private final LexerInput input;
private final Stack<Integer> markers = new Stack<>();

private int tokenMark = Integer.MAX_VALUE;
private int index = 0;

public LexerInputCharStream(LexerInput input) {
this.input = input;

}

@Override
public String getText(Interval intrvl) {
return input.readText(intrvl.a, intrvl.b).toString();
if (intrvl.a < tokenMark) {
throw new UnsupportedOperationException("Read before the current token start is not supported: " + intrvl.a + " < " + tokenMark);
}
int start = intrvl.a - tokenMark;
int end = intrvl.b - tokenMark + 1;
return String.valueOf(input.readText(start, end));
}

@Override
public void consume() {
input.read();
read();
}

@Override
Expand All @@ -55,51 +60,63 @@ public int LA(int count) {
int c = 0;
if (count > 0) {
for (int i = 0; i < count; i++) {
c = input.read();
c = read();
}
input.backup(count);
backup(count);
} else {
input.backup(count);
c = input.read();
backup(count);
c = read();
}
return c;
}

//Marks are for buffering in ANTLR4, we do not really need them
Copy link
Contributor

Choose a reason for hiding this comment

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

If I read the documentation correctly, we actually need buffering/seekability of the consumed stream (the documentation of CharStream declares this as required for lookahead), but the LexerInput already handles that - isn't it?

Reading further you actually buffer the whole file in memory, which kind of defeats the purpose of the CharStream implementation. getText can only look back to places, that are protected by a mark, so the buffer is limited (assuming limited lookahead and marking).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

LexerInput readText, readLength is scoped for the actual token being processed. CharStream getIndex and getText are work on stream level. Also getText is kind of an optional method. Fortunately it used in all Lexers. Just discovered to have a problem with the previos implementation on the Antlr lexer.

I'm tempted to look around the Lexing API and probably add a more ANTLR friendly interface. Will, see. I do not think that this would be the final implementation. It is kind of good enough for now.

Copy link
Contributor

Choose a reason for hiding this comment

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

Seeing, that ANTLR does not bother to implement its own interface (ANTLRInputStream) in an efficient way, I can't argue, that this implementation is inefficient, so ignore that.
For the tempation to change the lexer API to be "ANTLR" friedly: before going there, make sure you have a very good reason, at some point ANTLR will go away and we will retain the fallout.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Well, throwing out unmarked sections of StringBuilder could be implemented one day with mark supported.

Looked around the Lexing API yesterday. Accessing the underlying buffers is not as easy as I've thought.

@Override
public int mark() {
markers.push(index());
return markers.size() - 1;
return -1;
}

@Override
public void release(int marker) {
while (marker < markers.size()) {
markers.pop();
}
}

@Override
public int index() {
return input.readLengthEOF();
return index;
}

@Override
public void seek(int i) {
if (i < input.readLengthEOF()) {
input.backup(index() - i);
if (i < index()) {
backup(index() - i);
} else {
while (input.readLengthEOF() < i) {
if (input.read() == LexerInput.EOF) {
while (index() < i) {
if (read() == LexerInput.EOF) {
break;
}
}
}
}


private int read() {
int ret = input.read();
index += 1;
return ret;
}

private void backup(int count) {
index -= count;
input.backup(count);
}

public final void markToken() {
tokenMark = index;
}

@Override
public int size() {
return -1;
//throw new UnsupportedOperationException("Stream size is unknown.");
throw new UnsupportedOperationException("Stream size is unknown.");
}

@Override
Expand Down
141 changes: 0 additions & 141 deletions ide/languages.toml/src/org/netbeans/modules/languages/toml/TomlDataObject.java
View file
Open in desktop

This file was deleted.

Loading