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

Username with one or more spaces causes an error #849

Open
2gn opened this issue Mar 7, 2024 · 7 comments
Open

Username with one or more spaces causes an error #849

2gn opened this issue Mar 7, 2024 · 7 comments
Labels
bug Something isn't working help wanted Extra attention is needed windows Windows specific problem

Comments

@2gn
Copy link

2gn commented Mar 7, 2024

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.

@2gn 2gn added the bug Something isn't working label Mar 7, 2024
@Canop
Copy link
Owner

Canop commented Mar 7, 2024

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.

@Canop Canop added the windows Windows specific problem label Mar 7, 2024
@2gn
Copy link
Author

2gn commented Apr 30, 2024

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.

@Canop Canop added the help wanted Extra attention is needed label Apr 30, 2024
@2gn
Copy link
Author

2gn commented May 15, 2024

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"

$process = Start-Process -FilePath 'broot.exe' `
-ArgumentList "--outcmd $($cmd_file.FullName) $args" `
-NoNewWindow -PassThru -WorkingDirectory $PWD
Wait-Process -InputObject $process #Faster than Start-Process -Wait
If ($process.ExitCode -eq 0) {

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
Copy link
Author

2gn commented Jun 13, 2024

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
Copy link
Owner

Canop commented Jun 13, 2024

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

@2gn
Copy link
Author

2gn commented Jun 13, 2024

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
Copy link
Owner

Canop commented Jun 13, 2024

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working help wanted Extra attention is needed windows Windows specific problem
Projects
None yet
Development

No branches or pull requests

2 participants