mtxr / vscode-sqltools

Database management for VSCode
https://vscode-sqltools.mteixeira.dev?utm_source=github&utm_medium=homepage-link
MIT License
1.5k stars 302 forks source link

Enhance node path detection. Make node runtime default if available #957

Closed mtxr closed 2 years ago

mtxr commented 2 years ago

Describe here what is this PR about and what we are achieving merging this.

Improve node runtime detection.

Requires more testing on windows machines


Thank you for your contribution! Before submitting this PR, please make sure:

codecov[bot] commented 2 years ago

Codecov Report

Merging #957 (203ba55) into dev (52699c4) will decrease coverage by 0.04%. The diff coverage is 0.00%.

@@            Coverage Diff             @@
##              dev     #957      +/-   ##
==========================================
- Coverage   10.05%   10.00%   -0.05%     
==========================================
  Files         173      175       +2     
  Lines        5838     5867      +29     
  Branches     1372     1376       +4     
==========================================
  Hits          587      587              
- Misses       5251     5280      +29     
Flag Coverage Δ
extension 10.00% <0.00%> (-0.05%) :arrow_down:
formatter 100.00% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
packages/extension/src/index.ts 0.00% <0.00%> (ø)
packages/extension/src/language-client/client.ts 0.00% <0.00%> (ø)
.../extension/src/language-client/detect-node-path.ts 0.00% <0.00%> (ø)
packages/log/src/index.ts 0.00% <ø> (ø)
packages/log/src/lib/vscode.ts 0.00% <0.00%> (ø)
...connection-manager/dependency-manager/extension.ts 0.00% <0.00%> (ø)
...ager/webview/ui/components/ErrorBoundary/index.tsx 0.00% <ø> (ø)
...ings/components/DriverSelectorStepWidget/index.tsx 0.00% <0.00%> (ø)
packages/util/config-manager/vscode.ts 0.00% <0.00%> (ø)
packages/util/telemetry/vscode/index.ts 0.00% <0.00%> (ø)
... and 2 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

gjsjohnmurray commented 2 years ago

I began trying to test this on Windows. Turns out #955 broke things when the default terminal is PowerShell, because && doesn't work as a command separator:

