Enhance additionalPowerShellExes setting
#4686
Conversation
|
It might not have been bothering anyone but me, but it was really bothering me. |
|
|
||
| // Handle Windows where '.exe' and 'powershell' are things | ||
| if (this.platformDetails.operatingSystem === OperatingSystem.Windows) { | ||
| if (!exePath.endsWith("pwsh.exe") && !exePath.endsWith("powershell.exe")) { |
There was a problem hiding this comment.
Agree probably need some tests to validate all these potential permutations, looks good to me though.
There was a problem hiding this comment.
I realized last night I should also be able to handle the common case of ~/, and share that and the quote-stripping with the cwd setting, and then do a preview and then work on saving the cwd in a cache instead of a setting.
There was a problem hiding this comment.
Ok so I decided that since the Python extension uses untildify I'm fine bringing it too, which means I'll get that taken care of...in the next PR. @JustinGrote could you give this a review? It's got tests now!
andyleejordan
force-pushed
the
andyleejordan/better-finding
branch
from
ce9f6d7 to
05b837f
Compare
andyleejordan
changed the title
additionalPowerShellExes settingadditionalPowerShellExes setting
Now the warning that's emitted when an additional PowerShell executable isn't found can be suppressed with a setting. If the setting's value has single or double quotes, it handles it. If the executable name wasn't appended, it handles it. Smarter!
andyleejordan
force-pushed
the
andyleejordan/better-finding
branch
from
05b837f to
350eaff
Compare
andyleejordan
added
Issue-Enhancement
|
Sorry for the delay in testing, it LGTM to me too even tho it just got merged :) |
Now the warning that's emitted when an additional PowerShell executable isn't found can be suppressed with a setting. If the setting's value has single or double quotes, it handles it. If the executable name wasn't appended, it handles it. Smarter!
Work in progress because it needs unit tests... @JustinGrote what do you think of this?