Implement multi-language support - #130
Conversation
|
wondering about the common lexical scope and an option to have a lexer per entry rule |
78988e1 to
9f6d652
Compare
There was a problem hiding this comment.
⚠️ Performance Alert ⚠️
Possible performance regression was detected for benchmark.
Benchmark result of this commit is worse than the previous benchmark result exceeding threshold 1.50.
| Benchmark suite | Current: 40d792c | Previous: d803417 | Ratio |
|---|---|---|---|
BenchmarkWorkspaceCycle (typefox.dev/fastbelt/examples/statemachine) - MB/s |
13.16 MB/s |
5.21 MB/s |
2.53 |
This comment was automatically generated by workflow using github-action-benchmark.
|
@cdietrich Thanks for the input. I plan to finish implementing this once #127 lands, since it refactors a lot of the lexer infrastructure. |
ssmifi
left a comment
There was a problem hiding this comment.
Looks solid, thanks. I have a couple of questions to get a better understanding before merging and some documentation seems to be stale.
| @@ -55,7 +55,7 @@ func runGenerateCLI(opts generateOptions) error { | |||
| } | |||
|
|
|||
| sc := grammar.CreateServices() | |||
There was a problem hiding this comment.
Nice separation.
Question: The following lines are conceptionally doing for one file what parseAndMerge and reportDiagnostic do generically now. I was wondering if we can de-duplicate some code here and use come common helper?
|
|
||
| type DocumentMatcher func(uri URI) bool | ||
|
|
||
| // DocumentFilter matches a document by LSP language id and/or a glob over the |
| "typefox.dev/fastbelt/util/service" | ||
| ) | ||
|
|
||
| type DocumentMatcher func(uri URI) bool |
There was a problem hiding this comment.
As this is an exported symbol, it should probably get a doc, even if self-explanatory.
| service.Put[core.LanguageSelector]( | ||
| sc, | ||
| core.NewDefaultLanguageSelector(sc, | ||
| core.NewDocumentSelectorWithPatterns("arithmetics", "**/*.arithmetics"), |
There was a problem hiding this comment.
Puh, a document selector can also influence the folders up the file hierarchy. Also here you seem to be forced to use „**/“… this is broader than an array of file extensions. Not sure if we should go for this. You always need to prepend „**/*“ to your „.ext“.
Is this case-insensitive btw?
There was a problem hiding this comment.
Also here you seem to be forced to use „**/“… this is broader than an array of file extensions.
You're not forced to do it. Adopters can implement their own FileExtensionDocumentSelector if they want to. If absolutely required, we can also implement this in the framework, but I'm not really convinced that this is better than the glob pattern.
I'm not happy how this works in Langium, because the idea to use file extensions as the defining identifying feature of languages has a large negative impact on the API design.
Is this case-insensitive btw?
No, but neither is Langium's file extension comparison AFAIK.
There was a problem hiding this comment.
OK, I am not really for or against it. It just feels unusual.
But it serves the same purpose as before and goes even beyond that, which might be a good sign to keep it.
| // documents. | ||
| type Language struct { | ||
| Entry string | ||
| LanguageID string |
There was a problem hiding this comment.
Or combine it within the syntax of the entry rule:
entry XRule on mode YMode: ...
Omitting on mode YMode falls back to the default mode.
| @@ -0,0 +1,15 @@ | |||
| grammar MultilangModel | |||
There was a problem hiding this comment.
I thought this PR is also about multi file support.
But yes, this might be a smaller step towards this direction.
| DocStateLocalSymbols // 0x0010 | ||
| // DocStateLinked marks that cross-references were linked. | ||
| DocStateLinked // 0x0010 | ||
| DocStateLinked // 0x0020 |
There was a problem hiding this comment.
Why the comment "//0x0001" ...? Who needs it?
The only reason in my eyes: to not forget the flag nature of this constant group.
Alternative: Rename the constants to reflect the flag nature.
- DocStateFlagLinked
- DocStateBitMaskLinked
Closes #96
Does as the title says, similar to how it is outlined in the issue:
fastbelt/cmdpackage adopters can use to write their own CLIs to generate Fastbelt code.LanguageSelectorservice that is used to determine during runtime what language a URI/document belongs to.multilangexample that shows that this is working as expected (including parser, lexer and completion tests).FYI, while this has a
Pluginsfield in the API, plugins aren't actually supported in this PR. This will need a follow-up.