search

TheR1D / shell_gpt

A command-line productivity tool powered by AI large language models like GPT-4, will help you accomplish your tasks faster and more efficiently.
MIT License
8.84k stars 696 forks source link

Not accurate judgement on shell excutable in Shell Integration feature #464

Open psi-cmd opened 5 months ago

psi-cmd commented 5 months ago

https://github.com/TheR1D/shell_gpt/blob/ad6d297b28e52f9be48b0dad9ba9d06ccb1cd47b/sgpt/utils.py#L75-L83

In most system, the SHELL variable is set to /usr/bin/* instead of /bin/*.

sorokine commented 4 months ago

Properly detecting shell can be tricky. The best option is to use a dedicated package like shellingham. I did it in my fork but cannot pass the tests: https://github.com/sorokine/shell_gpt

slacksystem commented 4 months ago

Couldn't it just check for the basename? i.e.

if shell.split("/")[-1] == "zsh":

and

elif shell.split("/")[-1] == "bash":

This would allow for it to work with zsh or bash regardless of where it is installed, and unless I'm mistaken the capability of the integrations doesn't depend on the path of the shell binary, it just has to be zsh or bash.

sorokine commented 4 months ago

There are many cases when it's hard to detect which shell the program is running under like in Unix-like environments on Windows, none-interactive shells, SHELL variable is not set correctly, etc. As for checking for the basename, I am not even sure that it would work on Windows (anyway, better to use os.path.basename because it takes into account OS path name conventions).

There is also a case of none-interactive installs when the current shell name is irrelevant. I think it would be better to implement an argument to --install-integration option to specify the indented shell, e.g., --install-integration [bash|zsh]. Then if the argument is omitted try to guess the shell. shellingham does a good job of poking around to guess the shell but other solution are also possible.