Skip to content

fix: do not use dynamic module in post-script for PowerShell - #254

Merged
jasongin merged 1 commit into
jasongin:masterfrom
lewis-yeung:no-dynamic-module
Dec 6, 2022
Merged

fix: do not use dynamic module in post-script for PowerShell#254
jasongin merged 1 commit into
jasongin:masterfrom
lewis-yeung:no-dynamic-module

Conversation

@lewis-yeung

@lewis-yeung lewis-yeung commented Jul 27, 2022

Copy link
Copy Markdown

Currently, the dynamic module used in a post-script for PowerShell exports a prompt function, which overrides the original. It also uses an environment variable NVS_ORIGINAL_PROMPT to store the entire stringified original prompt function. However, there is a flaw: if the original prompt is defined inside a dynamic module as well, functions defined in that module will become inaccessible for $env:NVS_ORIGINAL_PROMPT, which is invoked by the NVS-defined prompt. This is caused by some "isolation" features of PowerShell modules. Related code:

nvs/lib/auto.js

Lines 120 to 123 in bf45ef0

'New-Module -Script {',
' function prompt { . "' + psScriptFile + '" "prompt" }',
' Export-ModuleMember -Function prompt',
'} > $null',

Related issue: JanDeDobbeleer/oh-my-posh#2568.

Since NVS is not a tool of prompt customization, it should respect the user's custom prompt function. As an alternative, we can save the original prompt in a global variable when we use nvs auto on to enable the auto-switching so that it can be invoked/restored correctly. Also, we have to store the original $? in another global variable which is accessible for the initialization script of a prompt customization tool and restore the original $global:LASTEXITCODE after the auto-switching done so that the original prompt function can get the correct error code.

❤️ Special thanks to @possum-enjoyer for the help and comments in #253.

@brian-wittenberg-relativity

Copy link
Copy Markdown

I can confirm this is working, as I've been using it for several weeks since I use posh-git and out of the box NVS was breaking my prompt when using nvs auto on.

@jasongin
jasongin merged commit 83ebf42 into jasongin:master Dec 6, 2022
@jasongin

jasongin commented Dec 6, 2022

Copy link
Copy Markdown
Owner

Thanks for the contribution! And sorry for the delay in merging.

@lewis-yeung
lewis-yeung deleted the no-dynamic-module branch December 7, 2022 02:59
@jasongin

jasongin commented Dec 7, 2022

Copy link
Copy Markdown
Owner

Released as nvs v1.7.0.

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.

3 participants