-
Notifications
You must be signed in to change notification settings - Fork 134
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
Add support for grep in find_files #625
Conversation
@@ -6,7 +6,7 @@ import { LinkCard } from '@astrojs/starlight/components'; | |||
|
|||
### Builtin tools | |||
|
|||
<LinkCard title="fs_find_files" description="Finds file matching a glob pattern." href="/genaiscript/reference/scripts/system#systemfs_find_files" /> | |||
<LinkCard title="fs_find_files" description="Finds file matching a glob pattern. Use pattern to specify a regular expression to search for in the file content." href="/genaiscript/reference/scripts/system#systemfs_find_files" /> |
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.
The description for fs_find_files
has been updated to include information about using a pattern to search for file content.
generated by pr-docs-review-commit
update_description
@@ -365,21 +365,21 @@ def(`File ${folder}/data.json`, `...`, { | |||
|
|||
### `system.fs_find_files` | |||
|
|||
File Find Files | |||
File find files |
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.
The title "File Find Files" has been updated to "File find files" to match the description change.
generated by pr-docs-review-commit
update_title
|
||
- tool `fs_find_files`: Finds file matching a glob pattern. | ||
- tool `fs_find_files`: Finds file matching a glob pattern. Use pattern to specify a regular expression to search for in the file content. |
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.
The description for the fs_find_files
tool has been updated to include information about using a pattern to search for file content.
generated by pr-docs-review-commit
update_description
pattern: { | ||
type: "string", | ||
description: "Optional regular expression pattern to search for in the file content.", | ||
} |
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.
A new parameter pattern
has been added to the fs_find_files
tool, allowing users to specify a regular expression pattern to search for in the file content.
generated by pr-docs-review-commit
add_parameter
const { glob, pattern } = args | ||
console.log(pattern ? `grep ${pattern} ${glob}` : `ls ${glob}`) | ||
const res = pattern ? (await (workspace.grep(pattern, glob, { readText: false }))).files : await workspace.findFiles(glob, { readText: false }) | ||
if (!res?.length) return "No files found." | ||
return res.map((f) => f.filename).join("\n") |
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.
The code for the fs_find_files
tool has been updated to handle the new pattern
parameter and provide appropriate console output and file searching behavior based on its presence.
generated by pr-docs-review-commit
update_code
arrayify(template.tools).forEach((tool) => | ||
systems.push(...resolveTools(prj, tool)) | ||
) | ||
} |
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.
There is no error handling for the case when template.tools
is undefined or null. Consider adding a check for this case to prevent potential runtime errors. 🛠️
generated by pr-review-commit
missing_error_handling
const system = prj.templates.find( | ||
function resolveTools(prj: Project, tool: string): string[] { | ||
const toolsRx = new RegExp(`defTool\\s*\\(\\s*('|"|\`)${tool}`) | ||
const system = prj.templates.filter( | ||
(t) => t.isSystem && toolsRx.test(t.jsSource) | ||
) |
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.
The regular expression defTool\\s*\\(\\s*('|"|\
)${tool}is missing the closing part for the quotes. This could lead to unexpected matches. Consider adding
('|"|`)` at the end of the regex. 🧐
generated by pr-review-commit
regex_incomplete
@@ -208,7 +208,7 @@ interface ScriptRuntimeOptions { | |||
/** | |||
* List of tools used by the prompt. | |||
*/ | |||
tools?: SystemToolId[] | |||
tools?: SystemToolId | SystemToolId[] |
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.
The type of tools
is ambiguous. It can be either SystemToolId
or SystemToolId[]
. This could lead to confusion and potential bugs in the future. Consider creating a separate type for this field or always use an array even for a single value. 🤔
generated by pr-review-commit
ambiguous_type
This pull request adds support for using a regular expression pattern to search for specific content in the files when using the
fs_find_files
tool. Previously, only glob patterns were supported. Now, users can specify a pattern to search for in the file content. This enhances the functionality of thefs_find_files
tool and provides more flexibility in file searching.