Would be great if multiline strings were categorized differently

[basarat]

As it stands, the default classification looks horrible

I would be happy if there was just InMultiLineString (notice I don't say Literal) + it gets classified as MultilineString

Current:

    const enum EndOfLineState {
        Start = 0,
        InMultiLineCommentTrivia = 1,
        InSingleQuoteStringLiteral = 2,
        InDoubleQuoteStringLiteral = 3,
    }
    enum TokenClass {
        Punctuation = 0,
        Keyword = 1,
        Operator = 2,
        Comment = 3,
        Whitespace = 4,
        Identifier = 5,
        NumberLiteral = 6,
        StringLiteral = 7,
        RegExpLiteral = 8,
    }

Note: feel free to call it something other than MultiLineString but give us something.

[DanielRosenwasser]

1744 has a fix for this.

[basarat]

@DanielRosenwasser perhaps you can quickly tell me what classifyKeywordsInGenerics is supposed to do. Just a hint on if I should keep it true or false

[basarat]

@DanielRosenwasser also is there some public API endpoint to get the syntacticClassifier?

Nevermind, got the answer: https://github.com/Microsoft/TypeScript/issues/1477#issuecomment-66904683

[DanielRosenwasser]

@basarat if you'll see my change, it's been renamed to syntacticClassifierAbsent.

Before the syntactic classifier was a thing, the lexical classifier had to do a bunch of hacky sort of tricks to try to emulate correct behavior.

Basically, you don't want to accidentally classify number as a keyword as a type parameter when it might just be a plain identifier after a less-than token. The syntactic classifier will overwrite this to be the correct thing (i.e. an identifier) and so you'll get flashing. However, if there is no syntactic classifier, the lexical classifier should use its awful tricks again.

Actually, you're using web workers for Atom right? Are you using the other classifiers?

[basarat]

you're using web workers for Atom right?

Nope. A child process : https://github.com/TypeStrong/atom-typescript/blob/master/CONTRIBUTING.md#architecture . Node-webworker's need C/C++ toolchain on the client to install so I just went with a child process to make it easier to install.

Are you using the other classifiers?

nope. Just a lexical classifier : https://github.com/TypeStrong/atom-typescript/blob/master/lib/main/atom/typescriptGrammar.ts#L47, so I'll make it true. Thanks :)

[basarat]

Reason for not using webworkers : https://github.com/atom/atom-shell/issues/797

[DanielRosenwasser]

I just realized, you're asking for a multiline string classification. What if it's just a string literal classification but it's done correctly?

[basarat]

In a single line it gets identitified as an identifier :

In mutliple line it gets identified as code as there is no finalLexState for stuff with "`"

So for both of these I would like "just a string literal classification but it's done correctly".

[basarat]

@DanielRosenwasser fixed? Or wont-fix :heart:

[DanielRosenwasser]

If we do this I don't have to buy you chocolates for Valentine's Day. :heart:

(Will fix, but unfortunately just for 1.5: #2026. I guess I'll reopen just to track this as an issue.)

[DanielRosenwasser]

The fix is in master.