kelleyma49 / PSFzf

A PowerShell wrapper around the fuzzy finder fzf
MIT License
787 stars 34 forks source link

Fix: UNC Handling and Optimize Invoke-FuzzyEdit #106

Closed mattcargile closed 2 years ago

mattcargile commented 2 years ago

Fixes #104

Added script:DefaultFileSystemCmdUnc to Get-FileSystemCmd to pushd to UNC path prior to calling dir.

For Invoke-FuzzyEdit, removed redundant try/catch block after grabbing files. Replaced files += array pattern. Moved all code into try block and added logic to bail out if $files is empty. Removing $files empty declaration and changing to $null check on $files from .Count check. Adding final check for $PWD at UNC to avoid code.cmd/cmd.exe throwing the UNC message.

kelleyma49 commented 2 years ago

Thank you for the PR. I need some time to review this

mattcargile commented 2 years ago

Yeah I see we have some conflicts now and would have with the other PR #117. Not sure if I should pull in all the recent changes and attempt a merge. With the new if blocks, it'll be interesting with another layer for UNC.

kelleyma49 commented 2 years ago

Apologies for the conflicts @mattcargile . I had only realized there were conflicts after merging the later PRs. I believe I have resolved the conflicts. Please take a look and see if I missed anything.

mattcargile commented 2 years ago

Thanks for the merge! 👍

Small variable change and I corrected a bug in the default cmd/dir usage when the current directory was a UNC path.

kelleyma49 commented 2 years ago

I'm curious about the Invoke-FuzzyEdit optimization. Was there a bottleneck? And how much faster is this version?

mattcargile commented 2 years ago

I added one small edit to further remove pipeline usage. To your question, this link explains it well. The += syntax isn't ideal as it recreates the array every time. Though it is fair that no one would be opening 10000 files at once though. I played with a sample like the below and the time was negligible with 100 items. It can be hard to get a test going with Invoke-Fzf because of the selection process.

+= Version

$stopwatch = [System.Diagnostics.Stopwatch]::StartNew()
$bucket = @()
1..100 | Foreach-Object {
$bucket += "I am adding $_"
}
$stopwatch.Stop()
$report = '{0} elements collected in {1:n1} milliseconds' 
$report -f $bucket.Count, $stopwatch.Elapsed.TotalMilliseconds

Non += Version

$stopwatch = [System.Diagnostics.Stopwatch]::StartNew()
$bucket = 1..100 | Foreach-Object {
"I am adding $_"
}
$stopwatch.Stop()
$report = '{0} elements collected in {1:n1} milliseconds' 
$report -f $bucket.Count, $stopwatch.Elapsed.TotalMilliseconds

The next thing is all the conflicts. Not sure if want me to squash my commits and make one against the master branch? I'll work on squashing my commits into one and pulling in the recent changes in master.

kelleyma49 commented 2 years ago

The next thing is all the conflicts. Not sure if want me to squash my commits and make one against the master branch? I'll work on squashing my commits into one and pulling in the recent changes in master.

Sounds good! Thank you!