refactor(system): Refactor PATH related functions

- `strip_path()` -> `Find-Path()` - `ensure_in_path()`/`add_first_in_path()` -> `Add-Path()` - `remove_from_path()` -> `Remove-Path()`
ScoopInstaller · Mar 22, 2024 · 8eb5fad · 8eb5fad
1 parent ae81ad0
commit 8eb5fad
Show file tree

Hide file tree

Showing 6 changed files with 83 additions and 45 deletions.
diff --git a/bin/uninstall.ps1 b/bin/uninstall.ps1
@@ -99,7 +99,6 @@ if ($purge) {
     if ($global) { keep_onlypersist $globaldir }
 }
 
-remove_from_path (shimdir $false)
-if ($global) { remove_from_path (shimdir $true) }
+Remove-Path -Path (shimdir $global) -Global:$global
 
 success 'Scoop has been uninstalled.'
diff --git a/lib/core.ps1 b/lib/core.ps1
@@ -581,12 +581,18 @@ function fullpath($path) {
     $ExecutionContext.SessionState.Path.GetUnresolvedProviderPathFromPSPath($path)
 }
 function friendly_path($path) {
-    $h = (Get-PsProvider 'FileSystem').home; if(!$h.endswith('\')) { $h += '\' }
-    if($h -eq '\') { return $path }
-    return "$path" -replace ([regex]::escape($h)), "~\"
+    $h = (Get-PSProvider 'FileSystem').Home
+    if (!$h.EndsWith('\')) {
+        $h += '\'
+    }
+    if ($h -eq '\') {
+        return $path
+    } else {
+        return $path -replace ([Regex]::Escape($h)), '~\'
+    }
 }
 function is_local($path) {
-    ($path -notmatch '^https?://') -and (test-path $path)
+    ($path -notmatch '^https?://') -and (Test-Path $path)
 }
 
 # operations
@@ -821,7 +827,7 @@ function warn_on_overwrite($shim, $path) {
 function shim($path, $global, $name, $arg) {
     if (!(Test-Path $path)) { abort "Can't shim '$(fname $path)': couldn't find '$path'." }
     $abs_shimdir = ensure (shimdir $global)
-    ensure_in_path $abs_shimdir $global
+    Add-Path -Path $abs_shimdir -Global:$global
     if (!$name) { $name = strip_ext (fname $path) }
 
     $shim = "$abs_shimdir\$($name.tolower())"

diff --git a/lib/install.ps1 b/lib/install.ps1
@@ -920,7 +920,7 @@ function env_add_path($manifest, $dir, $global, $arch) {
             if (!(is_in_dir $dir $path_dir)) {
                 abort "Error in manifest: env_add_path '$_' is outside the app directory."
             }
-            add_first_in_path $path_dir $global
+            Add-Path -Path $path_dir -Global:$global -Force
         }
     }
 }
@@ -935,7 +935,7 @@ function env_rm_path($manifest, $dir, $global, $arch) {
             } else {
                 $path_dir = Join-Path $dir $_
             }
-            remove_from_path $path_dir $global
+            Remove-Path -Path $path_dir -Global:$global
         }
     }
 }

diff --git a/lib/system.ps1 b/lib/system.ps1
@@ -71,44 +71,62 @@ function Set-EnvVar {
     Publish-EnvVar
 }
 
-function ensure_in_path($dir, $global) {
-    $path = Get-EnvVar -Name 'PATH' -Global:$global
-    $dir = fullpath $dir
-    if ($path -notmatch [regex]::escape($dir)) {
-        Write-Output "Adding $(friendly_path $dir) to $(if($global){'global'}else{'your'}) path."
-        Set-EnvVar -Name 'PATH' -Value "$dir;$path" -Global:$global # future sessions
-        $env:PATH = "$dir;$env:PATH" # current session
+function Find-Path {
+    param(
+        [string]$Name,
+        [string]$Path
+    )
+
+    if ($null -eq $Path -and $Path -eq '') {
+        return $false, $null
+    } else {
+        $strippedPath = $Path.Split(';', [System.StringSplitOptions]::RemoveEmptyEntries).Where({ $_ -ne $Name }) -join ';'
+        return ($strippedPath -ne $Path), $strippedPath
     }
 }
 
-function strip_path($orig_path, $dir) {
-    if ($null -eq $orig_path) { $orig_path = '' }
-    $stripped = [string]::join(';', @( $orig_path.split(';') | Where-Object { $_ -and $_ -ne $dir } ))
-    return ($stripped -ne $orig_path), $stripped
-}
+function Add-Path {
+    param(
+        [string]$Path,
+        [switch]$Global,
+        [switch]$Force
+    )
 
-function add_first_in_path($dir, $global) {
-    $dir = fullpath $dir
+    if (!$Path.Contains('%')) {
+        $Path = fullpath $Path
+    }
     # future sessions
-    $null, $currpath = strip_path (Get-EnvVar -Name 'PATH' -Global:$global) $dir
-    Set-EnvVar -Name 'PATH' -Value "$dir;$currpath" -Global:$global
+    $inPath, $strippedPath = Find-Path $Path (Get-EnvVar -Name 'PATH' -Global:$Global)
+    if (!$inPath -or $Force) {
+        Write-Output "Adding $(friendly_path $Path) to $(if ($Global) {'global'} else {'your'}) path."
+        Set-EnvVar -Name 'PATH' -Value (@($Path, $strippedPath) -join ';') -Global:$Global
+    }
     # current session
-    $null, $env:PATH = strip_path $env:PATH $dir
-    $env:PATH = "$dir;$env:PATH"
+    $inPath, $strippedPath = Find-Path $Path $env:PATH
+    if (!$inPath -or $Force) {
+        $env:PATH = @($Path, $strippedPath) -join ';'
+    }
 }
 
-function remove_from_path($dir, $global) {
-    $dir = fullpath $dir
+function Remove-Path {
+    param(
+        [string]$Path,
+        [switch]$Global
+    )
+
+    if (!$Path.Contains('%')) {
+        $Path = fullpath $Path
+    }
     # future sessions
-    $was_in_path, $newpath = strip_path (Get-EnvVar -Name 'PATH' -Global:$global) $dir
-    if ($was_in_path) {
-        Write-Output "Removing $(friendly_path $dir) from your path."
-        Set-EnvVar -Name 'PATH' -Value $newpath -Global:$global
+    $inPath, $strippedPath = Find-Path $Path (Get-EnvVar -Name 'PATH' -Global:$Global)
+    if ($inPath) {
+        Write-Output "Removing $(friendly_path $Path) from $(if ($Global) {'global'} else {'your'}) path."
+        Set-EnvVar -Name 'PATH' -Value $strippedPath -Global:$Global
     }
     # current session
-    $was_in_path, $newpath = strip_path $env:PATH $dir
-    if ($was_in_path) {
-        $env:PATH = $newpath
+    $inPath, $strippedPath = Find-Path $Path $env:PATH
+    if ($inPath) {
+        $env:PATH = $strippedPath
     }
 }
 
@@ -123,3 +141,18 @@ function env($name, $global, $val) {
         Get-EnvVar -Name $name -Global:$global
     }
 }
+
+function strip_path($orig_path, $dir) {
+    Show-DeprecatedWarning $MyInvocation 'Find-Path'
+    Find-Path -Name $dir -Path $orig_path
+}
+
+function add_first_in_path($dir, $global) {
+    Show-DeprecatedWarning $MyInvocation 'Add-Path'
+    Add-Path -Path $dir -Global:$global -Force
+}
+
+function remove_from_path($dir, $global) {
+    Show-DeprecatedWarning $MyInvocation 'Remove-Path'
+    Remove-Path -Path $dir -Global:$global
+}
diff --git a/test/Scoop-Core.Tests.ps1 b/test/Scoop-Core.Tests.ps1
@@ -168,7 +168,7 @@ Describe 'shim' -Tag 'Scoop', 'Windows' {
     BeforeAll {
         $working_dir = setup_working 'shim'
         $shimdir = shimdir
-        $(ensure_in_path $shimdir) | Out-Null
+        Add-Path $shimdir
     }
 
     It "links a file onto the user's path" {
@@ -202,7 +202,7 @@ Describe 'rm_shim' -Tag 'Scoop', 'Windows' {
     BeforeAll {
         $working_dir = setup_working 'shim'
         $shimdir = shimdir
-        $(ensure_in_path $shimdir) | Out-Null
+        Add-Path $shimdir
     }
 
     It 'removes shim from path' {
@@ -221,7 +221,7 @@ Describe 'get_app_name_from_shim' -Tag 'Scoop', 'Windows' {
     BeforeAll {
         $working_dir = setup_working 'shim'
         $shimdir = shimdir
-        $(ensure_in_path $shimdir) | Out-Null
+        Add-Path $shimdir
         Mock appsdir { $working_dir }
     }
 

diff --git a/test/Scoop-Install.Tests.ps1 b/test/Scoop-Install.Tests.ps1
@@ -58,17 +58,17 @@ Describe 'env add and remove path' -Tag 'Scoop', 'Windows' {
     }
 
     It 'should concat the correct path' {
-        Mock add_first_in_path {}
-        Mock remove_from_path {}
+        Mock Add-Path {}
+        Mock Remove-Path {}
 
         # adding
         env_add_path $manifest $testdir $global
-        Assert-MockCalled add_first_in_path -Times 1 -ParameterFilter { $dir -like "$testdir\foo" }
-        Assert-MockCalled add_first_in_path -Times 1 -ParameterFilter { $dir -like "$testdir\bar" }
+        Assert-MockCalled Add-Path -Times 1 -ParameterFilter { $Path -like "$testdir\foo" }
+        Assert-MockCalled Add-Path -Times 1 -ParameterFilter { $Path -like "$testdir\bar" }
 
         env_rm_path $manifest $testdir $global
-        Assert-MockCalled remove_from_path -Times 1 -ParameterFilter { $dir -like "$testdir\foo" }
-        Assert-MockCalled remove_from_path -Times 1 -ParameterFilter { $dir -like "$testdir\bar" }
+        Assert-MockCalled Remove-Path -Times 1 -ParameterFilter { $Path -like "$testdir\foo" }
+        Assert-MockCalled Remove-Path -Times 1 -ParameterFilter { $Path -like "$testdir\bar" }
     }
 }