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: run scripts in noninteractive shell (#1547) #1577

Conversation

xudyang1
Copy link
Contributor

@xudyang1 xudyang1 commented Jan 20, 2024

Firenvim on Windows relys on powershell scripts to run registry keys installation and uninstallation. Here are the codes:

let o = s:maybe_execute('system', ['powershell.exe', '-Command', '-'], readfile(l:ps1_path))

let o = system(['powershell.exe', '-Command', '-'], [l:ps1_content])

During debugging, I add echo o right after calling the system function, which then shows error

Cannot load PSReadline module.  Console is running without PSReadline.

But weirdly, v:shell_error is still 0. PSReadline is a module usually installed for interactive prompt usage. Powershell.exe provides option -NonInterative for running automated scripts in non-interactive shell. Adding the -NonInteractive option fixes #1547 as the scripts run successfully without error.

Note

This pull request only fixes firenvim installation on Windows host and
WSL2 environment. Uninstallation scripts run successfully when running firenvim on the host (%LOCALAPPDATA/firenvim/* files and corresponding registry keys are removed); run firenvim#uninstall() on WSL side only removes the binary in WSL
(stdpath('data')/firenvim in WSL or
$HOME/.local/share/firenvim/firenvim)

A separate issue #1576 tracks the uninstallation bug on WSL side

Firenvim on Windows relys on powershell scripts to run registry keys installation and
uninstallation. Here are the codes:

https://github.com/glacambre/firenvim/blob/8c6c00aae7e5762cbcb4cd0df5848e959c4a9572/autoload/firenvim.vim#L890

https://github.com/glacambre/firenvim/blob/8c6c00aae7e5762cbcb4cd0df5848e959c4a9572/autoload/firenvim.vim#L958

During debugging, I add `echo o` right after calling the `system`
function, the output shows error

```text
Cannot load PSReadline module.  Console is running without PSReadline.
```

But weirdly, `v:shell_error` is still `0`. `PSReadline` is a module
usually installed for interactive prompt usage. `Powershell.exe`
provides option `-NonInterative` for running automated scripts in
non-interactive shell. Adding the `-NonInteractive` option fixes glacambre#1547
as the scripts run successfully without error.

> [!NOTE]
> This pull request only fixes firenvim installation on Windows host and
WSL2 environment. Uninstallation scripts run successfully when running
nvim on the host (`%LOCALAPPDATA/firenvim/*` files and
corresponding registry keys are removed); run `firenvim#uninstall()` on
WSL side only removes the binary in WSL
(`stdpath('data')/firenvim` in WSL or
`$HOME/.local/share/firenvim/firenvim`)
>
> A separate issue glacambre#1576 tracks the uninstallation bug on WSL side
@glacambre
Copy link
Owner

Wow, great debugging skills, thank you for taking the time to open a PR! :)

@glacambre glacambre merged commit 6a034a0 into glacambre:master Jan 20, 2024
15 of 18 checks passed
xudyang1 added a commit to xudyang1/firenvim that referenced this pull request Jul 27, 2024
This PR is a continuation of glacambre#1199 and glacambre#1577. glacambre#1199 adds support for
running `powershell.exe` when it's not in the path (this occurs when
WSL user sets `appendWindowsPath=false` in `/etc/wsl.conf`). glacambre#1577 adds
`-NonInteractive` option to `powershell.exe` commands.
glacambre pushed a commit that referenced this pull request Jul 29, 2024
This PR is a continuation of #1199 and #1577. #1199 adds support for
running `powershell.exe` when it's not in the path (this occurs when
WSL user sets `appendWindowsPath=false` in `/etc/wsl.conf`). #1577 adds
`-NonInteractive` option to `powershell.exe` commands.
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.

Unable to run firenvim in Windows 10 Host System
2 participants