-
Notifications
You must be signed in to change notification settings - Fork 11
feat: add ember glimmer language parser (.gjs,.gts) extensions #154
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
base: main
Are you sure you want to change the base?
Conversation
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (11)
📝 WalkthroughWalkthroughAdds two new language packages: Changes
Sequence Diagram(s)(Skipped — changes are package additions with localized runtime resolution and tests; no multi-component control flow requiring sequence diagrams.) Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
0de3438 to
1e97ac4
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.
Actionable comments posted: 4
🤖 Fix all issues with AI agents
In `@packages/glimmer-javascript/README.md`:
- Around line 16-24: The example uses an invalid identifier name
"glimmer-javascript"; rename the import to a valid identifier (e.g.,
glimmerJavascript), update the object key passed to registerDynamicLanguage to
use that identifier (registerDynamicLanguage({ glimmerJavascript })), and ensure
the parse() call uses the matching language string/identifier
(parse('glimmerJavascript', `your code`)) so the import, registration, and parse
invocation all reference the same valid identifier and string.
In `@packages/glimmer-typescript/index.js`:
- Around line 21-30: The extensions list in module.exports currently registers
both "gjs" and "gts", which overlaps with glimmer-javascript; update the
extensions property in the module.exports object so this package only registers
TypeScript files (change extensions to include only "gts") to avoid ambiguous
language resolution; ensure you only modify the extensions array (referenced as
the extensions property on the exported object) and leave libraryPath,
languageSymbol, and expandoChar unchanged.
In `@packages/glimmer-typescript/README.md`:
- Around line 16-24: The example uses an invalid identifier
"glimmer-typescript"; change the import to a valid identifier (e.g.,
glimmerTypescript) and pass it into registerDynamicLanguage under the original
string key "glimmer-typescript" (use registerDynamicLanguage({
'glimmer-typescript': glimmerTypescript })); also update the parse call sample
to include non-empty example code (e.g., parse('glimmer-typescript', `your code
here`)). Ensure the unique symbols to change are the import line (currently
referencing glimmer-typescript), the registerDynamicLanguage call, and the parse
invocation.
- Around line 10-12: Update the README install command to reference the correct
NPM package name: replace the string "@tree-sitter/cli" with "tree-sitter-cli"
so the command becomes "pnpm install tree-sitter-cli --save-dev"; look for the
install line containing "@tree-sitter/cli" in README.md and change that token
only.
🧹 Nitpick comments (2)
packages/glimmer-javascript/nursery.js (1)
14-17: Incorrect regex pattern in test fixture.The regex
/^d+.d+.d+$/appears to intend matching semantic versions but is missing backslashes.dmatches literal 'd' and.matches any character.📝 Suggested fix
function isLatestVersion() { - const stableRegex = /^d+.d+.d+$/; + const stableRegex = /^\\d+\\.\\d+\\.\\d+$/; return !stableRegex.test(VERSION); }Note: Since this is test fixture code (not production logic), the incorrect regex doesn't affect actual functionality—just the realism of the test snippet.
packages/glimmer-typescript/nursery.js (1)
14-17: Same regex pattern issue as in glimmer-javascript.The regex
/^d+.d+.d+$/is missing backslashes for digit and dot escapes. Same fix as suggested for the JavaScript version.📝 Suggested fix
function isLatestVersion(): boolean { - const stableRegex = /^d+.d+.d+$/; + const stableRegex = /^\\d+\\.\\d+\\.\\d+$/; return !stableRegex.test(VERSION); }
| ```js | ||
| import glimmer-javascript from '@ast-grep/lang-glimmer-javascript' | ||
| import { registerDynamicLanguage, parse } from '@ast-grep/napi' | ||
|
|
||
| registerDynamicLanguage({ glimmer-javascript }) | ||
|
|
||
| const sg = parse('glimmer-javascript', `your code`) | ||
| sg.root().kind() | ||
| ``` |
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.
Invalid JavaScript identifier in usage example.
The identifier glimmer-javascript contains a hyphen, which is not valid JavaScript syntax. Hyphens are interpreted as the subtraction operator.
📝 Suggested fix
```js
-import glimmer-javascript from '@ast-grep/lang-glimmer-javascript'
+import glimmerJavascript from '@ast-grep/lang-glimmer-javascript'
import { registerDynamicLanguage, parse } from '@ast-grep/napi'
-registerDynamicLanguage({ glimmer-javascript })
+registerDynamicLanguage({ glimmerJavascript })
-const sg = parse('glimmer-javascript', `your code`)
+const sg = parse('glimmerJavascript', `your code`)
sg.root().kind()
```📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| ```js | |
| import glimmer-javascript from '@ast-grep/lang-glimmer-javascript' | |
| import { registerDynamicLanguage, parse } from '@ast-grep/napi' | |
| registerDynamicLanguage({ glimmer-javascript }) | |
| const sg = parse('glimmer-javascript', `your code`) | |
| sg.root().kind() | |
| ``` |
🤖 Prompt for AI Agents
In `@packages/glimmer-javascript/README.md` around lines 16 - 24, The example uses
an invalid identifier name "glimmer-javascript"; rename the import to a valid
identifier (e.g., glimmerJavascript), update the object key passed to
registerDynamicLanguage to use that identifier (registerDynamicLanguage({
glimmerJavascript })), and ensure the parse() call uses the matching language
string/identifier (parse('glimmerJavascript', `your code`)) so the import,
registration, and parse invocation all reference the same valid identifier and
string.
| # install the tree-sitter-cli if no prebuild is available | ||
| pnpm install @tree-sitter/cli --save-dev | ||
| ``` |
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.
🧩 Analysis chain
🏁 Script executed:
# Find and check the package.json in glimmer-typescript directory
find . -path "*/glimmer-typescript/package.json" -type fRepository: ast-grep/langs
Length of output: 100
🏁 Script executed:
# Once we find package.json, let's check its contents related to tree-sitter
if [ -f "packages/glimmer-typescript/package.json" ]; then
echo "=== package.json devDependencies ==="
cat packages/glimmer-typescript/package.json | jq '.devDependencies // empty'
fiRepository: ast-grep/langs
Length of output: 205
🏁 Script executed:
# Check the README to confirm the reference mentioned in the review
if [ -f "packages/glimmer-typescript/README.md" ]; then
echo "=== README.md lines 10-12 ==="
sed -n '10,12p' packages/glimmer-typescript/README.md
echo ""
echo "=== Broader context around tree-sitter ==="
grep -n "tree-sitter" packages/glimmer-typescript/README.md
fiRepository: ast-grep/langs
Length of output: 341
🌐 Web query:
@tree-sitter/cli npm package registry
💡 Result:
There is no published scoped package named @tree-sitter/cli. The Tree-sitter CLI is published on npm as tree-sitter-cli (latest shown v0.25.0). Install with npm i tree-sitter-cli or use the GitHub releases/binary. [1][2]
Sources:
- npm package page for tree-sitter-cli. [1]
- tree-sitter project / CLI source on GitHub. [2]
🌐 Web query:
tree-sitter-cli npm package registry
💡 Result:
Summary for the npm package "tree-sitter-cli":
- Package: tree-sitter-cli.
- Latest published npm version (as shown on npm): 0.22.5. Install: npm install tree-sitter-cli (or npm i [email protected]). [1]
- Repository / homepage: github.com/tree-sitter/tree-sitter (official Tree-sitter project). [1]
- Install alternatives: cargo install tree-sitter-cli (Rust crate) or download prebuilt binaries from GitHub Releases. [2]
- License: MIT. Common commands: tree-sitter generate, test, build, parse, etc.; docs on usage available in the Tree-sitter docs. [1][2]
Sources: npm package page for tree-sitter-cli [1]; official Tree-sitter docs (install/CLI usage) [2].
Update README.md line 11 to use correct package name tree-sitter-cli instead of @tree-sitter/cli.
The README references the non-existent scoped package @tree-sitter/cli, while package.json correctly uses tree-sitter-cli. The official Tree-sitter CLI package on NPM is published as tree-sitter-cli (unscoped).
Suggested fix for line 11:
- pnpm install `@tree-sitter/cli` --save-dev
+ pnpm install tree-sitter-cli --save-dev📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| # install the tree-sitter-cli if no prebuild is available | |
| pnpm install @tree-sitter/cli --save-dev | |
| ``` | |
| # install the tree-sitter-cli if no prebuild is available | |
| pnpm install tree-sitter-cli --save-dev |
🤖 Prompt for AI Agents
In `@packages/glimmer-typescript/README.md` around lines 10 - 12, Update the
README install command to reference the correct NPM package name: replace the
string "@tree-sitter/cli" with "tree-sitter-cli" so the command becomes "pnpm
install tree-sitter-cli --save-dev"; look for the install line containing
"@tree-sitter/cli" in README.md and change that token only.
HerringtonDarkholme
left a comment
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.
Can you add README and fix linting? Thanks
They're Ember.js component authoring and templating format.
Summary by CodeRabbit
New Features
Tests
Chores
✏️ Tip: You can customize this high-level summary in your review settings.