Skip to content

Commit

Permalink
cmd/cue: avoid calling os.Exit directly
Browse files Browse the repository at this point in the history
In https://cuelang.org/cl/552733 I improved and simplified the way
that we use Cobra, primarily to make New do less work upfront.

One aspect of the old design that I left behind, with some TODOs,
is that some commands still called os.Exit(1) directly.
This is unnecessary, as command functions return an error,
as well as problematic, as exiting prevents running any more code
such as writing to a CUE_STATS file or any deferred function calls.

Thankfully, the "root" command does the right thing now,
and a sub-command can signal an os.Exit(1) by returning ErrPrintedError.

Signed-off-by: Daniel Martí <mvdan@mvdan.cc>
Change-Id: I0627a9660433ba3259de921efafe0617d107faa8
Reviewed-on: https://review.gerrithub.io/c/cue-lang/cue/+/1192972
Reviewed-by: Roger Peppe <rogpeppe@gmail.com>
TryBot-Result: CUEcueckoo <cueckoo@cuelang.org>
Unity-Result: CUE porcuepine <cue.porcuepine@gmail.com>
  • Loading branch information
mvdan committed Apr 11, 2024
1 parent 87ad2e4 commit f047aa0
Show file tree
Hide file tree
Showing 6 changed files with 12 additions and 17 deletions.
8 changes: 7 additions & 1 deletion cmd/cue/cmd/common.go
Expand Up @@ -83,6 +83,12 @@ func getLang() language.Tag {
return language.Make(loc)
}

// exitOnErr uses cue/errors to print any error to stderr,
// and if fatal is true, it causes the command's exit code to be 1.
// Note that os.Exit is not called, as that would prevent defers from running.
//
// TODO(mvdan): can we avoid the panicError and recover shenanigans?
// They work, but they make the code flow somewhat confusing to follow.
func exitOnErr(cmd *Command, err error, fatal bool) {
if err == nil {
return
Expand All @@ -106,7 +112,7 @@ func exitOnErr(cmd *Command, err error, fatal bool) {
b := w.Bytes()
_, _ = cmd.Stderr().Write(b)
if fatal {
exit()
panicExit()
}
}

Expand Down
2 changes: 1 addition & 1 deletion cmd/cue/cmd/fmt.go
Expand Up @@ -146,7 +146,7 @@ func newFmtCmd(c *Command) *cobra.Command {
}
fmt.Fprintln(stdout, relPath)
}
os.Exit(1)
return ErrPrintedError
}

return nil
Expand Down
4 changes: 1 addition & 3 deletions cmd/cue/cmd/get.go
Expand Up @@ -16,7 +16,6 @@ package cmd

import (
"fmt"
"os"

"github.com/spf13/cobra"
)
Expand All @@ -43,8 +42,7 @@ per language and are documented in the respective subcommands.
fmt.Fprintf(stderr, "get must be run as one of its subcommands: unknown subcommand %q\n", args[0])
}
fmt.Fprintln(stderr, "Run 'cue help get' for known subcommands.")
os.Exit(1) // TODO: get rid of this
return nil
return ErrPrintedError
}),
}
cmd.AddCommand(newGoCmd(c))
Expand Down
11 changes: 1 addition & 10 deletions cmd/cue/cmd/mod.go
Expand Up @@ -48,8 +48,7 @@ See also:
fmt.Fprintf(stderr, "mod must be run as one of its subcommands: unknown subcommand %q\n", args[0])
}
fmt.Fprintln(stderr, "Run 'cue help mod' for known subcommands.")
os.Exit(1) // TODO: get rid of this
return nil
return ErrPrintedError
}),
}

Expand Down Expand Up @@ -83,14 +82,6 @@ in the module.
}

func runModInit(cmd *Command, args []string) (err error) {
defer func() {
if err != nil {
// TODO: Refactor Cobra usage to do something more principled
fmt.Fprintln(cmd.OutOrStderr(), err)
os.Exit(1)
}
}()

modulePath := ""
if len(args) > 0 {
if len(args) != 1 {
Expand Down
2 changes: 1 addition & 1 deletion cmd/cue/cmd/root.go
Expand Up @@ -328,6 +328,6 @@ type panicError struct {
Err error
}

func exit() {
func panicExit() {
panic(panicError{ErrPrintedError})
}
2 changes: 1 addition & 1 deletion cmd/cue/cmd/trim.go
Expand Up @@ -141,7 +141,7 @@ func runTrim(cmd *Command, args []string) error {
diff.Print(os.Stdout, script)
fmt.Println("Aborting trim, output differs after trimming. This is a bug! Use -i to force trim.")
fmt.Println("You can file a bug here: https://cuelang.org/issues/new?assignees=&labels=NeedsInvestigation&template=bug_report.md&title=")
os.Exit(1)
return ErrPrintedError
}
}
}
Expand Down

0 comments on commit f047aa0

Please sign in to comment.