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: UNC Handling and Optimize Invoke-FuzzyEdit #106

Closed
wants to merge 0 commits into from

Conversation

mattcargile
Copy link
Contributor

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

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

@mattcargile
Copy link
Contributor Author

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

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

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

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

@mattcargile
Copy link
Contributor Author

mattcargile commented Mar 16, 2022

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

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!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Invoke-FuzzyEdit - Cmd.exe UNC Handling
2 participants