-
Notifications
You must be signed in to change notification settings - Fork 225
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.
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
Implement semantic coloring #483
Conversation
You can try these changes at https://cadlplayground.z22.web.core.windows.net/prs/483/ |
916f3b9
to
1e5a0fa
Compare
dcbe2cf
to
4b3152f
Compare
4b3152f
to
8d90b39
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks great!!
import { copyFile } from "fs/promises"; | ||
|
||
await copyFile( | ||
"node_modules/@cadl-lang/compiler/dist/cadl.tmLanguage", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice to compute the projectRoot and resolve relative to that instead of depending on the cwd
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agree. We have this in a few other scripts/. I'll fix them all together in follow-up PR.
testColorization("semantic colorization", tokenizeSemantic); | ||
testColorization("tmlanguage", tokenizeTMLanguage); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So the semantic coloring basically changes how we resolve the color token but in the end it still create the same token.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It has a bit more detail available already, but that's collapsed in tokenizeSemantic to map to tokenizeTMLanguage. In practice, the bits of more detail (for example, this is a namespace, and this is an interface vs. just "type" don't actually change the colors in any VS Code theme I've tried. Eventually, we probably want to test this precision better, but my main goal here was parity with tmlanguage to make the playground pretty.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Arguably, there isn't much value yet in calling this code in VS Code or other editors with tmlanguage support, but I'm leaving it in so that we get some dogfooding coverage and people will see if there are problems with this in more places.
@daviwil, does emacs lsp-mode call the semantic coloring by chance? |
@nguerrera Just caught this in my notifications from a while back. Looks like |
Looks like it's also in PR for the LSP package I use called |
Fix #310. Colors are much nicer in the playground now.
Sorry for moving so many files around. I wanted to share test code for semantic coloring and tmlanguage and I ran into esm/commonjs hell so I moved tmlanguage to compiler and we copy it over to vscode.