Improve Windows custom folder installation instructions - #202
Conversation
📝 WalkthroughWalkthroughThe README now provides expanded Windows instructions for custom-folder installation, existing installation removal, separate Command Prompt and PowerShell commands, and user PATH configuration. ChangesWindows installation documentation
Estimated code review effort: 1 (Trivial) | ~2 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #202 +/- ##
==========================================
- Coverage 98.47% 98.05% -0.42%
==========================================
Files 87 133 +46
Lines 8637 13669 +5032
==========================================
+ Hits 8505 13403 +4898
- Misses 132 266 +134 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@README.md`:
- Around line 142-146: Update the PowerShell PATH instructions to read the
existing persistent User PATH rather than `$env:PATH`, append
`C:\REPLACE_ME\sapcli` only if it is not already present, and persist that
User-scoped value. Also update the current `$env:Path` for immediate
availability or explicitly instruct users to open a new shell.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
| Afterwards, make sure `C:\REPLACE_ME\sapcli` is on your PATH by adding it to the | ||
| user environment variables in the Windows system settings, or from PowerShell: | ||
|
|
||
| ```powershell | ||
| [System.Environment]::SetEnvironmentVariable("PATH", $env:PATH + ";C:\REPLACE_ME\sapcli", "User") |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf 'README excerpt:\n'
sed -n '135,150p' README.md || true
printf '\nRelevant repository Windows instructions:\n'
rg -n "REPLACE_ME|SetEnvironmentVariable|current PowerShell|new shell|sapcli" README.md || true
printf '\nPowerShell availability and environment behavior probe:\n'
if command -v pwsh >/dev/null 2>&1; then
pwsh -NoProfile -Command '
Set-StrictMode -Version Latest
$tempBase = [Environment]::GetFolderPath("LocalApplicationData")
$tmp = New-Guid | Out-String -Trim
$userBase = Join-Path $tempBase "PowShellEnvProbe-$tmp"
$machineBase = Join-Path $tempBase "PowerShellEnvProbeMachine-$tmp"
try {
New-Item -ItemType Directory -Path $userBase -Force | Out-Null
New-Item -ItemType Directory -Path $machineBase -Force | Out-Null
[Environment]::SetEnvironmentVariable("UserProbePath", $userBase, "User") | Out-Null
[Environment]::SetEnvironmentVariable("MachineProbePath", $machineBase, "User") | Out-Null
$env:UserProbePath, $env:MachineProbePath = $userBase, $machineBase
$processPath = [Environment]::GetEnvironmentVariable("Path", "Process")
$userPath = [Environment]::GetEnvironmentVariable("Path", "User")
$machinePath = [Environment]::GetEnvironmentVariable("Path", "Machine")
Write-Host '"PROCESS_CONTAINS_USER:"' ($processPath -split ";" -contains $userBase)
Write-Host '"PROCESS_CONTAINS_MACHINE:"' ($processPath -split ";" -contains $machineBase)
Write-Host '"USER_SCOPE:"' $userPath
Write-Host '"MACHINE_SCOPE:"' $machinePath
Write-Host '"CURRENT_SESSION_UPDATE_NEEDED:"' ($processPath -split ";" -notcontains $userBase)
} finally {
[Environment]::SetEnvironmentVariable("UserProbePath", $null, "User")
[Environment]::SetEnvironmentVariable("MachineProbePath", $null, "User")
}
'
else
echo pwsh not available
fi
printf '\nCSharp/.NET behavior probe if dotnet available:\n'
if command -v dotnet >/dev/null 2>&1; then
tmpdir="$(mktemp -d)"
cat > "$tmpdir/Probe.csproj" <<'EOF'
<Project Sdk="Microsoft.NET.Sdk">
<PropertyGroup>
<OutputType>Exe</OutputType>
<TargetFramework>net8.0</TargetFramework>
<ImplicitUsings>enable</ImplicitUsings>
<Nullable>enable</Nullable>
</PropertyGroup>
</Project>
EOF
cat > "$tmpdir/Program.cs" <<'EOF'
using System;
using System.Linq;
try
{
var user = Environment.GetEnvironmentVariable("Path", EnvironmentVariableTarget.User);
var machine = Environment.GetEnvironmentVariable("Path", EnvironmentVariableTarget.Machine);
var process = Environment.GetEnvironmentVariable("Path", EnvironmentVariableTarget.Process);
Console.WriteLine("USER_CONTAINS_MACHINE:" + machine.Split(';', StringSplitOptions.RemoveEmptyEntries).Intersect(user.Split(';', StringSplitOptions.RemoveEmptyEntries)).Any());
Console.WriteLine("PROCESS_CONTAINS_MACHINE:" + machine.Split(';', StringSplitOptions.RemoveEmptyEntries).Any(process.Split(';', StringSplitOptions.RemoveEmptyEntries).Contains));
}
catch (Exception ex)
{
Console.WriteLine($"ERROR:{ex.GetType().FullName}:{ex.Message}");
Environment.Exit(1);
}
EOF
cd "$tmpdir"
dotnet build --nologo -o build >/tmp/probe_build.txt 2>&1 && dotnet run --nologo --no-build
else
echo dotnet not available
fiRepository: jfilak/sapcli
Length of output: 5174
🌐 Web query:
Microsoft PowerShell about_environment_variables GetEnvironmentVariable SetEnvironmentVariable User Machine Process scope
💡 Result:
In PowerShell, environment variables can be managed using the Env: drive or the.NET System.Environment class [1][2][3]. While the Env: drive is convenient for session-based changes, the System.Environment class provides the necessary control for persistent modifications across Windows scopes [1][4][5]. Session-Based Management ($env:) The $env: prefix allows you to get, set, or remove environment variables for the current PowerShell process [1][3]. Changes made this way are volatile, meaning they exist only for the duration of the current session [1][3][5]. Example: $env:MY_VAR = 'Value' Persistent Management (System.Environment) To create or modify environment variables that persist across sessions on Windows, use the System.Environment class [1][5]. These methods accept an EnvironmentVariableTarget to specify the scope [4][6]. The System.Environment.SetEnvironmentVariable and GetEnvironmentVariable methods use the following target scopes: 1. Process: Stores or retrieves the variable only in the current process (equivalent to using $env:) [6][5]. 2. User: Stores or retrieves the variable in the HKEY_CURRENT_USER\Environment registry key [6][7]. 3. Machine: Stores or retrieves the variable in the HKEY_LOCAL_MACHINE\System\CurrentControlSet\Control\Session Manager\Environment registry key [6][7]. This typically requires elevated (administrator) privileges [7][8]. Usage Example: [System.Environment]::SetEnvironmentVariable('MY_VAR', 'PersistentValue', 'User') Important Considerations: - Persistence and Scope: Changes to User or Machine scopes are not immediately reflected in the current process; they are typically available to new processes started after the change [6][7]. If you need the change to take effect immediately in the current session, you should update both the persistent scope and the current process scope [7][5]. - Precedence: When resolving environment variables, Windows generally follows a hierarchy: Process > User > Machine [5]. If a variable with the same name exists in multiple scopes, the Process value takes precedence [5]. - Cross-Platform Limitations: The User and Machine targets are specific to Windows [4][9]. On macOS and Linux, environment variables are generally managed within the process scope, and attempting to use User or Machine targets in.NET methods will return null or fail [4][9].
Citations:
- 1: https://learn.microsoft.com/en-us/powershell/module/microsoft.powershell.core/about/about_environment_variables?view=powershell-7.6
- 2: https://learn.microsoft.com/en-us/powershell/scripting/lang-spec/chapter-03?view=powershell-7.4
- 3: https://learn.microsoft.com/en-us/powershell/module/microsoft.powershell.core/about/about_environment_provider?view=powershell-7.6
- 4: https://learn.microsoft.com/en-us/dotnet/api/system.environment.setenvironmentvariable?view=net-10.0
- 5: https://envmanager.com/blog/environment-variables-powershell
- 6: https://learn.microsoft.com/en-us/dotnet/api/system.environmentvariabletarget?view=netframework-4.8.1
- 7: https://env.dev/guides/windows-environment-variables
- 8: https://adamtheautomator.com/powershell-environment-variables/
- 9: https://learn.microsoft.com/en-us/dotnet/api/system.environment.getenvironmentvariable?view=net-9.0
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf 'README excerpt around PATH instructions:\n'
sed -n '115,152p' README.md
printf '\nPowerShell availability:\n'
command -v pwsh >/dev/null 2>&1 && echo pwsh_found || echo pwsh_not_found
printf '\n.NET probe if dotnet available:\n'
if command -v dotnet >/dev/null 2>&1; then
tmpdir="$(mktemp -d)"
cat > "$tmpdir/Probe.csproj" <<'EOF'
<Project Sdk="Microsoft.NET.Sdk">
<PropertyGroup>
<OutputType>Exe</OutputType>
<TargetFramework>net8.0</TargetFramework>
<ImplicitUsings>enable</ImplicitUsings>
<Nullable>enable</Nullable>
</PropertyGroup>
</Project>
EOF
cat > "$tmpdir/Program.cs" <<'EOF'
using System;
using System.Linq;
class Program
{
static void Main()
{
var user = Environment.GetEnvironmentVariable("Path", EnvironmentVariableTarget.User) ?? "";
var machine = Environment.GetEnvironmentVariable("Path", EnvironmentVariableTarget.Machine) ?? "";
var process = Environment.GetEnvironmentVariable("Path", EnvironmentVariableTarget.Process) ?? "";
var machineEntries = machine.Split(';', StringSplitOptions.RemoveEmptyEntries);
var userEntries = user.Split(';', StringSplitOptions.RemoveEmptyEntries);
var processEntries = process.Split(';', StringSplitOptions.RemoveEmptyEntries);
Console.WriteLine("USER_SCOPE_EMPTY:" + string.IsNullOrEmpty(user));
Console.WriteLine("USER_CONTAINS_MACHINE_ENTRIES:" + string.Join(",", machineEntries.Where(machineEntries.Contains)));
Console.WriteLine("MACHINE_SCOPE_EMPTY:" + string.IsNullOrEmpty(machine));
Console.WriteLine("MACHINE_CONTAINS_USER_ENTRIES:" + string.Join(",", userEntries.Where(userEntries.Contains)));
Console.WriteLine("PROCESS_CONTAINS_MACHINE_ENTRIES:" + string.Join(",", machineEntries.Where(processEntries.Contains)));
Console.WriteLine("PROCESS_CONTAINS_USER_ENTRIES:" + string.Join(",", userEntries.Where(processEntries.Contains)));
}
}
EOF
cd "$tmpdir"
dotnet run --nologo
else
echo dotnet_not_found
fiRepository: jfilak/sapcli
Length of output: 1517
Keep the User PATH and current session PATH separate.
The existing user PATH may already exclude Machine PATH entries, but $env:PATH is the current process PATH after both scopes are combined. Persisting $env:PATH copies the existing Machine-path entries into User scope. Use the persistent User scope as the source, add the custom directory once, then also update $env:Path or tell users to start a new shell so sapcli is available immediately.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@README.md` around lines 142 - 146, Update the PowerShell PATH instructions to
read the existing persistent User PATH rather than `$env:PATH`, append
`C:\REPLACE_ME\sapcli` only if it is not already present, and persist that
User-scoped value. Also update the current `$env:Path` for immediate
availability or explicitly instruct users to open a new shell.
1 participant
No description provided.