PS C:\Users\JohnM\AppData\Local\vscode-sqltools\Data> yarn add sqlite3@5.0.10 && exit 0
yarn add v1.22.19
info No lockfile found.
[1/4] Resolving packages...
[2/4] Fetching packages...
[3/4] Linking dependencies...
[4/4] Building fresh packages...
success Saved lockfile.
success Saved 88 new dependencies.
info Direct dependencies
└─ sqlite3@5.0.10
info All dependencies
├─ @gar/promisify@1.1.3
├─ @mapbox/node-pre-gyp@1.0.10
├─ @npmcli/fs@1.1.1
├─ @npmcli/move-file@1.1.2
├─ @tootallnate/once@1.1.2
├─ abbrev@1.1.1
├─ agent-base@6.0.2
├─ agentkeepalive@4.2.1
├─ aggregate-error@3.1.0
├─ ansi-regex@5.0.1
├─ aproba@2.0.0
├─ are-we-there-yet@2.0.0
├─ balanced-match@1.0.2
├─ brace-expansion@1.1.11
├─ cacache@15.3.0
├─ chownr@2.0.0
├─ clean-stack@2.2.0
├─ color-support@1.1.3
├─ concat-map@0.0.1
├─ debug@4.3.4
├─ delegates@1.0.0
├─ depd@1.1.2
├─ detect-libc@2.0.1
├─ emoji-regex@8.0.0
├─ encoding@0.1.13
├─ env-paths@2.2.1
├─ err-code@2.0.3
├─ fs-minipass@2.1.0
├─ fs.realpath@1.0.0
├─ gauge@3.0.2
├─ graceful-fs@4.2.10
├─ has-unicode@2.0.1
├─ http-cache-semantics@4.1.0
├─ http-proxy-agent@4.0.1
├─ https-proxy-agent@5.0.1
├─ humanize-ms@1.2.1
├─ iconv-lite@0.6.3
├─ imurmurhash@0.1.4
├─ indent-string@4.0.0
├─ infer-owner@1.0.4
├─ inflight@1.0.6
├─ inherits@2.0.4
├─ ip@2.0.0
├─ is-fullwidth-code-point@3.0.0
├─ is-lambda@1.0.1
├─ isexe@2.0.0
├─ make-dir@3.1.0
├─ make-fetch-happen@9.1.0
├─ minimatch@3.1.2
├─ minipass-fetch@1.4.1
├─ minipass-pipeline@1.2.4
├─ minipass-sized@1.0.3
├─ minizlib@2.1.2
├─ mkdirp@1.0.4
├─ ms@2.1.2
├─ negotiator@0.6.3
├─ node-addon-api@4.3.0
├─ node-fetch@2.6.7
├─ node-gyp@8.4.1
├─ nopt@5.0.0
├─ npmlog@5.0.1
├─ object-assign@4.1.1
├─ p-map@4.0.0
├─ path-is-absolute@1.0.1
├─ promise-inflight@1.0.1
├─ promise-retry@2.0.1
├─ readable-stream@3.6.0
├─ retry@0.12.0
├─ safe-buffer@5.2.1
├─ safer-buffer@2.1.2
├─ set-blocking@2.0.0
├─ signal-exit@3.0.7
├─ smart-buffer@4.2.0
├─ socks-proxy-agent@6.2.1
├─ socks@2.7.0
├─ sqlite3@5.0.10
├─ ssri@8.0.1
├─ string_decoder@1.3.0
├─ string-width@4.2.3
├─ tar@6.1.11
├─ tr46@0.0.3
├─ unique-filename@1.1.1
├─ unique-slug@2.0.2
├─ util-deprecate@1.0.2
├─ webidl-conversions@3.0.1
├─ whatwg-url@5.0.0
├─ which@2.0.2
└─ wide-align@1.1.5
Done in 31.85s.
exit: The term 'exit' is not recognized as a name of a cmdlet, function, script file, or executable program.
Check the spelling of the name, or if a path was included, verify that the path is correct and try again.
PS C:\Users\JohnM\AppData\Local\vscode-sqltools\Data>

Simpler example:

PS C:\Users\JohnM\Documents\GitHub\gjsjohnmurray\vscode-sqltools\test> date

07 September 2022 17:41:13

PS C:\Users\JohnM\Documents\GitHub\gjsjohnmurray\vscode-sqltools\test> date && exit 0

07 September 2022 17:41:20
exit: The term 'exit' is not recognized as a name of a cmdlet, function, script file, or executable program.
Check the spelling of the name, or if a path was included, verify that the path is correct and try again.

PS C:\Users\JohnM\Documents\GitHub\gjsjohnmurray\vscode-sqltools\test>

Changing from && to & seems to work in PowerShell and also in Windows Command Prompt (aka cmd), but I guess this will need to vary by shell:

https://github.com/mtxr/vscode-sqltools/blob/dd92ea8c486b8110282e49852c285c46d1f64bd0/packages/plugins/connection-manager/dependency-manager/extension.ts#L71

gjsjohnmurray commented 2 years ago

Per https://github.com/PowerShell/PowerShell/issues/13253#issuecomment-663149471 it seems this is how to do it on PS 7+

              terminal.sendText(`${dependencyManagerSettings.packageManager} ${args.join(" ")} && $(exit 0)`);
mtxr commented 2 years ago

@gjsjohnmurray have you tried msdos? I have no windows machines here atm. I need to setup a VM if necessary. I've tested on linux and macos, both are 100%.

If you can't i'll try a VM when possible

gjsjohnmurray commented 2 years ago

have you tried msdos?

Yes, it still works on my Windows VS Code after I set Command Prompt (cmd.exe) as the default shell.