georgewfraser / vscode-tree-sitter

Accurate syntax coloring for VSCode using tree-sitter
MIT License
178 stars 25 forks source link

Moved coloring code parsing to their own files / classes #17

Closed linus-skold closed 5 years ago

linus-skold commented 5 years ago

Moved the coloring into their own files, this should make it easier to manage and maintain the different languages, and avoid creating a blob file that just contains everything.

It's still simple and easy to add a new language.

georgewfraser commented 5 years ago

I appreciate your eagerness to help. To me, this is not an improvement---every function is now embedded in a class; more files means more imports. One file with 4 functions in it seems pretty good to me.

linus-skold commented 5 years ago

Alright, but I have a question, maybe you can help me understand. Is it bad that it's embedded in a class? I was questioning it myself earlier, but I can't see a reason why it would be worse than a free floating function.

georgewfraser commented 5 years ago

Functions are "dumber" in a good way, they usually don't hold internal state, they can only do one thing. My view is that it's always best to use the simplest possible construct. I think the fact that Typescript allows free-floating functions is strength of the language.

georgewfraser commented 5 years ago

I suppose each of these functions is actually using a bunch of internal functions, which share various local variables, and it would arguably be simpler and clearer to refactor them into classes. Like instead of doing:

function colorLang(x, visibleRanges) {
    const colors = []
    function scan(x) { ... }
    scan(x)
    return colors
}

we could do:

class ColorLang {
    private colors
    private visibleRanges
    private scan(x) { ... }
    color(x, visibleRanges) {
        this.colors = []
        this.visibleRanges = visibleRanges
        this.scan(x)
        return this.colors
    }
}

The advantage here is that we've explicitly specified what state is shared between methods. I feel like it could be argued both ways though.