shell_gpt icon indicating copy to clipboard operation
shell_gpt copied to clipboard

Not accurate judgement on shell excutable in Shell Integration feature

Open psi-cmd opened this issue 1 year ago • 3 comments

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/*.

psi-cmd avatar Feb 03 '24 20:02 psi-cmd

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

sorokine avatar Feb 13 '24 16:02 sorokine

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.

slacksystem avatar Feb 14 '24 04:02 slacksystem

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.

sorokine avatar Feb 14 '24 15:02 sorokine