Merged
Conversation
zimbatm
reviewed
Jan 4, 2025
Signed-off-by: Brian McGee <brian@bmcgee.ie>
brianmcgee
force-pushed
the
feat/verbosity-changes
branch
from
39a165f to
496211c
Compare
Member
Author
|
Revised my approach based on the feedback. |
zimbatm
reviewed
Jan 8, 2025
Member
zimbatm
left a comment
zimbatm
left a comment
There was a problem hiding this comment.
One small nit but I think it's ready. 👍
Signed-off-by: Brian McGee <brian@bmcgee.ie> diff --git a/cmd/format/format.go b/cmd/format/format.go index 7c053d5..041a28d 100644 --- a/cmd/format/format.go +++ b/cmd/format/format.go @@ -197,9 +197,9 @@ func Run(v *viper.Viper, statz *stats.Stats, cmd *cobra.Command, paths []string) return fmt.Errorf("failed to close walker: %w", err) } - // print stats to stdout, unless we are processing from stdin and therefore outputting the results to stdout - if !cfg.Stdin { - statz.Print() + // print stats to stderr + if !cfg.Quiet { + statz.PrintToStderr() } if formatErr != nil { diff --git a/cmd/root_test.go b/cmd/root_test.go index 9d42266..5707ec5 100644 --- a/cmd/root_test.go +++ b/cmd/root_test.go @@ -74,7 +74,7 @@ func TestOnUnmatched(t *testing.T) { // default is WARN t.Run("default", func(t *testing.T) { - treefmt(t, withNoError(t), withOutput(checkOutput(log.WarnLevel))) + treefmt(t, withNoError(t), withStderr(checkOutput(log.WarnLevel))) }) // should exit with error when using fatal @@ -99,7 +99,7 @@ func TestOnUnmatched(t *testing.T) { treefmt(t, withArgs("-vv", "--on-unmatched", levelStr), withNoError(t), - withOutput(checkOutput(level)), + withStderr(checkOutput(level)), ) t.Setenv("TREEFMT_ON_UNMATCHED", levelStr) @@ -107,7 +107,7 @@ func TestOnUnmatched(t *testing.T) { treefmt(t, withArgs("-vv"), withNoError(t), - withOutput(checkOutput(level)), + withStderr(checkOutput(level)), ) }) } @@ -1583,7 +1583,7 @@ func TestStdin(t *testing.T) { withError(func(err error) { as.EqualError(err, "exactly one path should be specified when using the --stdin flag") }), - withOutput(func(out []byte) { + withStderr(func(out []byte) { as.Equal("Error: exactly one path should be specified when using the --stdin flag\n", string(out)) }), ) @@ -1600,7 +1600,7 @@ func TestStdin(t *testing.T) { stats.Formatted: 1, stats.Changed: 1, }), - withOutput(func(out []byte) { + withStdout(func(out []byte) { as.Equal(`{ ...}: "hello" `, string(out)) }), @@ -1616,7 +1616,7 @@ func TestStdin(t *testing.T) { withError(func(err error) { as.Errorf(err, "path ../test.nix not inside the tree root %s", tempDir) }), - withOutput(func(out []byte) { + withStderr(func(out []byte) { as.Contains(string(out), "Error: path ../test.nix not inside the tree root") }), ) @@ -1639,7 +1639,7 @@ func TestStdin(t *testing.T) { stats.Formatted: 1, stats.Changed: 1, }), - withOutput(func(out []byte) { + withStdout(func(out []byte) { as.Equal(`| col1 | col2 | | ------ | --------- | | nice | fits | @@ -1806,7 +1806,9 @@ type options struct { value *config.Config } - assertOut func([]byte) + assertStdout func([]byte) + assertStderr func([]byte) + assertError func(error) assertStats func(*stats.Stats) @@ -1873,9 +1875,15 @@ func withNoError(t *testing.T) option { } } -func withOutput(fn func([]byte)) option { +func withStdout(fn func([]byte)) option { + return func(o *options) { + o.assertStdout = fn + } +} + +func withStderr(fn func([]byte)) option { return func(o *options) { - o.assertOut = fn + o.assertStderr = fn } } @@ -1931,17 +1939,19 @@ func treefmt( t.Logf("treefmt %s", strings.Join(args, " ")) tempDir := t.TempDir() - tempOut := test.TempFile(t, tempDir, "combined_output", nil) + + tempStdout := test.TempFile(t, tempDir, "stdout", nil) + tempStderr := test.TempFile(t, tempDir, "stderr", nil) // capture standard outputs before swapping them stdout := os.Stdout stderr := os.Stderr // swap them temporarily - os.Stdout = tempOut - os.Stderr = tempOut + os.Stdout = tempStdout + os.Stderr = tempStderr - log.SetOutput(tempOut) + log.SetOutput(tempStdout) defer func() { // swap outputs back @@ -1954,30 +1964,49 @@ func treefmt( root, statz := cmd.NewRoot() root.SetArgs(args) - root.SetOut(tempOut) - root.SetErr(tempOut) + root.SetOut(tempStdout) + root.SetErr(tempStderr) // execute the command cmdErr := root.Execute() - // reset and read the temporary output - if _, resetErr := tempOut.Seek(0, 0); resetErr != nil { + // reset and read the temporary outputs + if _, resetErr := tempStdout.Seek(0, 0); resetErr != nil { t.Fatal(fmt.Errorf("failed to reset temp output for reading: %w", resetErr)) } - out, readErr := io.ReadAll(tempOut) + if _, resetErr := tempStderr.Seek(0, 0); resetErr != nil { + t.Fatal(fmt.Errorf("failed to reset temp output for reading: %w", resetErr)) + } + + // read back stderr and validate + out, readErr := io.ReadAll(tempStderr) if readErr != nil { - t.Fatal(fmt.Errorf("failed to read temp output: %w", readErr)) + t.Fatal(fmt.Errorf("failed to read temp stderr: %w", readErr)) + } + + if opts.assertStderr != nil { + opts.assertStderr(out) } t.Log("\n" + string(out)) - if opts.assertStats != nil { - opts.assertStats(statz) + // read back stdout and validate + out, readErr = io.ReadAll(tempStdout) + if readErr != nil { + t.Fatal(fmt.Errorf("failed to read temp stdout: %w", readErr)) + } + + t.Log("\n" + string(out)) + + if opts.assertStdout != nil { + opts.assertStdout(out) } - if opts.assertOut != nil { - opts.assertOut(out) + // assert other properties + + if opts.assertStats != nil { + opts.assertStats(statz) } if opts.assertError != nil { diff --git a/stats/stats.go b/stats/stats.go index 70c174f..7c00efd 100644 --- a/stats/stats.go +++ b/stats/stats.go @@ -2,6 +2,7 @@ package stats import ( "fmt" + "os" "strings" "sync/atomic" "time" @@ -34,7 +35,7 @@ func (s *Stats) Elapsed() time.Duration { return time.Since(s.start) } -func (s *Stats) Print() { +func (s *Stats) PrintToStderr() { components := []string{ "traversed %d files", "emitted %d files for processing", @@ -42,7 +43,8 @@ func (s *Stats) Print() { "", } - fmt.Printf( + _, _ = fmt.Fprintf( + os.Stderr, strings.Join(components, "\n"), s.Value(Traversed), s.Value(Matched),
Signed-off-by: Brian McGee <brian@bmcgee.ie>
Signed-off-by: Brian McGee <brian@bmcgee.ie>
Signed-off-by: Brian McGee <brian@bmcgee.ie> diff --git a/cmd/root.go b/cmd/root.go index 72348f1..cd3f8f7 100644 --- a/cmd/root.go +++ b/cmd/root.go @@ -144,13 +144,19 @@ func runE(v *viper.Viper, statz *stats.Stats, cmd *cobra.Command, args []string) log.SetOutput(os.Stderr) log.SetReportTimestamp(false) - switch v.GetInt("verbose") { - case 0: - log.SetLevel(log.WarnLevel) - case 1: - log.SetLevel(log.InfoLevel) - default: - log.SetLevel(log.DebugLevel) + if v.GetBool("quiet") { + // if quiet, we only log errors + log.SetLevel(log.ErrorLevel) + } else { + // otherwise, the verbose flag controls the log level + switch v.GetInt("verbose") { + case 0: + log.SetLevel(log.WarnLevel) + case 1: + log.SetLevel(log.InfoLevel) + default: + log.SetLevel(log.DebugLevel) + } } // format diff --git a/cmd/root_test.go b/cmd/root_test.go index 2846504..628a69b 100644 --- a/cmd/root_test.go +++ b/cmd/root_test.go @@ -131,6 +131,33 @@ func TestOnUnmatched(t *testing.T) { }) } +func TestQuiet(t *testing.T) { + as := require.New(t) + tempDir := test.TempExamples(t) + + test.ChangeWorkDir(t, tempDir) + + // allow missing formatter + t.Setenv("TREEFMT_ALLOW_MISSING_FORMATTER", "true") + + noOutput := func(out []byte) { + as.Empty(out) + } + + treefmt(t, withArgs("-q"), withNoError(t), withStdout(noOutput), withStderr(noOutput)) + treefmt(t, withArgs("--quiet"), withNoError(t), withStdout(noOutput), withStderr(noOutput)) + + t.Setenv("TREEFMT_QUIET", "true") + treefmt(t, withNoError(t), withStdout(noOutput), withStderr(noOutput)) + + t.Setenv("TREEFMT_ALLOW_MISSING_FORMATTER", "false") + + // check it doesn't suppress errors + treefmt(t, withError(func(err error) { + as.ErrorContains(err, "error looking up 'foo-fmt'") + })) +} + func TestCpuProfile(t *testing.T) { as := require.New(t) tempDir := test.TempExamples(t) diff --git a/config/config.go b/config/config.go index f5d8e7a..c10df3f 100644 --- a/config/config.go +++ b/config/config.go @@ -22,6 +22,7 @@ type Config struct { Formatters []string `mapstructure:"formatters" toml:"formatters,omitempty"` NoCache bool `mapstructure:"no-cache" toml:"-"` // not allowed in config OnUnmatched string `mapstructure:"on-unmatched" toml:"on-unmatched,omitempty"` + Quiet bool `mapstructure:"quiet" toml:"-"` // not allowed in config TreeRoot string `mapstructure:"tree-root" toml:"tree-root,omitempty"` TreeRootFile string `mapstructure:"tree-root-file" toml:"tree-root-file,omitempty"` Verbose uint8 `mapstructure:"verbose" toml:"verbose,omitempty"` @@ -110,6 +111,9 @@ func SetFlags(fs *pflag.FlagSet) { "verbose", "v", "Set the verbosity of logs e.g. -vv. (env $TREEFMT_VERBOSE)", ) + fs.BoolP( + "quiet", "q", false, "Disable all logs except errors. (env $TREEFMT_QUIET)", + ) fs.String( "walk", "auto", "The method used to traverse the files within the tree root. Currently supports "+ diff --git a/config/config_test.go b/config/config_test.go index 281e18a..ae6dc83 100644 --- a/config/config_test.go +++ b/config/config_test.go @@ -323,6 +323,36 @@ func TestNoCache(t *testing.T) { checkValue(true) } +func TestQuiet(t *testing.T) { + as := require.New(t) + + cfg := &config.Config{} + v, flags := newViper(t) + + checkValue := func(expected bool) { + readValue(t, v, cfg, func(cfg *config.Config) { + as.Equal(expected, cfg.Quiet) + }) + } + + // default with no flag, env or config + checkValue(false) + + // set config value and check that it has no effect + // you are not allowed to set no-cache in config + cfg.Quiet = true + + checkValue(false) + + // env override + t.Setenv("TREEFMT_QUIET", "false") + checkValue(false) + + // flag override + as.NoError(flags.Set("quiet", "true")) + checkValue(true) +} + func TestOnUnmatched(t *testing.T) { as := require.New(t) diff --git a/docs/content/getting-started/configure.md b/docs/content/getting-started/configure.md index efc8f2b..623077e 100644 --- a/docs/content/getting-started/configure.md +++ b/docs/content/getting-started/configure.md @@ -254,6 +254,22 @@ Possible values are `<debug|info|warn|error|fatal>`. on-unmatched = "debug" ``` +### `quiet` + +Suppress all output except for errors. + +=== "Flag" + + ```console + treefmt --quiet + ``` + +=== "Env" + + ```console + TREEFMT_QUIET=true treefmt + ``` + ### `stdin` Format the context passed in via stdin.
brianmcgee
force-pushed
the
feat/verbosity-changes
branch
from
496211c to
10c9a72
Compare
zimbatm
approved these changes
Jan 11, 2025
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
I've made some changes to how verbose we are being:
Closes #501