diff --git a/command_run.go b/command_run.go index 676a14c676..1e9649300f 100644 --- a/command_run.go +++ b/command_run.go @@ -214,6 +214,10 @@ func (cmd *Command) run(ctx context.Context, osArgs []string) (_ context.Context for _, flag := range cmd.allFlags() { isSet := flag.IsSet() + // Propagate the command's slice/map separator config before PostParse + // so that env-var values are split with the same separator as CLI + // values (see https://github.com/urfave/cli/issues/2262). + cmd.setMultiValueParsingConfig(flag) if err := flag.PostParse(); err != nil { return ctx, err } diff --git a/command_test.go b/command_test.go index dafee0bee6..e0dfc2ad91 100644 --- a/command_test.go +++ b/command_test.go @@ -4590,6 +4590,28 @@ func TestCommandSliceFlagSeparator(t *testing.T) { r.Equal([]string{"ff", "dd", "gg", "t,u"}, cmd.Value("foo")) } +// TestCommandSliceFlagSeparatorFromEnvVar is a regression test for +// https://github.com/urfave/cli/issues/2262. +// SliceFlagSeparator must be respected when flag values come from env vars, +// not only when they are supplied on the command line. +func TestCommandSliceFlagSeparatorFromEnvVar(t *testing.T) { + t.Setenv("FOO", "ff;dd;gg") + + cmd := &Command{ + SliceFlagSeparator: ";", + Flags: []Flag{ + &StringSliceFlag{ + Name: "foo", + Sources: EnvVars("FOO"), + }, + }, + } + + r := require.New(t) + r.NoError(cmd.Run(buildTestContext(t), []string{"app"})) + r.Equal([]string{"ff", "dd", "gg"}, cmd.Value("foo")) +} + func TestCommandMapKeyValueFlagSeparator(t *testing.T) { cmd := &Command{ MapFlagKeyValueSeparator: ":", diff --git a/errors.go b/errors.go index f365a57990..ffd6471f23 100644 --- a/errors.go +++ b/errors.go @@ -150,10 +150,12 @@ func HandleExitCoder(err error) { } if exitErr, ok := err.(ExitCoder); ok { - if _, ok := exitErr.(ErrorFormatter); ok { - _, _ = fmt.Fprintf(ErrWriter, "%+v\n", err) - } else { - _, _ = fmt.Fprintln(ErrWriter, err) + if msg := err.Error(); msg != "" { + if _, ok := exitErr.(ErrorFormatter); ok { + _, _ = fmt.Fprintf(ErrWriter, "%+v\n", err) + } else { + _, _ = fmt.Fprintln(ErrWriter, err) + } } OsExiter(exitErr.ExitCode()) return diff --git a/errors_test.go b/errors_test.go index 35aaab54d4..6a497c58f5 100644 --- a/errors_test.go +++ b/errors_test.go @@ -232,3 +232,28 @@ func TestErrRequiredFlags_Error(t *testing.T) { expectedMsg = "Required flag \"flag1\" not set" assert.Equal(t, expectedMsg, err.Error()) } + +// TestHandleExitCoder_EmptyMessage is a regression test for +// https://github.com/urfave/cli/issues/2263. +// HandleExitCoder must not print an empty line to ErrWriter when the ExitCoder +// message is empty (e.g. cli.Exit("", code)). +func TestHandleExitCoder_EmptyMessage(t *testing.T) { + called := false + + OsExiter = func(rc int) { + if !called { + called = true + } + } + ErrWriter = &bytes.Buffer{} + + defer func() { + OsExiter = fakeOsExiter + ErrWriter = fakeErrWriter + }() + + HandleExitCoder(Exit("", 2)) + + assert.True(t, called) + assert.Empty(t, ErrWriter.(*bytes.Buffer).String(), "expected no output for empty-message exit") +} diff --git a/flag_bool_with_inverse.go b/flag_bool_with_inverse.go index 272dd98fec..680fe2c64e 100644 --- a/flag_bool_with_inverse.go +++ b/flag_bool_with_inverse.go @@ -179,6 +179,14 @@ func (bif *BoolWithInverseFlag) String() string { prefix = "-" } + // Guard against a FlagStringer that returns a string without a tab (e.g. + // a custom stringer or the default stringer when the flag does not + // implement DocGenerationFlag). In that case treat the entire output as + // the tab-delimited suffix so the slice never goes out of bounds. + if i < 0 { + i = 0 + } + return fmt.Sprintf("%s[%s]%s%s", prefix, bif.inversePrefix(), bif.Name, out[i:]) } diff --git a/flag_bool_with_inverse_test.go b/flag_bool_with_inverse_test.go index 981494586b..fbb7e43ff8 100644 --- a/flag_bool_with_inverse_test.go +++ b/flag_bool_with_inverse_test.go @@ -520,3 +520,26 @@ func TestBoolWithInverseFlag_SatisfiesVisibleFlagInterface(t *testing.T) { _ = f.IsVisible() } + +// TestBoolWithInverseFlagStringNoPanicWithNoTabStringer is a regression test for +// https://github.com/urfave/cli/issues/2303. +// BoolWithInverseFlag.String() panicked with "slice bounds out of range [-1:]" +// when the FlagStringer returned a string without a tab character. +func TestBoolWithInverseFlagStringNoPanicWithNoTabStringer(t *testing.T) { + orig := FlagStringer + defer func() { FlagStringer = orig }() + + FlagStringer = func(f Flag) string { + return "no tab here" + } + + flag := &BoolWithInverseFlag{ + Name: "verbose", + } + + // Must not panic. + got := flag.String() + if !strings.Contains(got, "verbose") { + t.Errorf("expected String() to contain the flag name, got %q", got) + } +}