added some properties and methods #4525

RobEin · 2024-02-04T21:59:13Z

Lexer._modeStack
Lexer.mode()
Token.source
Recognizer.getSymbolicNames()
Recognizer.getErrorListenerDispatch()

TypeScript missing critical class definition exports

Lexer._modeStack Lexer.mode() Token.source Recognizer.getSymbolicNames() Recognizer.getErrorListenerDispatch() Signed-off-by: Robert Einhorn <[email protected]>

RobEin · 2024-02-04T23:07:02Z

Unfortunately, I cannot comment on this:
failing at "build (macos-11, php) " - Install dependecies ...

Thanks,
Robert

ericvergnaud · 2024-02-05T09:36:29Z

runtime/JavaScript/src/antlr4/Lexer.d.ts


    constructor(input: CharStream);
    reset(): void;
    nextToken(): Token;
    skip(): void;
    more(): void;
    more(m: number): void;
+    mode(m: number): void;


I guess the intent with the 2nd more was to be mode. Can you merge these ?

The 2 more are already in the current Lexer.d.ts.

There should only be 1, it's a bug in the existing file.

ericvergnaud · 2024-02-05T09:37:15Z

runtime/JavaScript/src/antlr4/Recognizer.d.ts

@@ -6,4 +6,6 @@ export declare class Recognizer<TSymbol> {

    removeErrorListeners(): void;
    addErrorListener(listener: ErrorListener<TSymbol>): void;
+    getSymbolicNames(): string[];


Maybe also add getLiteralNames ?

I would add all JavaScript attributes, but you wrote: "I only export what is strictly required"

I believe in encapsulation. I'm ok to provide data via methods, not so much direct read/write access to fields that may change anytime... Adding getLiteralNames is perfectly fine.

ericvergnaud · 2024-02-05T09:39:39Z

runtime/JavaScript/src/antlr4/Token.d.ts

@@ -7,6 +9,7 @@ export declare class Token {
    static DEFAULT_CHANNEL: number;
    static HIDDEN_CHANNEL: number;

+    source: [ TokenSource, InputStream ];


I'd rather you add getTokenSource and getInputStream. source being a tuple is an internal detail

I have a method where I refer to the source property and a getTokenSource() would look strange in my opinion:.

private getCommonTokenByToken(oldToken: Token): CommonToken { const cToken = new CommonToken(oldToken.source, oldToken.type, oldToken.channel, oldToken.start, oldToken.stop); cToken.tokenIndex = oldToken.tokenIndex; cToken.line = oldToken.line; cToken.column = oldToken.column; cToken.text = oldToken.text; return cToken; }

Sorry, maybe what I wrote was a little misunderstood.
Perhaps it would have been more understandable if I had referred to a public field instead of a property.
However, VS Code marks public fields as properties.

I'm really not keen to expose such a poorly designed construct. The d.ts file already has getInputStream, so you only need to add getTokenSource.

ericvergnaud

See my comments

ericvergnaud · 2024-02-05T09:42:38Z

runtime/JavaScript/src/antlr4/Lexer.d.ts

@@ -16,13 +16,16 @@ export declare class Lexer extends Recognizer<number> {
    _tokenStartLine: number;
    _tokenStartColumn: number;
    _type: number;
+    _modeStack: number[];
+    _mode: number;


maybe rather add getMode an getModeStack ?

The public fields would be better for me because I have also used them in my various portings:
Java port
JavaScript port
Pyton port

Well these fields are clearly private... (I guess there was a bit of laziness with the first implementation 30 years ago)

ericvergnaud · 2024-02-06T10:10:25Z

As a heads-up, we've started working on antlr5 which will come with a cleaned-up API (and speed in JS and Python).
It's early days, but since you're dealing with rather low-level details, it could be useful to get involved at some point such that we understand your use cases and make sure they are supported ?

RobEin · 2024-02-06T16:56:00Z

The new ANTLR5 looks very exciting.
If I can help in any way, I will gladly do so.

RobEin

I apologize for not showing up until now, but I've been very busy (including other ANTLR4 stuff).
I have implemented the changes you suggested.
I hope you thought so.

Instead of the getMode() and setMode() methods, wouldn't a mode property be better?

ericvergnaud · 2024-03-05T09:22:02Z

Instead of the getMode() and setMode() methods, wouldn't a mode property be better?

It would, but you can't make it a property in the d.ts without doing the same in the .js, which would break backwards compatibility

ericvergnaud · 2024-03-05T09:17:32Z

runtime/JavaScript/src/antlr4/Recognizer.d.ts

@@ -6,4 +6,7 @@ export declare class Recognizer<TSymbol> {

    removeErrorListeners(): void;
    addErrorListener(listener: ErrorListener<TSymbol>): void;
+    getErrorListenerDispatch(): ErrorListener<TSymbol>;


Can we rename this to say getRootErrorListener() (open to suggestions). Appreciate it's an instance of ErrorListenerDispatch but that's a terrible name...

The name mode is really not a good property name, because there is already a mode() method in the JavaScript Lexer.
Maybe a property for JavaScript/TypeScript called lexerMode or tokenizerMode?
And the mode() method would be depreciated in the JavaScript Lexer.

I don't like the name getErrorListenerDispatch either.

Actually we can't change that name without breaking backwards compatibility in JS...

Indeed, unfortunately the getErrorListenerDispatch method should not be renamed.
I think an independent TypeScript runtime (no *.js, no *.d.ts) would be ideal for this and other more modern designs.
Maybe in ANTLR5?

I remove my setMode method because there is already a mode method that does the same thing.
However, a setMode/getMode pair would have been more elegant than a mode/getMode pair.

added some properties and methods

ee4c88f

Lexer._modeStack Lexer.mode() Token.source Recognizer.getSymbolicNames() Recognizer.getErrorListenerDispatch() Signed-off-by: Robert Einhorn <[email protected]>

ericvergnaud reviewed Feb 5, 2024

View reviewed changes

ericvergnaud requested changes Feb 5, 2024

View reviewed changes

ericvergnaud reviewed Feb 5, 2024

View reviewed changes

RobEin added 2 commits March 5, 2024 05:45

Merge branch 'antlr:dev' into dev

9193b09

added some properties and methods (2)

29a3d4e

RobEin commented Mar 5, 2024

View reviewed changes

ericvergnaud requested changes Mar 5, 2024

View reviewed changes

Merge branch 'antlr:dev' into dev

dbf7983

RobEin deleted the branch antlr:dev March 17, 2024 23:37

RobEin closed this Mar 17, 2024

RobEin deleted the dev branch March 17, 2024 23:37

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

added some properties and methods #4525

added some properties and methods #4525

RobEin commented Feb 4, 2024 •

edited

Loading

RobEin commented Feb 4, 2024

ericvergnaud Feb 5, 2024

RobEin Feb 6, 2024

ericvergnaud Feb 6, 2024

ericvergnaud Feb 5, 2024

RobEin Feb 6, 2024

ericvergnaud Feb 6, 2024

ericvergnaud Feb 5, 2024

RobEin Feb 6, 2024 •

edited

Loading

RobEin Feb 6, 2024

ericvergnaud Feb 6, 2024

ericvergnaud left a comment

ericvergnaud Feb 5, 2024

RobEin Feb 6, 2024 •

edited

Loading

ericvergnaud Feb 6, 2024

ericvergnaud commented Feb 6, 2024

RobEin commented Feb 6, 2024

RobEin left a comment •

edited

Loading

ericvergnaud commented Mar 5, 2024 •

edited

Loading

ericvergnaud Mar 5, 2024

RobEin Mar 5, 2024 •

edited

Loading

ericvergnaud Mar 12, 2024

RobEin Mar 13, 2024 •

edited

Loading

added some properties and methods #4525

added some properties and methods #4525

Conversation

RobEin commented Feb 4, 2024 • edited Loading

RobEin commented Feb 4, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

RobEin Feb 6, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ericvergnaud left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

RobEin Feb 6, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ericvergnaud commented Feb 6, 2024

RobEin commented Feb 6, 2024

RobEin left a comment • edited Loading

Choose a reason for hiding this comment

ericvergnaud commented Mar 5, 2024 • edited Loading

Choose a reason for hiding this comment

RobEin Mar 5, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

RobEin Mar 13, 2024 • edited Loading

Choose a reason for hiding this comment

RobEin commented Feb 4, 2024 •

edited

Loading

RobEin Feb 6, 2024 •

edited

Loading

RobEin Feb 6, 2024 •

edited

Loading

RobEin left a comment •

edited

Loading

ericvergnaud commented Mar 5, 2024 •

edited

Loading

RobEin Mar 5, 2024 •

edited

Loading

RobEin Mar 13, 2024 •

edited

Loading