-
-
Notifications
You must be signed in to change notification settings - Fork 569
fix: broader the dev environment detection #748
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
✅ Deploy Preview for melodious-froyo-4871f8 ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
src/utils.ts
Outdated
| return !!(false | ||
| || process.env.VSCODE_PID | ||
| || process.env.VSCODE_CWD | ||
| || Object.keys(process.env).find(value => /^VSCODE_.*/.test(value)) |
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 this is way too broad and easy to lead to false positives. I'd prefer to list more known envs hard-coded.
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.
Ok.
Maybe process.env.TERM_PROGRAM === "vscode"?
This one is set in my VSCodium environment.
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.
Sure
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.
Hi there,
I‘ve modified the code accordingly (see 8110667).
VSCodium does not set `VSCODE_PID` so it was not detected as a dev environment. The code now checks if "TERM_PROGRAM" variable is set to `vscode` instead. See [code in `addTerminalEnvironmentKeys` function](https://github.com/microsoft/vscode/blob/3c257409f43577bc797a94acee32b9213b382958/src/vs/workbench/contrib/terminal/common/terminalEnvironment.ts#L62)
| return !!(false | ||
| || process.env.VSCODE_PID | ||
| || process.env.VSCODE_CWD | ||
| || process.env.TERM_PROGRAM === "vscode" |
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 we should keep the oringal ones and add this as an addotion
Description
VSCodium does not set
VSCODE_PIDso it was not detected as a dev environment.The code now checks if there is at least one environment variable starting with "VSCODE_" (there are a few ones on VSCodium)
Linked Issues
Fixes #747
Additional context
I was investigating why my editor was falling into this kind of error: "eslint RangeError: Error while loading rule 'prefer-const': Maximum call stack size exceeded".