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

Fix: Fixed an issue with the 'Run with PowerShell' action not supporting files with spaces in their names #16455

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

Lamparter
Copy link
Contributor

Resolved / Related Issues

To prevent extra work, all changes to the Files codebase must link to an approved issue marked as Ready to build. Please insert the issue number following the hashtag with the issue number that this Pull Request resolves.

Co-authored-by: 0x5bfa <62196528+0x5bfa@users.noreply.github.com>
@0x5bfa
Copy link
Member

0x5bfa commented Nov 10, 2024

Please provide steps you validated this change.

@Lamparter
Copy link
Contributor Author

Hang on

@yaira2 yaira2 changed the title Fix: Added quotes to PowerShell location launch arguments Fix: Fixed an issue where 'Run with PowerShell' action didn't include quotes with launch arguments Nov 10, 2024
@yaira2
Copy link
Member

yaira2 commented Nov 10, 2024

It looks like the issue is still present

@Lamparter
Copy link
Contributor Author

Yeah I know I'm working on it

@Lamparter
Copy link
Contributor Author

Yes I'm aware, I'll try again with it a bit later

@Lamparter
Copy link
Contributor Author

Sorry for opening the pr early before finding out whether the issue is fixed it's just the only way I can get the package is from the ci run.

@yaira2
Copy link
Member

yaira2 commented Nov 11, 2024

That's fine, I'll wait until it's ready.

@yaira2 yaira2 marked this pull request as draft November 11, 2024 16:02
@Lamparter Lamparter closed this Nov 15, 2024
@Lamparter
Copy link
Contributor Author

There's still something else I haven't tried but it's unlikely to work.

@yaira2
Copy link
Member

yaira2 commented Nov 21, 2024

This should fix the issue

return Win32Helper.RunPowershellCommandAsync($"\"{context.ShellPage?.SlimContentPage?.SelectedItem?.ItemPath}\"", PowerShellExecutionOptions.None);

@Lamparter
Copy link
Contributor Author

This should fix the issue

return Win32Helper.RunPowershellCommandAsync($"\"{context.ShellPage?.SlimContentPage?.SelectedItem?.ItemPath}\"", PowerShellExecutionOptions.None);

That didn't work for me, see 1db5871

@yaira2
Copy link
Member

yaira2 commented Nov 21, 2024

I forgot to add a space to the file name when I tested 😀

@yaira2
Copy link
Member

yaira2 commented Nov 21, 2024

This should work (I tested a file with a space this time).

return Win32Helper.RunPowershellCommandAsync($"& '{context.ShellPage?.SlimContentPage?.SelectedItem?.ItemPath}'", PowerShellExecutionOptions.None);

@Lamparter Lamparter reopened this Nov 21, 2024
Co-authored-by: Yair <39923744+yaira2@users.noreply.github.com>
@Lamparter Lamparter marked this pull request as ready for review November 21, 2024 16:34
Copy link
Member

@yaira2 yaira2 left a comment

Choose a reason for hiding this comment

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

LGTM

@yaira2 yaira2 changed the title Fix: Fixed an issue where 'Run with PowerShell' action didn't include quotes with launch arguments Fix: Fixed an issue with the 'Run with PowerShell' action not supporting files with spaces in their names Nov 21, 2024
@yaira2 yaira2 added ready to merge Pull requests that are approved and ready to merge and removed needs - code review labels Nov 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready to merge Pull requests that are approved and ready to merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Bug: Run with PowerShell toolbar button doesn't quote path
3 participants