feat: reset button #650
feat: reset button #650
Conversation
WalkthroughThe pull request introduces a new reset functionality for the AST grep playground. A new Changes
Assessment against linked issues
Poem
✨ Finishing Touches
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 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: 1
🧹 Nitpick comments (7)
website/src/components/astGrep/template/source.ts (2)
4-6: Add return type annotation for the function.
Consider explicitly annotating the return type fortryAstGrep()to improve clarity in TypeScript.-function tryAstGrep() { +function tryAstGrep(): void { console.log('matched in metavar!') }
8-11: Use consistent indentation and formatting for multi-line expressions.
The multi-line chaining is valid, but consider a consistent indentation style to improve readability.website/src/components/ResetConfig.vue (1)
7-11: Consider preserving user modifications before resetting.
You might retrieve the current config or notify the user before overwriting. Otherwise, there's a risk of unintended data loss.website/src/components/editors/Monaco.vue (3)
28-29: Consider using a more specific type instead ofany.The second event parameter for
changeCursoris typed asany. You could use a more specific type (e.g.,monaco.IModelContentChangedEventor another relevant Monaco type) for clearer type safety and improved maintainability.
86-86: Provide a more explicit cursor event type.Currently,
'changeCursor'emits the raweobject without explicit type information. Consider specifying the appropriate Monaco interface for stronger type checking and better IDE support.
187-187: Use of self-closing<div />in Vue templates may be incompatible with some tooling.While many Vue build pipelines handle self-closing tags, standard HTML5 doesn't officially allow self-closing
<div>. For maximum compatibility, consider writing<div class="editor" ref="containerRef"></div>.- <div class="editor" ref="containerRef" /> + <div class="editor" ref="containerRef"></div>website/src/components/astGrep/template/config.yaml (1)
3-8: Consider extending the rule set to cover more console methods.Currently, only
console.logandconsole.debugare matched. If you aim to replace all console calls with a custom logger, you might also include methods likeconsole.warnorconsole.errorfor completeness.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
website/.vitepress/config.ts(1 hunks)website/src/components/Playground.vue(4 hunks)website/src/components/ResetConfig.vue(1 hunks)website/src/components/astGrep/state.ts(3 hunks)website/src/components/astGrep/template/config.yaml(1 hunks)website/src/components/astGrep/template/source.ts(1 hunks)website/src/components/editors/Monaco.vue(6 hunks)website/src/icons/Reset.vue(1 hunks)
✅ Files skipped from review due to trivial changes (2)
- website/.vitepress/config.ts
- website/src/icons/Reset.vue
🧰 Additional context used
🪛 Biome (1.9.4)
website/src/components/astGrep/template/source.ts
[error] 12-12: Unexpected constant condition.
(lint/correctness/noConstantCondition)
🔇 Additional comments (15)
website/src/components/astGrep/template/source.ts (1)
1-3: No issues with the initial comments.
They appear to be demonstrative statements for console logging.website/src/components/ResetConfig.vue (4)
1-3: Imports look correct.
These references are straightforward and do not raise any issues.
5-5: Confirm the availability of thedefineModel()utility.
Ensure thatdefineModel()is properly registered or imported, as it may be part of a custom or experimental API.
14-18: Button usage is straightforward.
The reset button and icon usage is clear and self-explanatory.
20-31: No styling conflicts identified.
The scoped styles are minimal and align with the component’s functionality.website/src/components/astGrep/state.ts (2)
5-7: Good switch to external resources.
Importingsourceandconfigfrom separate files improves maintainability and keeps this file neat.
45-45: Default query is clear and concise.
Defining'console.log($MATCH)'as the default fosters a straightforward starting point.website/src/components/Playground.vue (4)
7-7: New import forResetConfigis consistent.
The newly added component reference is valid and properly placed.
17-17: Double-check the parser initialization.
Commenting outawait initializeParser()may break certain functionalities if parser setup is needed elsewhere.
99-102: Effective placement of the reset functionality.
IntegratingResetConfigwith the language selector is intuitive and improves usability.
143-143: No significant style modifications beyond this line.
Nothing additional to note.website/src/components/editors/Monaco.vue (4)
83-83: The change event logic is correct.Emitting
'update:modelValue'here ensures that the parent's modelValue is properly synchronized with the editor content. No issues found.
102-108: The highlight transformation logic looks good.By offsetting indices to align with Monaco's 1-based line/column convention, the highlights and ranges will be rendered accurately. This is correct and consistent with Monaco’s API.
147-154: Excellent use of a watcher to maintain synchronization.The check prevents redundant updates and avoids potential infinite loops. This ensures the editor content remains in sync with external changes to
modelValue.
202-206: Styling for.monaco-match-spanis well-defined.The background color is clearly differentiated from the highlight span. This helps users visually distinguish different match categories.
| if (true) { | ||
| const notThis = 'console.log("not me")' | ||
| } else { | ||
| console.debug('matched by YAML') | ||
| } |
There was a problem hiding this comment.
Avoid using a constant condition.
if (true) causes the else block to be unreachable. If your intention is to test both blocks, replace true with a real condition or remove the else.
-if (true) {
+if (someDynamicCondition) {
const notThis = 'console.log("not me")'
} else {
console.debug('matched by YAML')
}Committable suggestion skipped: line range outside the PR's diff.
🧰 Tools
🪛 Biome (1.9.4)
[error] 12-12: Unexpected constant condition.
(lint/correctness/noConstantCondition)
| @@ -13,7 +14,7 @@ import { initializeParser, useAstGrep, Mode as ModeImport } from './astGrep' | |||
| import '../style.css' | |||
|
|
|||
| // important initialization | |||
| await initializeParser() | |||
| // await initializeParser() | |||
There was a problem hiding this comment.
this should not be removed
There was a problem hiding this comment.
Sorry, it's needed here. I commented it out during debugging and forgot to withdraw it.
| @@ -36,40 +39,10 @@ export function deserialize(str: string): State { | |||
| return JSON.parse(atou(str)) | |||
| } | |||
|
|
|||
| const source = | |||
There was a problem hiding this comment.
I don't think moving it to standalone files make too much difference. I am okay with hardcoding these default templates.
There was a problem hiding this comment.
Maybe will use it somewhere else in the future, so I used a separate file.
reset config for PatternConfig
20250124_170927.mp4
close #267
Summary by CodeRabbit
Release Notes
New Features
Improvements
Code Quality