Skip to content
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.

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

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

Merged
merged 1 commit into from
Dec 6, 2022

Conversation

lewis-yeung
Copy link
Contributor

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

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

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
Copy link
Owner

jasongin commented Dec 6, 2022

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
Copy link
Owner

jasongin commented Dec 7, 2022

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