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

Fixed support for env files #439

Merged
merged 6 commits into from
Apr 6, 2023
Merged

Conversation

Tiggax
Copy link
Contributor

@Tiggax Tiggax commented Apr 6, 2023

This pull adds support to activate environmental directories referenced in issue #401.

Currently the script works with directories passed as argument, but it doesn't autocomplete them.
might add that later on.

@Tiggax Tiggax requested a review from Hofer-Julian as a code owner April 6, 2023 08:45
Copy link
Collaborator

@Hofer-Julian Hofer-Julian left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me

virtual_environments/conda.nu Outdated Show resolved Hide resolved
@Tiggax
Copy link
Contributor Author

Tiggax commented Apr 6, 2023

This pull now has added autocomplete for venv folders, but the function is getting really slow.

@Hofer-Julian
Copy link
Collaborator

This pull now has added autocomplete for venv folders, but the function is getting really slow.

Always, or only when trying to auto complete?

@Tiggax
Copy link
Contributor Author

Tiggax commented Apr 6, 2023

the script calls conda multiple times during its run. in the past this resulted in a 1-3 sec wait time after running conda activate <env> or autocompletion, but is getting longer.
for autocompletion i didn't know how to solve it, since the names are parsed from raw output of conda info --envs but folders in path are checked using conda info --json | form json. Multiple functions call conda info to fetch information about the instance, but the call takes almost a second to complete, which results in long conda activation times.

I've been wandering to maybe refactor the script to improve its speed, or maybe move to nu_conda.nu.

TL;DR only takes longer when trying to autocomplete

@Hofer-Julian
Copy link
Collaborator

I've been wandering to maybe refactor the script to improve its speed, or maybe move to nu_conda.nu.

That would be much appreciated. Within this PR or in a follow up is then up to you

TL;DR only takes longer when trying to autocomplete

👍

@Tiggax
Copy link
Contributor Author

Tiggax commented Apr 6, 2023

Within this PR or in a follow up is then up to you

Possibly in a follow up PR

@Hofer-Julian
Copy link
Collaborator

My feeling is that we should remove autocompletion of paths for now and try again after we refactored/sped up the script.

Copy link
Collaborator

@Hofer-Julian Hofer-Julian left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you!

@Hofer-Julian Hofer-Julian merged commit e5e171c into nushell:main Apr 6, 2023
@Tiggax Tiggax deleted the unnamed_envs_support branch May 19, 2023 18:40
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.

2 participants