Skip to content

[code-cleanup] Rename devbox.Writer to Stderr #1507

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.

Sign up for GitHub

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

Merged
merged 6 commits into from
Sep 27, 2023
Merged

[code-cleanup] Rename devbox.Writer to Stderr #1507

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
87073cd
[code-cleanup] Rename Writer to Stderr
mikeland73 Sep 27, 2023
64cbb86
rename print functions
mikeland73 Sep 27, 2023
5893444
Simplify interface
mikeland73 Sep 27, 2023
97bcfd2
Fix
mikeland73 Sep 27, 2023
55ee060
Requested changes
mikeland73 Sep 27, 2023
32bb47e
Fix test
mikeland73 Sep 27, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 16 additions & 21 deletions devbox.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,41 +16,36 @@ import (

// Devbox provides an isolated development environment.
type Devbox interface {
// Add adds Nix packages to the config so that they're available in the devbox
// environment. It validates that the Nix packages exist, and install them.
// Adding duplicate packages is a no-op.
Add(ctx context.Context, platforms, excludePlatforms []string, pkgs ...string) error
Config() *devconfig.Config
ProjectDir() string
// Generate creates the directory of Nix files and the Dockerfile that define
// the devbox environment.
Generate(ctx context.Context) error
GenerateDevcontainer(ctx context.Context, generateOpts devopt.GenerateOpts) error
GenerateDockerfile(ctx context.Context, generateOpts devopt.GenerateOpts) error
GenerateEnvrcFile(ctx context.Context, force bool, envFlags devopt.EnvFlags) error
Info(ctx context.Context, pkg string, markdown bool) error
EnvVars(ctx context.Context) ([]string, error)
Info(ctx context.Context, pkg string, markdown bool) (string, error)
Install(ctx context.Context) error
IsEnvEnabled() bool
ListScripts() []string
PrintEnv(ctx context.Context, includeHooks bool) (string, error)
PrintEnvVars(ctx context.Context) ([]string, error)
PrintGlobalList() error
NixEnv(ctx context.Context, includeHooks bool) (string, error)
PackageNames() []string
Copy link
Collaborator

Choose a reason for hiding this comment

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

Are these config PackageNames or installable PackageNames?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Just what is in config. This is a no-op (PrintGlobaList used to call PackageNames).

We can chat about whether this is correct or not (I think it's fine), but would rather not change it in this PR.

ProjectDir() string
Pull(ctx context.Context, opts devopt.PullboxOpts) error
Push(ctx context.Context, opts devopt.PullboxOpts) error
// Remove removes Nix packages from the config so that it no longer exists in
// the devbox environment.
Remove(ctx context.Context, pkgs ...string) error
RestartServices(ctx context.Context, services ...string) error
RunScript(ctx context.Context, scriptName string, scriptArgs []string) error
Services() (services.Services, error)
// Shell generates the devbox environment and launches nix-shell as a child process.
Shell(ctx context.Context) error
Update(ctx context.Context, opts devopt.UpdateOpts) error

// Interact with services
ListServices(ctx context.Context) error
RestartServices(ctx context.Context, services ...string) error
Services() (services.Services, error)
StartProcessManager(ctx context.Context, requestedServices []string, background bool, processComposeFileOrDir string) error
StartServices(ctx context.Context, services ...string) error
StopServices(ctx context.Context, allProjects bool, services ...string) error
ListServices(ctx context.Context) error

Update(ctx context.Context, opts devopt.UpdateOpts) error
// Generate files
Generate(ctx context.Context) error
GenerateDevcontainer(ctx context.Context, generateOpts devopt.GenerateOpts) error
GenerateDockerfile(ctx context.Context, generateOpts devopt.GenerateOpts) error
GenerateEnvrcFile(ctx context.Context, force bool, envFlags devopt.EnvFlags) error
}

// Open opens a devbox by reading the config file in dir.
Expand Down
2 changes: 1 addition & 1 deletion internal/boxcli/add.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ func addCmd() *cobra.Command {
func addCmdFunc(cmd *cobra.Command, args []string, flags addCmdFlags) error {
box, err := devbox.Open(&devopt.Opts{
Dir: flags.config.path,
Writer: cmd.ErrOrStderr(),
Stderr: cmd.ErrOrStderr(),
AllowInsecureAdds: flags.allowInsecure,
})
if err != nil {
Expand Down
4 changes: 2 additions & 2 deletions internal/boxcli/cloud.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,7 @@ func runCloudShellCmd(cmd *cobra.Command, flags *cloudShellCmdFlags) error {

box, err := devbox.Open(&devopt.Opts{
Dir: flags.config.path,
Writer: cmd.ErrOrStderr(),
Stderr: cmd.ErrOrStderr(),
})
if err != nil {
return errors.WithStack(err)
Expand All @@ -158,7 +158,7 @@ func runCloudInit(cmd *cobra.Command, flags *cloudShellCmdFlags) error {

box, err := devbox.Open(&devopt.Opts{
Dir: flags.config.path,
Writer: cmd.ErrOrStderr(),
Stderr: cmd.ErrOrStderr(),
})
if err != nil {
return errors.WithStack(err)
Expand Down
4 changes: 2 additions & 2 deletions internal/boxcli/generate.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,7 @@ func runGenerateCmd(cmd *cobra.Command, flags *generateCmdFlags) error {
// Check the directory exists.
box, err := devbox.Open(&devopt.Opts{
Dir: flags.config.path,
Writer: cmd.ErrOrStderr(),
Stderr: cmd.ErrOrStderr(),
})
if err != nil {
return errors.WithStack(err)
Expand Down Expand Up @@ -167,7 +167,7 @@ func runGenerateDirenvCmd(cmd *cobra.Command, flags *generateCmdFlags) error {

box, err := devbox.Open(&devopt.Opts{
Dir: flags.config.path,
Writer: cmd.ErrOrStderr(),
Stderr: cmd.ErrOrStderr(),
})
if err != nil {
return errors.WithStack(err)
Expand Down
9 changes: 6 additions & 3 deletions internal/boxcli/global.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,12 +68,15 @@ func listGlobalCmdFunc(cmd *cobra.Command, args []string) error {

box, err := devbox.Open(&devopt.Opts{
Dir: path,
Writer: cmd.OutOrStdout(),
Stderr: cmd.ErrOrStderr(),
})
if err != nil {
return errors.WithStack(err)
}
return box.PrintGlobalList()
for _, p := range box.PackageNames() {
fmt.Fprintf(cmd.OutOrStdout(), "* %s\n", p)
}
return nil
}

var globalConfigPath string
Expand Down Expand Up @@ -125,7 +128,7 @@ func ensureGlobalEnvEnabled(cmd *cobra.Command, args []string) error {

box, err := devbox.Open(&devopt.Opts{
Dir: path,
Writer: cmd.ErrOrStderr(),
Stderr: cmd.ErrOrStderr(),
})
if err != nil {
return err
Expand Down
9 changes: 7 additions & 2 deletions internal/boxcli/info.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,11 +36,16 @@ func infoCmd() *cobra.Command {
func infoCmdFunc(cmd *cobra.Command, pkg string, flags infoCmdFlags) error {
box, err := devbox.Open(&devopt.Opts{
Dir: flags.config.path,
Writer: cmd.OutOrStdout(),
Stderr: cmd.ErrOrStderr(),
})
if err != nil {
return errors.WithStack(err)
}

return box.Info(cmd.Context(), pkg, flags.markdown)
info, err := box.Info(cmd.Context(), pkg, flags.markdown)
if err != nil {
return errors.WithStack(err)
}
cmd.Print(info)
return nil
}
2 changes: 1 addition & 1 deletion internal/boxcli/install.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ func installCmdFunc(cmd *cobra.Command, flags runCmdFlags) error {
// Check the directory exists.
box, err := devbox.Open(&devopt.Opts{
Dir: flags.config.path,
Writer: cmd.ErrOrStderr(),
Stderr: cmd.ErrOrStderr(),
})
if err != nil {
return errors.WithStack(err)
Expand Down
4 changes: 2 additions & 2 deletions internal/boxcli/integrate.go
View file
Open in desktop
Copy link
Collaborator

Choose a reason for hiding this comment

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

In this case either works, but stdErr is logically correct. 👍

Original file line number Diff line number Diff line change
Expand Up @@ -75,13 +75,13 @@ func runIntegrateVSCodeCmd(cmd *cobra.Command) error {
// todo: add error handling - consider sending error message to parent process
box, err := devbox.Open(&devopt.Opts{
Dir: message.ConfigDir,
Writer: cmd.OutOrStdout(),
Stderr: cmd.ErrOrStderr(),
})
if err != nil {
return err
}
// Get env variables of a devbox shell
envVars, err := box.PrintEnvVars(cmd.Context())
envVars, err := box.EnvVars(cmd.Context())
if err != nil {
return err
}
Expand Down
2 changes: 1 addition & 1 deletion internal/boxcli/midcobra/telemetry.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ func getPackagesAndCommitHash(c *cobra.Command) ([]string, string) {

box, err := devbox.Open(&devopt.Opts{
Dir: path,
Writer: os.Stdout,
Stderr: os.Stderr,
IgnoreWarnings: true,
})
if err != nil {
Expand Down
2 changes: 1 addition & 1 deletion internal/boxcli/pull.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ func pullCmd() *cobra.Command {
func pullCmdFunc(cmd *cobra.Command, url string, flags *pullCmdFlags) error {
box, err := devbox.Open(&devopt.Opts{
Dir: flags.config.path,
Writer: cmd.ErrOrStderr(),
Stderr: cmd.ErrOrStderr(),
})
if err != nil {
return errors.WithStack(err)
Expand Down
2 changes: 1 addition & 1 deletion internal/boxcli/push.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ func pushCmd() *cobra.Command {
func pushCmdFunc(cmd *cobra.Command, url string, flags pushCmdFlags) error {
box, err := devbox.Open(&devopt.Opts{
Dir: flags.config.path,
Writer: cmd.ErrOrStderr(),
Stderr: cmd.ErrOrStderr(),
})
if err != nil {
return errors.WithStack(err)
Expand Down
2 changes: 1 addition & 1 deletion internal/boxcli/rm.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ func removeCmd() *cobra.Command {
func runRemoveCmd(cmd *cobra.Command, args []string, flags removeCmdFlags) error {
box, err := devbox.Open(&devopt.Opts{
Dir: flags.config.path,
Writer: cmd.ErrOrStderr(),
Stderr: cmd.ErrOrStderr(),
})
if err != nil {
return errors.WithStack(err)
Expand Down
5 changes: 5 additions & 0 deletions internal/boxcli/root.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,9 @@ func RootCmd() *cobra.Command {
command := &cobra.Command{
Use: "devbox",
Short: "Instant, easy, predictable development environments",
// Warning, PersistentPreRunE is not called if a subcommand also declares
// it. TODO: Figure out a better way to implement this so that subcommands
// can't accidentally override it.
PersistentPreRun: func(cmd *cobra.Command, args []string) {
if flags.quiet {
cmd.SetErr(io.Discard)
Expand All @@ -49,6 +52,8 @@ func RootCmd() *cobra.Command {
SilenceErrors: true,
SilenceUsage: true,
}
command.SetOut(os.Stdout)
Copy link
Contributor

Choose a reason for hiding this comment

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

isn't this a no-op?

Copy link
Collaborator

Choose a reason for hiding this comment

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

maybe I am misunderstanding.

I thought the idea behind cmd.OutOrStdout() was that tests could do command.SetOut and then inspect the output.

If we're overriding here what the tests may set, then it won't work as desired.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So out is not set by default in cobra. So cmd.OutOrStdout returns stdout because out is not set. But cmd.Print uses OutOrStderr (don't ask me why, I don't understand it) so it prints to stderr.

I'll fix this by removing cmd.Print and replacing with fmt.Fprint(cmd.OutOrStdout()


// Stable commands
command.AddCommand(addCmd())
if featureflag.Auth.Enabled() {
Expand Down
4 changes: 2 additions & 2 deletions internal/boxcli/run.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ func runCmd() *cobra.Command {
func listScripts(cmd *cobra.Command, flags runCmdFlags) []string {
box, err := devbox.Open(&devopt.Opts{
Dir: flags.config.path,
Writer: cmd.ErrOrStderr(),
Stderr: cmd.ErrOrStderr(),
Pure: flags.pure,
IgnoreWarnings: true,
})
Expand Down Expand Up @@ -108,7 +108,7 @@ func runScriptCmd(cmd *cobra.Command, args []string, flags runCmdFlags) error {
// Check the directory exists.
box, err := devbox.Open(&devopt.Opts{
Dir: path,
Writer: cmd.ErrOrStderr(),
Stderr: cmd.ErrOrStderr(),
Pure: flags.pure,
Env: env,
OmitBinWrappersFromPath: omitBinWrappersFromPath,
Expand Down
10 changes: 5 additions & 5 deletions internal/boxcli/services.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ func servicesCmd(persistentPreRunE ...cobraFunc) *cobra.Command {
func listServices(cmd *cobra.Command, flags servicesCmdFlags) error {
box, err := devbox.Open(&devopt.Opts{
Dir: flags.config.path,
Writer: cmd.ErrOrStderr(),
Stderr: cmd.ErrOrStderr(),
})
if err != nil {
return errors.WithStack(err)
Expand All @@ -133,7 +133,7 @@ func startServices(cmd *cobra.Command, services []string, flags servicesCmdFlags
box, err := devbox.Open(&devopt.Opts{
Dir: flags.config.path,
Env: env,
Writer: cmd.ErrOrStderr(),
Stderr: cmd.ErrOrStderr(),
})
if err != nil {
return errors.WithStack(err)
Expand All @@ -155,7 +155,7 @@ func stopServices(
box, err := devbox.Open(&devopt.Opts{
Dir: servicesFlags.config.path,
Env: env,
Writer: cmd.ErrOrStderr(),
Stderr: cmd.ErrOrStderr(),
})
if err != nil {
return errors.WithStack(err)
Expand All @@ -178,7 +178,7 @@ func restartServices(
box, err := devbox.Open(&devopt.Opts{
Dir: flags.config.path,
Env: env,
Writer: cmd.ErrOrStderr(),
Stderr: cmd.ErrOrStderr(),
})
if err != nil {
return errors.WithStack(err)
Expand All @@ -201,7 +201,7 @@ func startProcessManager(
Dir: servicesFlags.config.path,
Env: env,
CustomProcessComposeFile: flags.processComposeFile,
Writer: cmd.ErrOrStderr(),
Stderr: cmd.ErrOrStderr(),
})
if err != nil {
return errors.WithStack(err)
Expand Down
4 changes: 2 additions & 2 deletions internal/boxcli/shell.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ func runShellCmd(cmd *cobra.Command, flags shellCmdFlags) error {
Dir: flags.config.path,
Env: env,
Pure: flags.pure,
Writer: cmd.ErrOrStderr(),
Stderr: cmd.ErrOrStderr(),
})
if err != nil {
return errors.WithStack(err)
Expand All @@ -66,7 +66,7 @@ func runShellCmd(cmd *cobra.Command, flags shellCmdFlags) error {
if flags.printEnv {
// false for includeHooks is because init hooks is not compatible with .envrc files generated
// by versions older than 0.4.6
script, err := box.PrintEnv(cmd.Context(), false /*includeHooks*/)
script, err := box.NixEnv(cmd.Context(), false /*includeHooks*/)
if err != nil {
return err
}
Expand Down
4 changes: 2 additions & 2 deletions internal/boxcli/shellenv.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ func shellEnvFunc(cmd *cobra.Command, flags shellEnvCmdFlags) (string, error) {
}
box, err := devbox.Open(&devopt.Opts{
Dir: flags.config.path,
Writer: cmd.ErrOrStderr(),
Stderr: cmd.ErrOrStderr(),
Pure: flags.pure,
Env: env,
})
Expand All @@ -74,7 +74,7 @@ func shellEnvFunc(cmd *cobra.Command, flags shellEnvCmdFlags) (string, error) {
}
}

envStr, err := box.PrintEnv(cmd.Context(), flags.runInitHook)
envStr, err := box.NixEnv(cmd.Context(), flags.runInitHook)
if err != nil {
return "", err
}
Expand Down
2 changes: 1 addition & 1 deletion internal/boxcli/update.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ func updateCmd() *cobra.Command {
func updateCmdFunc(cmd *cobra.Command, args []string, flags *updateCmdFlags) error {
box, err := devbox.Open(&devopt.Opts{
Dir: flags.config.path,
Writer: cmd.ErrOrStderr(),
Stderr: cmd.ErrOrStderr(),
})
if err != nil {
return errors.WithStack(err)
Expand Down
Loading