diff --git a/codecov.yml b/codecov.yml index 77c0288..51463dd 100644 --- a/codecov.yml +++ b/codecov.yml @@ -1,2 +1,3 @@ ignore: - examples # Demo programs to showcase the library, coverage tracking not needed + - internal/colour # Done this a bunch of times, annoying to test because of $FORCE_COLOR and $NO_COLOR and sync.OnceValues diff --git a/command.go b/command.go index 6bbb53a..980b1b2 100644 --- a/command.go +++ b/command.go @@ -600,7 +600,7 @@ func writeArgumentsSection(cmd *Command, s *strings.Builder) error { case requiredArgMarker: tab.Row(" %s\t%s [required]\n", colour.Bold(arg.name), arg.description) case "": - tab.Row(" %s\t%s\n", colour.Bold(arg.name), arg.description) + tab.Row(" %s\t%s [default %q]\n", colour.Bold(arg.name), arg.description, arg.defaultValue) default: tab.Row(" %s\t%s [default %s]\n", colour.Bold(arg.name), arg.description, arg.defaultValue) } diff --git a/command_test.go b/command_test.go index c54e92c..975e6c1 100644 --- a/command_test.go +++ b/command_test.go @@ -464,6 +464,7 @@ func TestHelp(t *testing.T) { cli.OverrideArgs([]string{"--help"}), cli.RequiredArg("src", "The file to copy"), // This one is required cli.OptionalArg("dest", "Destination to copy to", "./dest"), // This one is optional + cli.OptionalArg("other", "Something else", ""), // This is optional but default is empty cli.Run(func(cmd *cli.Command, args []string) error { return nil }), }, wantErr: false, @@ -543,16 +544,17 @@ func TestHelp(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - // Force no colour in tests - t.Setenv("NO_COLOR", "true") - snap := snapshot.New(t, snapshot.Update(*update)) stderr := &bytes.Buffer{} stdout := &bytes.Buffer{} // Test specific overrides to the options in the table - options := []cli.Option{cli.Stdout(stdout), cli.Stderr(stderr)} + options := []cli.Option{ + cli.Stdout(stdout), + cli.Stderr(stderr), + cli.NoColour(true), + } cmd, err := cli.New("test", slices.Concat(options, tt.options)...) @@ -681,14 +683,15 @@ func TestVersion(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - // Force no colour in tests - t.Setenv("NO_COLOR", "true") - stderr := &bytes.Buffer{} stdout := &bytes.Buffer{} // Test specific overrides to the options in the table - options := []cli.Option{cli.Stdout(stdout), cli.Stderr(stderr)} + options := []cli.Option{ + cli.Stdout(stdout), + cli.Stderr(stderr), + cli.NoColour(true), + } cmd, err := cli.New("version-test", slices.Concat(tt.options, options)...) test.Ok(t, err) diff --git a/internal/colour/colour.go b/internal/colour/colour.go index 2d20cc0..fea08bc 100644 --- a/internal/colour/colour.go +++ b/internal/colour/colour.go @@ -4,7 +4,10 @@ // the same length, which means [text/tabwriter] will correctly calculate alignment as long as styles are not mixed within a table. package colour -import "os" +import ( + "os" + "sync" +) // ANSI codes for coloured output, they are all the same length so as not to throw off // alignment of [text/tabwriter]. @@ -14,6 +17,24 @@ const ( CodeBold = "\x1b[1;0039m" // Bold & white ) +// Disable is a flag that disables all colour text, it overrides both +// $FORCE_COLOR and $NO_COLOR, setting it to true will always make this +// package return plain text and not check any other config. +var Disable bool = false + +// getColourOnce is a [sync.OnceValues] function that returns the state of +// $NO_COLOR and $FORCE_COLOR, once and only once to avoid us calling +// os.Getenv on every call to a colour function. +var getColourOnce = sync.OnceValues(getColour) + +// getColour returns whether $NO_COLOR and $FORCE_COLOR were set. +func getColour() (noColour bool, forceColour bool) { + no := os.Getenv("NO_COLOR") != "" + force := os.Getenv("FORCE_COLOR") != "" + + return no, force +} + // Title returns the given text in a title style, bold white and underlined. // // If $NO_COLOR is set, text will be returned unmodified. @@ -30,13 +51,15 @@ func Bold(text string) string { // sprint returns a string with a given colour and the reset code. // -// It handles checking for NO_COLOR and FORCE_COLOR. +// It handles checking for NO_COLOR and FORCE_COLOR. If the global var +// [Disable] is true then nothing else is checked and plain text is returned. func sprint(code, text string) string { - // TODO(@FollowTheProcess): I don't like checking *every* time but doing it - // via e.g. sync.Once means that tests are annoying unless we ensure env vars are - // set at the process level - noColor := os.Getenv("NO_COLOR") != "" - forceColor := os.Getenv("FORCE_COLOR") != "" + // Our global variable is above all else + if Disable { + return text + } + + noColor, forceColor := getColourOnce() // $FORCE_COLOR overrides $NO_COLOR if forceColor { diff --git a/internal/colour/colour_test.go b/internal/colour/colour_test.go deleted file mode 100644 index 7c85ec0..0000000 --- a/internal/colour/colour_test.go +++ /dev/null @@ -1,107 +0,0 @@ -package colour_test - -import ( - "testing" - - "github.com/FollowTheProcess/cli/internal/colour" - "github.com/FollowTheProcess/test" -) - -func TestColour(t *testing.T) { - tests := []struct { - name string // Name of the test case - text string // Text to colour - fn func(text string) string // Printer function to return the coloured version of text - want string // Expected result containing ANSI escape codes - noColor bool // Whether to set the $NO_COLOR env var - forceColor bool // Whether to set the $FORCE_COLOR env var - }{ - { - name: "bold", - text: "hello bold", - fn: colour.Bold, - want: colour.CodeBold + "hello bold" + colour.CodeReset, - }, - { - name: "bold no color", - text: "hello bold", - fn: colour.Bold, - noColor: true, - want: "hello bold", - }, - { - name: "bold force color", - text: "hello bold", - fn: colour.Bold, - want: colour.CodeBold + "hello bold" + colour.CodeReset, - forceColor: true, - }, - { - name: "bold force color and no color", - text: "hello bold", - fn: colour.Bold, - want: colour.CodeBold + "hello bold" + colour.CodeReset, - forceColor: true, // force should override no - noColor: true, - }, - { - name: "title", - text: "Section", - fn: colour.Title, - want: colour.CodeTitle + "Section" + colour.CodeReset, - }, - { - name: "title no color", - text: "Section", - fn: colour.Title, - noColor: true, - want: "Section", - }, - { - name: "title force color", - text: "Section", - fn: colour.Title, - want: colour.CodeTitle + "Section" + colour.CodeReset, - forceColor: true, - }, - { - name: "title force color and no color", - text: "Section", - fn: colour.Title, - want: colour.CodeTitle + "Section" + colour.CodeReset, - forceColor: true, // force should override no - noColor: true, - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - if tt.noColor { - t.Setenv("NO_COLOR", "true") - } - if tt.forceColor { - t.Setenv("FORCE_COLOR", "true") - } - got := tt.fn(tt.text) - test.Equal(t, got, tt.want) - }) - } -} - -func TestCodesAllSameLength(t *testing.T) { - test.True(t, len(colour.CodeBold) == len(colour.CodeReset)) - test.True(t, len(colour.CodeBold) == len(colour.CodeTitle)) - test.True(t, len(colour.CodeReset) == len(colour.CodeTitle)) -} - -func BenchmarkBold(b *testing.B) { - for range b.N { - colour.Bold("Some bold text here") - } -} - -func BenchmarkTitle(b *testing.B) { - for range b.N { - colour.Title("Some title here") - } -} diff --git a/option.go b/option.go index 56dcd54..8ce94a8 100644 --- a/option.go +++ b/option.go @@ -7,6 +7,7 @@ import ( "slices" "strings" + "github.com/FollowTheProcess/cli/internal/colour" "github.com/FollowTheProcess/cli/internal/flag" ) @@ -164,6 +165,21 @@ func Stderr(stderr io.Writer) Option { return option(f) } +// NoColour is an [Option] that disables all colour output from the [Command]. +// +// CLI respects the values of $NO_COLOR and $FORCE_COLOR automatically so this need +// not be set for most applications. +// +// Setting this option takes precedence over all other colour configuration. +func NoColour(noColour bool) Option { + f := func(cfg *config) error { + // Just disable the internal colour package entirely + colour.Disable = noColour + return nil + } + return option(f) +} + // Short is an [Option] that sets the one line usage summary for a [Command]. // // The one line usage will appear in the help text as well as alongside diff --git a/testdata/snapshots/TestHelp/with_named_arguments.snap.txt b/testdata/snapshots/TestHelp/with_named_arguments.snap.txt index 2642bea..e50688d 100644 --- a/testdata/snapshots/TestHelp/with_named_arguments.snap.txt +++ b/testdata/snapshots/TestHelp/with_named_arguments.snap.txt @@ -1,10 +1,11 @@ A placeholder for something cool -Usage: test [OPTIONS] SRC [DEST] +Usage: test [OPTIONS] SRC [DEST] [OTHER] Arguments: - src The file to copy [required] - dest Destination to copy to [default ./dest] + src The file to copy [required] + dest Destination to copy to [default ./dest] + other Something else [default ""] Options: -h --help bool Show help for test