Canop / broot

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

Output from Br shell function in powershell doesn't print to stdout #888

Open Alfamari opened 1 month ago

Alfamari commented 1 month ago

EDIT: Nothing outputs to stdout. Including br --help, or execution: "echo {file}" (if from_shell: false).

Print Path doesn't print to stdout on windows with powershell br function, I've also even tried 2>&1 which I've had success with other programs that had issues printing to stdout like ffmpeg. This isn't really an issue in and of itself though because I easily created a workaround, but it leads to other problems I explain later: `{

name: "Print to stdout"
invocation: "stdout"
external: "echo {file}"
from_shell: true

}`

I'm using bat as an example because I think it best illustrates the issue, though the issue is universal.

wezterm-gui_aCdnOuRt1K

wezterm-gui_lh4n4HZBD9

The issue stems from a couple of problems with staging multiple files. One: "only verbs returning to broot on end can be executed on a multi-selection". This eliminates the workaround as a possibility because the docs state "from_shell isn't compatible with leave_broot = false" The workaround also doesn't work without from_shell = true. Now print_path is able to be executed on multiple staged files, it just cannot be piped anywhere after.

Two: The default file operation commands operate sequentially which is substantially slower by spawning a brand new cmd instance each time rather than either putting all files on one line with cmd /c or using an array in powershell to let the shells handle the batch operations because there isn't a {staged_items} variable to contain multiple items yet. This has caused me to want to pipe for a speed increase and also to try and workaround some of the current limitations with staged items. Like mentioned: https://github.com/Canop/broot/issues/695#issuecomment-1518698471 https://github.com/Canop/broot/issues/266#issuecomment-972113711

In the process of making this submission I found that it works properly with the broot executable, just not the br function.

wezterm-gui_qBxiWwymqX

Though the extra newline at the end can cause issues with pipes:

wezterm-gui_64PN91wWdP

I'm using Pwsh Core 7.4.2 on Windows 10 but the issue is also present on windows powershell 5.1.

Finally, thank you for this incredible masterpiece! I'll be taking it everywhere on all my machines going forward from now on (I even put this sucker on my steam deck lol). It's a shame it isn't mentioned nearly as much as LF, ranger, or vifm. You even created a very clever workaround to the cd issue previously thought to be impossible by other terminal file managers because of the limitations between parent and child processes. Because of your implementation, it's also allowed me to save items into current shell variables which couldn't be done with the other terminal file manager implementations and is a very crucial, almost mandatory feature for me.

AeliusSaionji commented 4 days ago

Try this and tell me if it works for what you're trying to do

save the following snippet into a br.ps1, and source with . br.ps1 then br | bat...

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

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

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

The current solution I came up with involves redirecting broot's stdout to a file, then reading the file back to shell stdout, because of current oddities in Start-Process that may or may not be addressed in the near future.

https://github.com/PowerShell/PowerShell/issues/18026 https://github.com/PowerShell/PowerShell/issues/5184 https://github.com/PowerShell/PowerShell/issues/4332

Alfamari commented 3 days ago

@AeliusSaionji Yes! It works, thank you!

wezterm-gui_FWo68ddlpG

I'll just have to be wary of the new line at the end during my piping and try to have the foresight to filter it myself in the pipeline. Though, I'm not sure if I should close the issue with this or not. While it has fixed my issue, not sure if I should leave it open until it's officially implemented.

AeliusSaionji commented 2 days ago

I'll make a PR and reference this issue, don't close until it's merged :)

I can reproduce the extra newline with printpath on linux with fish's br.fish, so @Canop might want to address this in broot itself.

However it's trivial to add here, so you can use this while you wait:

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
  If ($process.ExitCode -eq 0) {
    $cmd = Get-Content $cmd_file
    $out = Get-Content $out_file -Raw
    Remove-Item $cmd_file, $out_file
    If ($out -ne $null) { Write-Output $out.TrimEnd() } # print broot's stdout to shell stdout
    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)"
  }
}
Canop commented 2 days ago

I clearly prefer the correct script to be installed by broot. But as I can't test myself on Windows, I let you create and check the PR.

AeliusSaionji commented 2 days ago

Maybe you misunderstood-

broot's printpath verb adds a newline at the end of its output, regardless of shell. Is this something that would be easy to suppress? Would it be missed if gone?

image

PowerShell pipes treat every line as an object, so it presents an issue when piping the output to powershell. If the issue is powershell specific and/or if it's desirable to keep the newline for other systems, then I'll PR the br.ps1 that trims newlines from the output.

Canop commented 2 days ago

Instead of changing :print_path, which is too specific anyway, why not having a :write_stdout verb, similar to the current :write_output but writing to stdout instead of writing to the file provided with the --verb-output launch argument.

This would allow verbs like this:

    {
    invocation: wp
    cmd: ":write {file};:quit"
    }

But would be more generic.

I would also define a :writeln_stdout.

Canop commented 2 days ago

@AeliusSaionji Can you have a look at https://github.com/Canop/broot/pull/901 ?