✨ Bash #46
✨ Bash #46
Conversation
WalkthroughThis pull request introduces a new Bash language package ( Changes
Sequence Diagram(s)sequenceDiagram
participant U as User
participant PM as Package Manager
participant PI as postinstall.js
participant SL as @ast-grep/setup-lang
U->>PM: Install @ast-grep/lang-bash
PM->>PI: Execute postinstall script
PI->>SL: Call setup({ dirname: __dirname })
SL-->>PI: Confirm setup completion
sequenceDiagram
participant TR as Test Runner
participant NS as nursery.js
participant LC as Language Config (index.js)
participant P as Parser (Tree-sitter)
TR->>NS: Start testing Bash language
NS->>LC: Load registration configuration
NS->>P: Parse "echo test" snippet
P-->>NS: Return syntax tree
NS->>TR: Assert node kind "command" exists
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (7)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (6)
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (5)
packages/bash/README.md (1)
14-24: Consider expanding usage examples.The usage example demonstrates the basic functionality but could benefit from more complex examples showing different Bash constructs and how to query them.
Consider adding examples like:
- Parsing conditional statements
- Handling pipes
- Working with variable assignments
- Extracting specific nodes from more complex scripts
This would help users understand the full capabilities of the Bash language support.
packages/bash/index.d.ts (1)
1-7: Add JSDoc comments to improve documentation.The type definition is clear but could benefit from JSDoc comments explaining the purpose of each field, especially for the optional properties like
languageSymbol,metaVarChar, andexpandoChar.+/** + * Configuration for registering a language with ast-grep + */ type LanguageRegistration = { + /** Path to the language library file */ libraryPath: string + /** File extensions associated with this language */ extensions: string[] + /** Symbol used to identify the language */ languageSymbol?: string + /** Character used for meta variables in patterns */ metaVarChar?: string + /** Character used for expanding nodes in patterns */ expandoChar?: string }packages/bash/index.js (1)
4-21: Well-structured language configuration with comprehensive file extensions.The exported configuration object properly defines all necessary properties for Bash language support, with an extensive list of file extensions covering various Bash-related file types.
Consider adding a brief comment explaining the purpose of the
expandoCharproperty for better code documentation.module.exports = { libraryPath: libPath, extensions: [ 'bash', 'bats', 'cgi', 'command', 'env', 'fcgi', 'ksh', 'sh', 'tmux', 'tool', 'zsh', ], languageSymbol: 'tree_sitter_bash', + // Character used for variable expansion in Bash expandoChar: '$', }packages/bash/package.json (2)
1-5: Add package description for better discoverability.The package metadata is generally correct with appropriate naming and versioning, but the description field is empty.
Add a meaningful description to provide context about the package purpose:
{ "name": "@ast-grep/lang-bash", "version": "0.0.1", - "description": "", + "description": "Bash language support for ast-grep", "main": "index.js",
21-23: Complete the author field.The author field is empty and should be filled with appropriate information.
"keywords": ["ast-grep"], - "author": "", + "author": "NatoBoram", "license": "ISC",
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (6)
packages/bash/README.md(1 hunks)packages/bash/index.d.ts(1 hunks)packages/bash/index.js(1 hunks)packages/bash/nursery.js(1 hunks)packages/bash/package.json(1 hunks)packages/bash/postinstall.js(1 hunks)
🔇 Additional comments (8)
packages/bash/postinstall.js (1)
1-4: LGTM! Good implementation of the postinstall script.The script correctly imports and calls the
postinstallfunction from@ast-grep/setup-langwith the current directory, following the pattern used by other language packages.packages/bash/nursery.js (1)
1-4: LGTM! Good setup of dependencies for the test environment.The required dependencies are properly imported to set up the testing environment for the Bash language.
packages/bash/README.md (1)
1-13: LGTM! Clear installation instructions.The installation section provides clear steps for setting up the Bash language support in a pnpm project, including the appropriate dependencies.
packages/bash/index.d.ts (1)
9-10: LGTM! Proper export of the language registration.The constant declaration and default export are correctly implemented.
packages/bash/index.js (1)
1-3: LGTM: Module path handling is correctly implemented.The use of the
node:prefix for the built-in path module is a good practice, and the library path is correctly constructed using__dirnamefor cross-platform compatibility.packages/bash/package.json (3)
6-12: LGTM: Build and test scripts are properly configured.The scripts section defines appropriate commands for building, testing, and package lifecycle events.
24-39: LGTM: Dependencies are appropriately structured.The dependency structure is well-organized with clear separation between runtime, peer, and development dependencies. The optional peer dependency for tree-sitter-cli is a good practice.
40-46: LGTM: Publishing and pnpm configurations are properly set.The publishing configuration correctly sets public access, and the pnpm configuration for built dependencies is appropriate.
| "files": [ | ||
| "index.js", | ||
| "index.d.ts", | ||
| "type.d.ts", | ||
| "postinstall.js", | ||
| "src", | ||
| "prebuilds" | ||
| ], |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Verify the existence of type.d.ts and consider adding nursery.js to files.
The files list appears to include most necessary files, but has potential issues.
🏁 Script executed:
#!/bin/bash
# Check if type.d.ts exists and if nursery.js is needed in the files array
# Check for type.d.ts
echo "Checking for type.d.ts file..."
if [ -f "packages/bash/type.d.ts" ]; then
echo "type.d.ts exists."
else
echo "type.d.ts is listed in files array but does not exist."
fi
# Check for nursery.js
echo "Checking for nursery.js file..."
if [ -f "packages/bash/nursery.js" ]; then
echo "nursery.js exists but is not included in the files array."
else
echo "nursery.js does not exist."
fiLength of output: 462
Action Required: Update Files Array in packages/bash/package.json
- Missing File: The entry
type.d.tsis listed but does not exist in the repository. Please either create this file if it's needed or remove the entry from the files array. - Omitted File: The file
nursery.jsexists in the repository but is not included in the files array. Please consider adding it if it’s meant to be part of the package distribution.
|
Hi! thanks for the language contribution. However, given that we will have more languages in the foreseeable future, I think changeset #31 should be first introduced to the repo. I don't have time during the weekday but I will give it a try during the upcoming weekend. Thank you for your patience! |
|
That looks like difficult operation I think I'll continue with adding the test and opening the PRs and later on I'll add all the changesets all at once after it's implemented |
|
@NatoBoram changeset is added! you can update main branch and use |
It's not working locally, but I wonder if it's just a local issue. Let's see if it works in the CI.
|
Thanks! |
|
Alright, I should've added a changeset for every opened PR! |
And so it begins...
I'll write a test in the other ones before opening them.
CodeRabbit Metadata
Summary by CodeRabbit
package.jsonfile for the@ast-grep/lang-bashpackage, specifying metadata and scripts for management.