-
Notifications
You must be signed in to change notification settings - Fork 23
feat: auto detect globs #389
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
Conversation
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.
❌ Changes requested. Reviewed everything up to 09130bf in 39 seconds
More details
- Looked at
302lines of code in9files - Skipped
1files when reviewing. - Skipped posting
2drafted comments based on config settings.
1. src/extension/common.ts:43
- Draft comment:
Ensureworkspace.workspaceFoldersis defined before accessinguris[0]to prevent potential errors when there are no workspace folders. - Reason this comment was not posted:
Comment did not seem useful.
2. src/webview/hooks/useQuery.tsx:73
- Draft comment:
Consider cleaning up thechildPort.onMessagelistener inuseEffectto prevent potential memory leaks when the component is unmounted. - Reason this comment was not posted:
Marked as duplicate.
Workflow ID: wflow_2Mf3wb6D4k2e70MP
Want Ellipsis to fix these issues? Tag @ellipsis-dev in a comment. You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.
|
Headless test is failed, anything I missed? @HerringtonDarkholme |
5ede730 to
f8a73da
Compare
|
I have rewritten the logic, now there is no auto-detect, we will force enable globs if the |
| const validIncludeFile = includeFile.split(',').filter(Boolean) | ||
| const hasGlobPattern = validIncludeFile.some(i => i.includes('*')) | ||
| if (hasGlobPattern) { | ||
| args.push(...validIncludeFile.map(i => `--globs=${i}`)) |
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.
looks nice
| <li> | ||
| If you're using glob patterns in the include file, please ensure | ||
| that your version of ast-grep is above | ||
| <code style={codeStyle}>v0.28.0</code>. |
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 is good enough. thanks
Now ast-grep vscode supports
globswhen binary version is abovev0.28.0.Important
Adds support for auto-detecting and using glob patterns in ast-grep VSCode extension when binary version is 0.28.0 or higher.
detectSgVersion()incommon.tsto determine the binary version.buildCommand()insearch.tsto include glob patterns if supported.IncludeFile.tsxto update placeholder text and handle glob input based on version.useDetectGlobFeatureAvailable()inuseFeature.tsxto check glob feature availability.useQuery.tsxanduseSearch.tsxto handle glob state in search queries.compare-versionstopackage.jsonfor version comparison.This description was created by
for 09130bf. It will automatically update as commits are pushed.