Skip to content

Conversation

@alexzhang1030
Copy link
Contributor

@alexzhang1030 alexzhang1030 commented Oct 8, 2024

Now ast-grep vscode supports globs when binary version is above v0.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.

  • Behavior:
    • Adds detectSgVersion() in common.ts to determine the binary version.
    • Updates buildCommand() in search.ts to include glob patterns if supported.
    • Modifies IncludeFile.tsx to update placeholder text and handle glob input based on version.
  • UI:
    • Adds useDetectGlobFeatureAvailable() in useFeature.tsx to check glob feature availability.
    • Updates useQuery.tsx and useSearch.tsx to handle glob state in search queries.
  • Dependencies:
    • Adds compare-versions to package.json for version comparison.

This description was created by Ellipsis for 09130bf. It will automatically update as commits are pushed.

Copy link

@ellipsis-dev ellipsis-dev bot left a 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 302 lines of code in 9 files
  • Skipped 1 files when reviewing.
  • Skipped posting 2 drafted comments based on config settings.
1. src/extension/common.ts:43
  • Draft comment:
    Ensure workspace.workspaceFolders is defined before accessing uris[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 the childPort.onMessage listener in useEffect to 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.

@alexzhang1030
Copy link
Contributor Author

Headless test is failed, anything I missed? @HerringtonDarkholme

@alexzhang1030
Copy link
Contributor Author

I have rewritten the logic, now there is no auto-detect, we will force enable globs if the include path has any * char.

const validIncludeFile = includeFile.split(',').filter(Boolean)
const hasGlobPattern = validIncludeFile.some(i => i.includes('*'))
if (hasGlobPattern) {
args.push(...validIncludeFile.map(i => `--globs=${i}`))
Copy link
Member

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>.
Copy link
Member

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

@HerringtonDarkholme HerringtonDarkholme merged commit ff03a5b into ast-grep:main Oct 9, 2024
1 of 4 checks passed
@alexzhang1030 alexzhang1030 deleted the feat/glob branch October 9, 2024 04:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants