You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
regardless of what your current shell is, heroku autocomplete --refresh-cache fails if your default shell is not supported.
for example:
my default shell is fish. fish is not supported.
i switch to bash which is supported, but leave my default shell as fish.
I try and run the command above. I get an error that fish shell is not supported despite the fact that i'm not in fish.
What is the expected behavior?
when a user is running a supported shell they should never get an error about some other shell not being supported.
To put it another way, the function that test for supported shells should be testing the current shell not the default shell.
Please mention your heroku and OS version.
heroku/7.60.2 darwin-x64 node-v14.19.0
macOS 12.4
Please state if you are behind an HTTP proxy or company firewall as well.
yes, but that's not relevant.
BUG SOURCE
I believe the bug originates in the _shell() function in config.js which is using the wrong environment variable to test the current shell.
_shell() {
let shellPath;
/// BUG HERE vvvvvvvvvvvvvvvvvvvvv
const { SHELL, COMSPEC } = process.env;
/// BUG THERE ^^^^^^^^^^^^^^^^^
if (SHELL) {
shellPath = SHELL.split('/');
}
else if (this.windows && COMSPEC) {
shellPath = COMSPEC.split(/\\|\//);
}
else {
shellPath = ['unknown'];
}
return shellPath[shellPath.length - 1];
}
Specifically, the problem is that the SHELL environment variable returns the default shell not the current shell. If you want the current shell you should ask for the 0 (zero) environment variable. In bash and zsh this returns bash or zsh respectively. in fish it returns nothing.
[edit: after further testing I don't think that that function is the source of my problem (at least not with autocomplete) but it is definitely going to be the source of problems and both instances of that function need to be corrected.
example in use
❯ bash # switching to bash
➜ echo $0
bash
➜ zsh # switching to zsh
% echo $0
zsh
according to this line
plugin-autocomplete/lib/base.js
18: if (!['bash', 'zsh'].includes(shell)) {
those are you only two supported shells, so the 0 environment variable should be sufficient for your tests. However... it doesn't appear that process.env["0"] returns anything so... you may have to get at that info in a different way. More info on this problem can be found in this stack overflow answer but i haven't figured out what the idiomatic node way of obtaining this information is.
Regardless, SHELL is absolutely not the correct way to access the information you're looking for.
The text was updated successfully, but these errors were encountered:
masukomi
changed the title
testing for autocompletion support uses faulty test
testing for current shell uses incorrect environment variable
Jul 1, 2022
What is the current behavior?
regardless of what your current shell is,
heroku autocomplete --refresh-cache
fails if your default shell is not supported.for example:
What is the expected behavior?
when a user is running a supported shell they should never get an error about some other shell not being supported.
To put it another way, the function that test for supported shells should be testing the current shell not the default shell.
yes, but that's not relevant.
BUG SOURCE
I believe the bug originates in the
_shell()
function inconfig.js
which is using the wrong environment variable to test the current shell.Specifically, the problem is that the
SHELL
environment variable returns the default shell not the current shell. If you want the current shell you should ask for the0
(zero) environment variable. Inbash
andzsh
this returnsbash
orzsh
respectively. in fish it returns nothing.[edit: after further testing I don't think that that function is the source of my problem (at least not with autocomplete) but it is definitely going to be the source of problems and both instances of that function need to be corrected.
example in use
according to this line
those are you only two supported shells, so the
0
environment variable should be sufficient for your tests. However... it doesn't appear thatprocess.env["0"]
returns anything so... you may have to get at that info in a different way. More info on this problem can be found in this stack overflow answer but i haven't figured out what the idiomatic node way of obtaining this information is.Regardless,
SHELL
is absolutely not the correct way to access the information you're looking for.The text was updated successfully, but these errors were encountered: