Canop / broot

A new way to see and navigate directory trees : https://dystroy.org/broot
MIT License
10.35k stars 226 forks source link

Username with one or more spaces causes an error #849

Open 2gn opened 4 months ago

2gn commented 4 months ago

Steps to reproduce

  1. (optional) change your username to include one or more spaces
  2. If you have already br installed, run br --install
  3. run broot --install
  4. the command above creates $HOME\Documents\WindowsPowershell\Profile.ps1 with the following contents (when username is "User Name")
    . C:\Users\User\ Name\AppData\Roaming\dystroy\broot\config\launcher\powershell\br.ps1

Possible solution

Use $HOME environment variable instead so windows will automatically expand it on execution appropriately.

Canop commented 4 months ago

Using $HOME directly in the command isn't something you can do when creating a file in a program, you have to expand the path.

But an appropriate escaping is probably missing.

2gn commented 2 months ago

This also happens when trying to use inshellisense, by the way.

PS …\User Name\Development> inshellisense
'C:\Users\User' is not recognized as an internal or external command,
operable program or batch file.
2gn commented 1 month ago

It seems there are two errors that should be resolved.

  1. issue above
  2. File not found error
PS …\Development\broot> br
File not found: "C:\\Users\\User Name\\AppData\\Local\\Temp\\tmpBA8F.tmp"

https://github.com/Canop/broot/blob/8f50c72f58fbfd48d686aeb6ed1b1e79b5069204/src/shell_install/powershell.rs#L27-L33

In line 29 of powershell.rs, (New-TemporaryFile).FullName is expanded inside string expression. That's causing the error.

Plus, I noticed that broot --install patches the script only for natively installed powershell, and doesn't work with powershell core, which is newer. I'll make a different PR/issue for this.

2gn commented 3 weeks ago

Here's a hacky solution I've discovered (with help of GPT)

  1. Manually replace the path to br.ps1 created by br --install with . "$HOME\AppData\Roaming\dystroy\broot\config\launcher\powershell\br.ps1" inside $PROFILE
  2. replace the content of the file $HOME\AppData\Roaming\dystroy\broot\config\launcher\powershell\br.ps1 with following

https://github.com/Canop/broot/issues/460#issuecomment-1303005689

Function br { $args = $args -join ' ' $cmd_file = New-TemporaryFile $cmd_file_path = $cmd_file.FullName $quoted_cmd_file = ""$cmd_file_path""

$process = Start-Process -FilePath 'broot.exe' -ArgumentList "--outcmd $quoted_cmd_file $args" -NoNewWindow -PassThru -WorkingDirectory $PWD

Wait-Process -InputObject $process #Faster than Start-Process -Wait If ($process.ExitCode -eq 0) { $cmd = Get-Content $cmd_file_path Remove-Item -Path $cmd_file_path If ($cmd -ne $null) { Invoke-Expression -Command $cmd } } Else { Remove-Item -Path $cmd_file_path Write-Host "`n" # Newline to tidy up broot unexpected termination Write-Error "broot.exe exited with error code $($process.ExitCode)" } }



I know the code is pretty dirty and hacky but it now works fine. Just a workaround for people who are still struggling with this problem.
Canop commented 3 weeks ago

Instead of manually replacing those files, can a clean enough script be proposed in a PR ?

2gn commented 3 weeks ago

Instead of manually replacing those files, can a clean enough script be proposed in a PR ?

I'm working on it but since I'm not a very good programmer, it may take some time.

Canop commented 3 weeks ago

I'm working on it but since I'm not a very good programmer, it may take some time.

Nobody's born a very good programmer. Make your PR and we'll try to have some people with powershell knowledge review it and provide advice or fixes.

AeliusSaionji commented 3 days ago

Try this and tell me how it goes!

Function br {
  $args = $args -join ' '
  $cmd_file = New-TemporaryFile
  $out_file = New-TemporaryFile

  $process = Start-Process -FilePath 'broot.exe' `
                           -ArgumentList "--outcmd `"$cmd_file`" $args" `
                           -NoNewWindow -PassThru -WorkingDirectory $PWD `
                           -RedirectStandardOutput $out_file

  Wait-Process -InputObject $process # Faster than Start-Process -Wait
  Get-Content $out_file # print broot's stdout to shell stdout
  If ($process.ExitCode -eq 0) {
    $cmd = Get-Content $cmd_file
    Remove-Item $cmd_file, $out_file
    If ($cmd -ne $null) { Invoke-Expression -Command $cmd }
  } Else {
    Remove-Item $cmd_file, $out_file
    Write-Host "`n" # Newline to tidy up broot unexpected termination
    Write-Error "broot.exe exited with error code $($process.ExitCode)"
  }
}