Fall back for older PSReadLine ReadLine overloads#30
Conversation
|
Given the minimum requirement of PowerShell 7.4 (= PSReadLine 2.3.4), do we still need this shim? |
|
Yeah, fair question. I think the 7.4 floor is for PSNativeCommandPreserveBytePipe rather than PSReadLine itself. This wasn't meant as support for pwsh < 7.4. The issue seems to be that a supported pwsh can still load an older user-installed PSReadLine instead of the bundled one. The reporter said they're on pwsh 7.6.2, but PSReadLine 2.1.0 is what gets loaded. So if the intended support policy is really "pwsh 7.4+ with its bundled PSReadLine", I'm fine closing this as an unsupported environment. Otherwise this guard seems like a small defensive fix for a pretty bad failure mode. Also, is the 7.4+ requirement mainly for PSNativeCommandPreserveBytePipe? If yes, I get why that's needed for full correctness. I was only wondering whether older PowerShell could still have a best-effort mode, since many Windows sandboxes still only have Windows PowerShell 5.1. |
|
I think it may be best to check for the PSReadLine version in the installer and abort it if it doesn't meet the requirement. People may not be aware that they're on an older version (because they have installed it as a local module years ago) and continue to miss out on important updates for years to come, without ever realizing it. |
That makes sense. I can change this into an installer-side PSReadLine version check and fail the PowerShell integration with a clearer message. One caveat is AllUsers installs: the installer can only check the module resolution for the account running it, while another user may still have an older user-scoped PSReadLine. So an installer check catches the common case, but a small runtime guard would still prevent the profile hook from breaking another user's prompt. |
Oh, that's a great point. Let's keep the guard in then! I'll give it another thought soon. (I'm currently swamped with work, so I'll take a short break from this project though.) |
|
Sounds good, thanks. I'll keep the guard as-is for now. |
| 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." | ||
| } | ||
| } |
There was a problem hiding this comment.
I don't think we'll need this right? The installer now checks the PSReadLine version, so this is redundant?
2 participants
Fixes #20.
The injected
PSConsoleHostReadLineoverride calls the 3-argumentPSConsoleReadLine.ReadLine(..., lastRunStatus)overload. Older PSReadLine versions do not have that overload; their 3-argument overload takes aCancellationToken, so the call throws on every prompt read.This now checks the resolved PSReadLine version during PowerShell profile installation and fails the integration with a clear message when it is older than 2.3.4, matching the PSReadLine version bundled with PowerShell 7.4.
The profile snippet also detects the
Nullable[bool]overload once at load time and falls back to the 2-argumentReadLineoverload when it is not available. That keeps the hook from breaking another user's prompt in cases the installer cannot fully inspect, such as all-users installs with user-scoped PSReadLine modules.Validated locally by:
src/pwsh-install.ps1with PowerShell's parserpwsh -NoProfile -NonInteractiveand checking the resolved version/path