Skip to content

Conversation

mohsenari
Copy link
Collaborator

Summary

In shellPath() function where we decide which type of shell to use when dropping user in devbox shell, if we can't figure out the shell, we drop the user in the default bash that nix installs in store. But this bash doesn't handle arrow keys and keyboard combinations very well, so it creates a frustrating experience for users (including pure shell users). bashInteractive package has all these capabilities and makes the experience better for a 'default' shell.

So this PR changes this:

  • nix eval <bash> to get store path and use to create default shell
    to this:
  • nix eval <bashInteractive> to get store path
  • nix build
  • use store path to create default shell

Addresses #1193

How was it tested?

  • compile
  • ./devbox shell --pure
    OR
  • unset $SHELL
  • ./devbox shell

@mohsenari mohsenari requested a review from mikeland73 June 28, 2023 00:44
Comment on lines +124 to +129
cmd = exec.Command("nix", "build", bashNixStorePath, "--no-link")
cmd.Args = append(cmd.Args, nix.ExperimentalFlags()...)
err = cmd.Run()
if err != nil {
return "", errors.WithStack(err)
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Thoughts on using devbox.addDevboxUtilityPackage instead? It installs in a specific profile. (not sure if that works with bashNixStorePath)

Copy link
Collaborator Author

@mohsenari mohsenari Jun 28, 2023

Choose a reason for hiding this comment

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

@mikeland73 I tried and it works, I had to pass just the package name (bashInteractive) but the issue is it shows bashInteractive: success every time you go to a pure shell.
see:

➜  shellpure2 ./devbox shell --pure
Starting a devbox shell...
bashInteractive: Success
Welcome to devbox!
(devbox) Mohsen-Ansaris-MacBook-Pro:shellpure2 mohsenansari$

I spoke with John and the preference is to not show the install message every time and only show it if we're installing for the first time to explain the delay in shell start time. Which I can do a follow up PR for that.

@mohsenari mohsenari merged commit 7648993 into main Jun 28, 2023
@mohsenari mohsenari deleted the mohsen--bash-interactive branch June 28, 2023 20:09
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.

2 participants