Skip to content
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

standalone-installer-windows: Use call operator (&) instead of Invoke-Expression #277

Merged
merged 1 commit into from
May 3, 2023

Conversation

victorlin
Copy link
Member

@victorlin victorlin commented Apr 28, 2023

Description of proposed changes

It is not uncommon for user home directories to have spaces, and Invoke-Expression does not work well with paths that have spaces. The call operator is more suitable for this use case.

Quoting updated to reflect call operator syntax.

Related issue(s)

Fixes #276

Testing

  • Manually tested on a Windows VM installing to C:\Users\test user\.
  • Checks pass some are failing because of unrelated reasons, but this change should not affect the outcome of those checks

@victorlin victorlin requested a review from tsibley April 28, 2023 23:04
@victorlin victorlin self-assigned this Apr 28, 2023
@victorlin victorlin changed the title Use call operator (&) instead of Invoke-Expression standalone-installer-windows: Use call operator (&) instead of Invoke-Expression Apr 28, 2023
Copy link
Member

@tsibley tsibley left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, yes, this is much better. I vaguely knew about PowerShell's call operator, but didn't really grok the subtlety until now.

Totally minor, but in the commit message, I'd probably link to the official docs—https://learn.microsoft.com/en-us/powershell/module/microsoft.powershell.core/about/about_operators?view=powershell-7.3#call-operator-—instead of a third-party site.

CI is failing due to #274.

It is not uncommon for user home directories to have spaces, and
Invoke-Expression does not work well with paths that have spaces¹. The
call operator is more suitable for this use case.

Quoting updated to reflect call operator² syntax³.

¹ https://devblogs.microsoft.com/powershell/invoke-expression-considered-harmful/
² https://learn.microsoft.com/en-us/powershell/module/microsoft.powershell.core/about/about_operators?view=powershell-7.3#call-operator-
³ https://ss64.com/ps/call.html
@victorlin victorlin force-pushed the victorlin/fix-windows-install branch from 4066a63 to 6f89bf4 Compare May 3, 2023 19:33
@victorlin
Copy link
Member Author

in the commit message, I'd probably link to the official docs … instead of a third-party site.

Fair point. Updated with both links, since the official docs don't provide the syntax reference.

@victorlin
Copy link
Member Author

CI / test-source is failing but on master as well – looks to by pyright issues which are unrelated to these changes. Merging.

@victorlin victorlin merged commit 4234240 into master May 3, 2023
@victorlin victorlin deleted the victorlin/fix-windows-install branch May 3, 2023 21:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

Installation fails when there is a space in the path
2 participants