Skip to content

refactor(core): Rewrite and separate path-related functions to system.ps1#5836

Merged
niheaven merged 7 commits intodevelopfrom
refactor-system
Mar 27, 2024
Merged

refactor(core): Rewrite and separate path-related functions to system.ps1#5836
niheaven merged 7 commits intodevelopfrom
refactor-system

Conversation

@niheaven
Copy link
Copy Markdown
Member

@niheaven niheaven commented Mar 20, 2024

Description

Ed. Separate part of isolate PATH to #5840, this PR is for refactoring now.

Use config option use_isolated_path to separate Scoop apps PATH to SCOOP_PATH, and just add %SCOOP_PATH% to $env:Path when install app with env_add_path first time.

Motivation and Context

What the commits do:

  • refactor(core): Remove unused ensure_robocopy_in_path()
  • refactor(system): Move EnvVar-related functions to system.ps1
    • Move from core.ps1: Publish-Env(), env(), search_in_path(), ensure_in_path(), strip_path(), add_first_in_path(), remove_from_path()
    • Add necessary system.ps1 importing
  • refactor(EnvVar): Separate env() to Get-EnvVar() and Set-EnvVar()
  • refactor(system): Remove search_in_path()
    • Replace with Get-Command().Source
  • refactor(system): Refactor PATH-related functions
    • strip_path() -> Find-Path()
    • ensure_in_path()/add_first_in_path() -> Add-Path()
      • ensure_in_path() is only used in shim creation
      • Use -Force to forcedly add a path in the first position as add_first_in_path()
      • Add-Path() supports %
    • remove_from_path() -> Remove-Path()

How Has This Been Tested?

Passed all Scoop tests.

Checklist:

  • I have read the Contributing Guide.
  • I have ensured that I am targeting the develop branch.
  • I have updated the documentation accordingly.
  • I have updated the tests accordingly.
  • I have added an entry in the CHANGELOG.

@niheaven niheaven force-pushed the refactor-system branch 2 times, most recently from d7f0580 to 6a3fbed Compare March 21, 2024 09:33
- `strip_path()` -> `Find-Path()`
- `ensure_in_path()`/`add_first_in_path()` -> `Add-Path()`
- `remove_from_path()` -> `Remove-Path()`
@niheaven niheaven force-pushed the refactor-system branch 2 times, most recently from 7241009 to e88260e Compare March 22, 2024 02:23
@niheaven niheaven changed the title feat(path): Isolate Scoop apps' PATH refactor(core): Rewrite and separate path-related functions to system.ps1 Mar 22, 2024
Comment thread lib/core.ps1
Comment thread lib/system.ps1 Outdated
Comment thread lib/system.ps1 Outdated
}

function strip_path($orig_path, $dir) {
Show-DeprecatedWarning $MyInvocation 'Find-Path'
Copy link
Copy Markdown
Member

@chawyehsu chawyehsu Mar 27, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I realize there are references of this function in some manifests, but I think this function should not be provided and used outside of the core (though I know there is impossible to hide a function based on the current corebase, all functions are exposed, which I dislike).

What they really want is a way to manipulate $env:PATH-like environment variables, something like Add-PathLikeEnvVar and Remove-PathLikeEnvVar (further abstraction of Add-Path and Remove-Path). This is useful because there are many operations on $env:PATH-like env vars, plus the proposal of $env:SCOOP_PATH is staging. Add-Path and Remove-Path are $env:PATH-specific and may not be much useful if/after $env:SCOOP_PATH is implemented.

Copy link
Copy Markdown
Member Author

@niheaven niheaven Mar 27, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe a refactoring into modules could be done if I have time.

For now, keep the deprecated warning of strip_path, and will remove outside use after this PR goes to main.

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.

2 participants