-
Notifications
You must be signed in to change notification settings - Fork 8
Add and polish some tools #4
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
base: main
Are you sure you want to change the base?
Conversation
* llm-tool-collection.el (llm-tool-collection-deftool): Fix checkdoc errors in documentation string.
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.
Sincere apologies for the delay; life unfortunately got in the way. Thanks so much for these! Looks good, just need to add the new tools to the README (follow the existing structure there) 🙂.
* llm-tool-collection.el (llm-tool-collection--view-text): New helper function. (view-buffer): Add error handling and use the new helper function. (view-file): New tool for viewing file contents. (llm-tool-collection--make-edit): New helper function. (edit-buffer): Improve error handling and use the helper function. (edit-file, glob, replace-buffer, replace-file, grep, ls) (buffer-search, list-buffers, bash): New tools. * README.org: Add table of contents sections for Search and System. Add CUSTOM_ID properties to all sections to support navigation in Org. (view-file, edit-file, glob, replace-file, ls, replace-buffer) (buffer-search, list-buffers, grep, bash): Add documentation
* llm-tool-collection.el (llm-tool-collection-deftool): Add missing space after period. (llm-tool-collection--make-edit): Use grave accent quotes for variable names in error messages. Fix error message punctuation. (edit-file): Improve docstring clarity and format. (list-buffers): Remove redundant period in error message. (bash): Remove unused error variable in condition-case.
* README.org (llm-tool-collection-register-with-gptel): Simplify so as to simply call 'gptel-make-tool'. The latter function has a setter that manages the internal 'gptel--known-tools' and 'gptel-tools' lists and correctly handles duplicates.
|
Thanks, went ahead and added them to the README (also added custom_id properties, for navigation in Org). Let me know if I missed anything else |
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.
Thanks! Not a blocker, but I am curious about your opinion on two points:
- I've seen other examples of tools name them in very simple and explicit vocabulary. For example, instead of
greporbash, should we consider naming tools along the lines ofsearch_textorrun_commandinstead? - I'd like to minimize tool duplication and overlap wherever possible. Is there any specific reason to create a new
lstool instead of modifying the existinglist_directorytool? Presumably, we can simply add the new optional parameters, since adding optional parameters doesn't really change existing interfaces too much.
|
Good points! Let's see.
I like the Unix-style names like I do see the value in user-friendly plain-English names. We could consider adding an
Another key difference is the output format: To focus such discussions, it might help to start collecting common tool use scenarios. This would help clarify which tools work best in practice and provide a foundation for testing. Personally, the vast majority of my tool use so far has just been with |
|
Hello, just thought I'd check if anything else was missing on my end here. |
Hi, I sat down and lightly polished the tools I had written earlier, so I figured I'd get them here so that others can use and improve upon them. Happy to update the README if the overall shape of the PR is approved.