-
Notifications
You must be signed in to change notification settings - Fork 12
fix: get os from context tool #11
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
Get the local operating system using the official context tool and remove instructions to strip the file suffix from input. The latter will be handled in `gptscript` when the `--ui` option is provided. Signed-off-by: Nick Hale <[email protected]>
3. If no {file} was provided AND the local operating system is windows, use the open-windows tool then skip the remaining steps | ||
4. If no {file} was provided use the open-nix tool then skip the remaining steps | ||
5. If the operating system is windows, use the open-file-windows tool then skip the remaining steps | ||
6. Use the open-file-nix 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.
I think there's probably a more clever way to select the right tools based on OS via contexts, but ATM this was the most straightforward way I could come up with.
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.
I'm a little surprised that the LLM does the right thing and doesn't try to do something for each listed step.
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.
I did some basic testing to make sure it works for remote and local paths, but I didn't try every combination here (e.g. on windows). I'll check on windows and take a look at the events to see exactly what the LLM does here.
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.
checked and remote URLs are working properly on windows
tools: service, port, open-file-nix, open-nix, open-file-windows, open-windows | ||
params: file: (optional) A reference to a tool file. Can refer to either a local or remote file. | ||
|
||
Never run steps in parallel. |
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.
Was having a problem with steps 1 and 2 running in parallel. This corrects for that.
@@ -61,7 +46,7 @@ open http://localhost:${PORT} | |||
|
|||
--- | |||
|
|||
name: start-file | |||
name: open-file-windows |
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.
renamed to make it more obvious that these toosl are just the "windows variants" of the others.
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.
Eh, OK.
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.
Well, I can switch it back if we're feeling "meh" about it. Lmk
@@ -61,7 +46,7 @@ open http://localhost:${PORT} | |||
|
|||
--- | |||
|
|||
name: start-file | |||
name: open-file-windows |
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.
Eh, OK.
3. If no {file} was provided AND the local operating system is windows, use the open-windows tool then skip the remaining steps | ||
4. If no {file} was provided use the open-nix tool then skip the remaining steps | ||
5. If the operating system is windows, use the open-file-windows tool then skip the remaining steps | ||
6. Use the open-file-nix 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.
I'm a little surprised that the LLM does the right thing and doesn't try to do something for each listed step.
Get the local operating system using the official context tool and remove instructions to strip the file suffix from input. The latter will be handled in
gptscript
when the--ui
option is provided.Depends on gptscript-ai/gptscript#530