search

opensouls / terminal-copilot

A smart terminal assistant that helps you find the right command.
Apache License 2.0
573 stars 43 forks source link

Include last 40 commands of fish history in context #20

Closed 2mawi2 closed 1 year ago

2mawi2 commented 1 year ago

This pull request addresses issue #8 by adding bash history support for the prompt for fish shells. This can easily be extended to support other types of shells as well. I have tested this locally and it works as expected. With this feature, you can easily refer to previous statements and terminal-copilot will pick them up.

One constraint to consider:

I have limited the maximum command length to 100 to prevent very long commands from interfering with the maximum token amount of the API. This results in an approximate maximum of around 4100 tokens. I'm open to feedback on whether this is too much or not enough.

2mawi2 commented 1 year ago

There is another issue to consider: As the number of tokens sent to the API increases, the cost for users also increases. We could consider giving the user the ability to configure the amount of context sent to the API, giving them full control over the costs.

JoelKronander commented 1 year ago

Very cool! Nice solution you found! 🤩 Also neat code with tests!

Maybe to give users more transparency into longer prompts send to OpenAI, and also "giving their permissions" to send history that might contain sensitvie private information to OpenAI, should we add this option behind a flag like -h or --history to include it, similar to aliases now? What do you think?

2mawi2 commented 1 year ago

I like the suggestion to add a flag to enable the command history in the prompt. It might be a good idea to hide it behind a flag first, and then possibly allow the user to configure terminal-copilot with options like copilot config set-context --enable-history=true --enable-alias=true.

Would we legally be required to persist the permission of the user ? Or is intended to increase awareness about what data that will be sent? It might be a good idea to include a disclaimer about the use of these context options in the first usage of the feature and in the project's Readme.

JoelKronander commented 1 year ago

Yeah, makes sense to in addition to flag also have some option to set some config to have it be persistent. Not sure what option is easiest for storing persistent settings? Hidden config file in user home directory?

As long as it's clear how the setting works form that should work I think. Agree we should add the explanation of flags/setting to README.md to! And also include some section on what data is sent to open ai by default.

2mawi2 commented 1 year ago

@JoelKronander I implemented our discussed changes. I also added the respective documentation in Readme and the arguments help. I noticed that a command executed with os.system(cmd)does not end up in the terminal history. I had a look at https://github.com/nvbn/thefuck and it appears that the executed command has to be added manually to the history.

JoelKronander commented 1 year ago

this looks amazing! can't wait to try this out!