diffs: add starlark file extensions#1012
Conversation
If shiki ever grows Starlark support, this should be revised to use that starlark support, but Starlark is a strict subset of Python anyway. ref: https://bazel.build/concepts/build-ref https://buck2.build/docs/rule_authors/package_files/ https://buck2.build/docs/bxl/
|
@lf- is attempting to deploy a commit to the Pierre Computer Company Team on Vercel. A member of the Team first needs to authorize it. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ddee11dd01
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ddee11dd01
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| // appears in shiki. For now, Starlark is highlighted like the Python subset | ||
| // it is. | ||
| BUCK: 'python', | ||
| BUILD: 'python', |
There was a problem hiding this comment.
When a Bazel/Buck file is not at the repository root, this exact-filename entry is never used: parsed diffs keep directory prefixes (for example packages/diffs tests snapshot names like packages/diffs/src/index.ts), and getFiletypeFromFileName only checks EXTENSION_TO_FILE_FORMAT[fileName] before falling back to extensions. As a result getFiletypeFromFileName('src/BUILD') and getFiletypeFromFileName('foo/BUCK') still return text, so the common nested package files this change is trying to support won't be highlighted unless they use .bazel/.bzl.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Yeah this issue is due to my laziness around our file name detection (i.e. i need to probably extend this to properly test the filename), but probably shouldn't be apart of this PR.
| // appears in shiki. For now, Starlark is highlighted like the Python subset | ||
| // it is. | ||
| BUCK: 'python', | ||
| BUILD: 'python', |
There was a problem hiding this comment.
Yeah this issue is due to my laziness around our file name detection (i.e. i need to probably extend this to properly test the filename), but probably shouldn't be apart of this PR.
| BUILD: 'python', | ||
| MODULE: 'python', | ||
| PACKAGE: 'python', | ||
| WORKSPACE: 'python', |
There was a problem hiding this comment.
I'm a bit worried about including
BUILD, MODULE, and PACKAGE, since those feel pretty generic. The other ones definitely seem bazel related. What do you think about removing those three since there's a custom API you can use to extend for the more ambiguous ones?
Another possibility might be to say, what if we just remove all of these direct filenames, until i can improve that part of the system to properly track BUCK files in a sub directory?
There was a problem hiding this comment.
Some thoughts:
Bazel is super popular, so BUILD is most likely always going to be Bazel, and false positives are probably worth the benefit for these vs the possibility to have a false positive. Approximately one MODULE file appears in a Bazel repo; so it's lower value compared to BUILD.
PACKAGE is a buck specific thing and is, of the options, the least credible just given that Buck is a lot less popular than Bazel (c.f. the time i tried to add it to linguist lmao). This is the one I would put into consumers rather than the defaults; I guess the other thought I have is that the consumers could override the decision if they don't like the defaults for BUILD or similar.
It's definitely worth distinguishing direct filenames from extensions in the recognizer.
2 participants
If shiki ever grows Starlark support, this should be revised to use that starlark support, but Starlark is a strict subset of Python anyway.
ref for the file extensions:
https://bazel.build/concepts/build-ref
https://buck2.build/docs/rule_authors/package_files/
https://buck2.build/docs/bxl/
Description
Add support for highlighting Starlark files like those from Bazel/Buck2/Copybara.
Motivation & Context
I like my diffs for my build files to be readable.
Type of changes
first discussed with the dev team and they should be aware that this PR is
being opened
You must have first discussed with the dev team and they should be aware
that this PR is being opened
Checklist
contributing guidelines
moon run root:lint)moon run root:format)moonx diffs:test)How was AI used in generating this PR
wrote this with claude then realized it was all out of order and redid it by hand lmao
Related issues