Skip to content

Conversation

gcurtis
Copy link
Collaborator

@gcurtis gcurtis commented Feb 16, 2023

Summary

This command prints shell commands that add Devbox packages to the user's PATH. The output is intended to be evaluated in the user's shell rcfile:

echo 'eval "$(devbox global shellenv)"' >> ~/.zshrc

We don't expand the environment variables in the generated commands so that it's clear to the user what the commands are doing.

How was it tested?

Ran eval $(devbox global shellenv) with PATH and XDG_DATA_HOME set to various values.

This command prints shell commands that add Devbox packages to the
user's PATH. The output is intended to be evaluated in the user's shell
rcfile:

	echo 'eval "$(devbox global shellenv)"' >> ~/.zshrc

We don't expand the environment variables in the generated commands so
that it's clear to the user what the commands are doing.
Copy link
Contributor

@loreto loreto left a comment

Choose a reason for hiding this comment

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

LGTM

@gcurtis gcurtis merged commit 35752b3 into main Feb 16, 2023
@gcurtis gcurtis deleted the gcurtis/shellenv branch February 16, 2023 20:00
Copy link
Contributor

@mikeland73 mikeland73 left a comment

Choose a reason for hiding this comment

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

This is cool! My main comment is that maybe we should rename this to devbox activate so it can be more generic. Also we could add a flag that let's the user run this command manually to refresh their shell.

// evaluated twice (once by the "parent" shell and once by the devbox
// shell). Prevent this by making the eval a no-op when we're already
// inside a devbox shell.
if IsDevboxShellEnabled() {
Copy link
Contributor

Choose a reason for hiding this comment

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

This means that if a package is installed while in shell, there's not way for the user to update the current shell to make it work. They would have to quit the shell, add the eval statement to their rc files and start it again.

Maybe we add

devbox activate --force (or --refresh) or something like that? So we can let the user force the configuration while they are in a devbox shell.

Copy link
Contributor

Choose a reason for hiding this comment

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

Another alternative is to have GenerateShellEnv set a new env variable _DEBOX_SHELL_ENV_SET=1 which is independent of shell. This lets apply this configuration only in cases it hasn't been applied.

We do still run into the issue @ipince brought up that we don't want global packages to supersede local ones. Maybe we splice in the new path instead?

Comment on lines +103 to +110
func globalShellenvCmd() *cobra.Command {
return &cobra.Command{
Use: "shellenv",
Short: "Print shell commands that add Devbox packages to your PATH",
Run: func(*cobra.Command, []string) {
fmt.Print(impl.GenerateShellEnv())
},
}
Copy link
Contributor

Choose a reason for hiding this comment

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

@loreto had suggested using devbox activate for this. I like that a bit more because then it's not related to global but actually a more general command that let's us modify and improve rc configuration over time. Thoughts?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants