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

Add support of Nushell to venv activation #19359

Closed
karthiknadig opened this issue Jun 24, 2022 · 8 comments · Fixed by #20842
Closed

Add support of Nushell to venv activation #19359

karthiknadig opened this issue Jun 24, 2022 · 8 comments · Fixed by #20842
Assignees
Labels
author-verification-requested Issues potentially verifiable by issue author community ask Feature request that the community expressed interest in feature-request Request for new features or functionality needs proposal Need to make some design decisions verification-needed Verification of issue is requested verified Verification succeeded
Milestone

Comments

@karthiknadig
Copy link
Member

I'm experiencing something similar. With Nushell, a new terminal instance tries to activate the selected venv with
& c:/Users/ehawman/Documents/Programming/Python/PA-FindReplace/.venv/Scripts/Activate.ps1
image

The correct command here should be
source c:/Users/ehawman/Documents/Programming/Python/PA-FindReplace/.venv/Scripts/activate.nu

Weirdly, when actually using pwsh it truncates the activation command:
sers/ehawman/Documents/Programming/Python/PA-FindReplace/.venv/Scripts/Activate.ps1, causing it to fail.
image

Do you think this falls under the same umbrella or should I start a new issue?

Originally posted by @ehawman in #19285 (comment)

@karthiknadig karthiknadig added the feature-request Request for new features or functionality label Jun 24, 2022
@github-actions github-actions bot added the triage-needed Needs assignment to the proper sub-team label Jun 24, 2022
@ehawman
Copy link

ehawman commented Jun 24, 2022

@karthiknadig thank you! I was just scouring the issue list to see if a request existed. I vaguely recalled seeing one but I think that was for a different shell.

See #8341 and #8575

It looks like virtualenv creates activation scripts for:

  • Bash
  • CMD
  • Fish
  • Nu
  • PowerShell
  • Python(?)

image

@karrtikr mentioned using VSCode API to detect shells in this post. Is it possible to expand the scope of this feature request to a general "support all shells supported by virtualenv", and gracefully tell the user that their shell isn't supported if it isn't?

@karthiknadig karthiknadig added the needs community feedback Awaiting community feedback label Jun 27, 2022
@karthiknadig
Copy link
Member Author

Thanks for the feature request! We are going to give the community 60 days from when this issue was created to provide 7 👍 upvotes on the opening comment to gauge general interest in this idea. If there's enough upvotes then we will consider this feature request in our future planning. If there's unfortunately not enough upvotes then we will close this issue.

@karthiknadig karthiknadig removed the triage-needed Needs assignment to the proper sub-team label Jun 28, 2022
@phiresky
Copy link

phiresky commented Jul 12, 2022

For me (on Linux) it pastes the path of bash activation script (.venv/bin/activate) into nushell but doesn't actually execute it

@brettcannon brettcannon added needs proposal Need to make some design decisions community ask Feature request that the community expressed interest in and removed needs community feedback Awaiting community feedback labels Aug 25, 2022
@brettcannon
Copy link
Member

Thank you to everyone who upvoted this issue! Since the community showed interest in this feature request we will leave this issue open as something to consider implementing at some point in the future.

We do encourage people to continue 👍 the first/opening comment as it helps us prioritize our work based on what the community seems to want the most.

@flying-sheep
Copy link

flying-sheep commented Jan 6, 2023

Should be pretty easy:

  1. Copy the Bash class and rename the copy to Nushell:
    @injectable()
    export class Bash extends VenvBaseActivationCommandProvider {
    protected readonly scripts = SCRIPTS;
    public async getActivationCommandsForInterpreter(
    pythonPath: string,
    targetShell: TerminalShellType
    ): Promise<string[] | undefined> {
    const scriptFile = await this.findScriptFile(pythonPath, targetShell);
    if (!scriptFile) {
    return;
    }
    return [`source ${scriptFile.fileToCommandArgument()}`];
    }
    }
  2. modify it to find the file activate.nu
  3. make it return [`overlay activate ${scriptFile.fileToCommandArgument()}`]
  4. inject a private readonly nushell like here:
    @inject(ITerminalActivationCommandProvider)
    @named(TerminalActivationProviders.bashCShellFish)
    private readonly bashCShellFish: ITerminalActivationCommandProvider,
    and add it to this list:
    const providers = [this.pipenv, this.pyenv, this.bashCShellFish, this.commandPromptAndPowerShell];

@karrtikr
Copy link

FYI we do not support detecting Nushell as well, so in addition to the above a support will need to be added here:

const detectableShells = new Map<TerminalShellType, RegExp>();
detectableShells.set(TerminalShellType.powershell, IS_POWERSHELL);
detectableShells.set(TerminalShellType.gitbash, IS_GITBASH);
detectableShells.set(TerminalShellType.bash, IS_BASH);
detectableShells.set(TerminalShellType.wsl, IS_WSL);
detectableShells.set(TerminalShellType.zsh, IS_ZSH);
detectableShells.set(TerminalShellType.ksh, IS_KSH);
detectableShells.set(TerminalShellType.commandPrompt, IS_COMMAND);
detectableShells.set(TerminalShellType.fish, IS_FISH);
detectableShells.set(TerminalShellType.tcshell, IS_TCSHELL);
detectableShells.set(TerminalShellType.cshell, IS_CSHELL);
detectableShells.set(TerminalShellType.powershellCore, IS_POWERSHELL_CORE);
detectableShells.set(TerminalShellType.xonsh, IS_XONSH);

@karrtikr karrtikr changed the title Add support of Nushell Add support of Nushell to venv activation Mar 31, 2023
@karrtikr karrtikr added this to the April 2023 milestone Mar 31, 2023
karrtikr pushed a commit that referenced this issue Mar 31, 2023
Fixes #19359

---------

Co-authored-by: Kartik Raj <karraj@microsoft.com>
@karrtikr
Copy link

Fix should be out in the pre-release version of the extension in a day, use the following to try it out:

image

@karrtikr karrtikr added verification-needed Verification of issue is requested author-verification-requested Issues potentially verifiable by issue author labels Mar 31, 2023
@flying-sheep
Copy link

Great! I did that, opened a Python project, and it worked!

@karrtikr karrtikr added the verified Verification succeeded label Apr 3, 2023
@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 4, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
author-verification-requested Issues potentially verifiable by issue author community ask Feature request that the community expressed interest in feature-request Request for new features or functionality needs proposal Need to make some design decisions verification-needed Verification of issue is requested verified Verification succeeded
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants