Skip to content

Check PSReadLine version during PowerShell integration install #30

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.

Sign up for GitHub

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

Open
caomengxuan666 wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Check PSReadLine version during PowerShell integration install #30

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
26d4a2d
Fall back for older PSReadLine ReadLine overloads
caomengxuan666 Jun 3, 2026
7f43fec
Check PSReadLine version during pwsh install
caomengxuan666 Jun 3, 2026
7ab6738
Remove PSReadLine runtime fallback
caomengxuan666 Jun 9, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 22 additions & 0 deletions src/pwsh-install.ps1
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,9 @@ $SectionMarker = '60b36fc6-2d59-49df-be51-28dd2f4c3c9a'
$MarkerLine = "# DO NOT MODIFY -- coreutils -- $SectionMarker"
# Earliest PowerShell that supports PSNativeCommandPreserveBytePipe.
$MinPwshVersion = [version]'7.4.0'
# PowerShell 7.4 ships PSReadLine 2.3.4, which has the
# ReadLine(Runspace, EngineIntrinsics, bool?) overload used by the profile hook.
$MinPSReadLineVersion = [version]'2.3.4'
# Contains SID --> Microsoft.PowerShell_profile.ps1 mappins,
# such that we can clean them up on uninstall.
$ProfilesRegPath = 'HKLM:\SOFTWARE\Microsoft\coreutils\PowerShellProfiles'
Expand All @@ -34,6 +37,24 @@ function Remove-FileIfExists([string]$Path) {
}
}

function Assert-PSReadLineRequirement {
try {
$module = Import-Module PSReadLine -PassThru -ErrorAction Stop | Select-Object -First 1
}
catch {
throw "PowerShell integration requires PSReadLine $MinPSReadLineVersion or newer, but PSReadLine could not be loaded: $($_.Exception.Message)"
}

if (!$module -or !$module.Version) {
throw "PowerShell integration requires PSReadLine $MinPSReadLineVersion or newer, but the resolved PSReadLine module did not report a version"
}

if ($module.Version -lt $MinPSReadLineVersion) {
$path = if ($module.Path) { " from '$($module.Path)'" } else { '' }
throw "PowerShell integration requires PSReadLine $MinPSReadLineVersion or newer, but PowerShell resolved PSReadLine $($module.Version)$path. Update or remove the older PSReadLine module and re-run the installer."
}
}
Comment on lines +40 to +56

@lhecker lhecker Jun 8, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I don't think we'll need this right? The installer now checks the PSReadLine version, so this is redundant?

@caomengxuan666 caomengxuan666 Jun 9, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Agreed. I removed the runtime overload fallback and kept the installer-side PSReadLine version check as the single guard.

I also updated the PR title/body so it no longer claims to fall back at runtime.

@lhecker lhecker Jun 9, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

But it's this, the Assert-PSReadLineRequirement check, that I don't think serves a purpose. Am I missing something?

@caomengxuan666 caomengxuan666 Jun 9, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Ah, got it.

I was thinking about the #20 case, where the user had a supported PowerShell but an older PSReadLine was still resolved first.

So this check was meant to fail earlier with a clearer message before installing the hook, not to support old PowerShell.

But maybe that is too defensive. If you prefer treating it as an environment issue, I’m fine closing this PR.


function Get-InjectedSection([string]$CmdDir) {
$templatePath = Join-Path $PSScriptRoot 'pwsh-install-template.ps1'
$template = Get-Content -LiteralPath $templatePath -Raw
Expand Down Expand Up @@ -243,6 +264,7 @@ function Get-ProfilePlan([bool] $Install, [string]$Scope) {
$install = $Action -eq 'Install'
$plan = @(Get-ProfilePlan $install $Scope)
$section = if ($install) {
Assert-PSReadLineRequirement
Get-InjectedSection $CmdDir
}
else {
Expand Down