From 9934a5764d350e78ee2d5993fa1de35a76b14af1 Mon Sep 17 00:00:00 2001 From: Brian McGee Date: Fri, 31 May 2024 15:00:46 +0100 Subject: [PATCH] fix: --stdin flag This was incorrectly ported from Rust to Go. When provided, `treefmt` will take the contents of stdin and place them into the file provided with the `--stdin` flag, then format it according to the configured formatters. If the file doesn't exist it is created. If it exists, it is first truncated and then populated with stdin. Signed-off-by: Brian McGee --- cli/cli.go | 4 +- cli/format.go | 75 ++++++++++---------------------------- cli/format_test.go | 66 ++++++++++++++++----------------- cli/helpers_test.go | 3 +- config/config_test.go | 2 +- docs/usage.md | 37 ++++++++++--------- foo/bar.md | 1 + test/examples/treefmt.toml | 1 + test/temp.go | 30 +++++++++++++-- walk/filesystem.go | 8 ++-- walk/git.go | 6 +-- walk/walker.go | 14 +++---- 12 files changed, 116 insertions(+), 131 deletions(-) create mode 100644 foo/bar.md diff --git a/cli/cli.go b/cli/cli.go index 542ca4e..aaa4abd 100644 --- a/cli/cli.go +++ b/cli/cli.go @@ -25,8 +25,8 @@ type Format struct { OnUnmatched log.Level `name:"on-unmatched" short:"u" default:"warn" help:"Log paths that did not match any formatters at the specified log level, with fatal exiting the process with an error. Possible values are ."` - Paths []string `name:"paths" arg:"" type:"path" optional:"" help:"Paths to format. Defaults to formatting the whole tree."` - Stdin bool `help:"Format the context passed in via stdin."` + Paths []string `name:"paths" arg:"" type:"path" optional:"" help:"Paths to format. Defaults to formatting the whole tree." xor:"paths"` + Stdin string `type:"path" optional:"" help:"Format stdin, placing the output into the provided path. Formatters are matched based on the path's file extension." xor:"paths"` CpuProfile string `optional:"" help:"The file into which a cpu profile will be written."` } diff --git a/cli/format.go b/cli/format.go index 02f8d3d..384759f 100644 --- a/cli/format.go +++ b/cli/format.go @@ -1,16 +1,15 @@ package cli import ( - "bufio" "context" "errors" "fmt" + "io" "os" "os/signal" "path/filepath" "runtime" "runtime/pprof" - "strings" "syscall" "git.numtide.com/numtide/treefmt/format" @@ -221,66 +220,30 @@ func updateCache(ctx context.Context) func() error { func walkFilesystem(ctx context.Context) func() error { return func() error { - eg, ctx := errgroup.WithContext(ctx) - pathsCh := make(chan string, BatchSize) + // 1. Check if we have been provided with an explicit list of paths to process + // 2. If not, check if we have been passed in some content to format via stdin + // 3. If not, we process the tree root as normal. + paths := Cli.Paths + if Cli.Stdin != "" { - walkPaths := func() error { - defer close(pathsCh) - - var idx int - for idx < len(Cli.Paths) { - select { - case <-ctx.Done(): - return ctx.Err() - default: - pathsCh <- Cli.Paths[idx] - idx += 1 - } + // read from stdin and place the contents into the provided path before processing + if err := os.MkdirAll(filepath.Dir(Cli.Stdin), 0o755); err != nil { + return fmt.Errorf("failed to ensure the directory existed for stdin processing: %w", err) + } else if file, err := os.Create(Cli.Stdin); err != nil { + return fmt.Errorf("failed to open file for stdin processing: %w", err) + } else if _, err = io.Copy(file, os.Stdin); err != nil { + return fmt.Errorf("failed to read stdin: %w", err) + } else if err = file.Close(); err != nil { + return fmt.Errorf("failed to close file for stdin processing: %w", err) } - return nil - } - - walkStdin := func() error { - defer close(pathsCh) - - // determine the current working directory - cwd, err := os.Getwd() - if err != nil { - return fmt.Errorf("failed to determine current working directory: %w", err) - } - - // read in all the paths - scanner := bufio.NewScanner(os.Stdin) - - for scanner.Scan() { - select { - case <-ctx.Done(): - return ctx.Err() - default: - path := scanner.Text() - if !strings.HasPrefix(path, "/") { - // append the cwd - path = filepath.Join(cwd, path) - } - pathsCh <- path - } - } - return nil - } - - if len(Cli.Paths) > 0 { - eg.Go(walkPaths) - } else if Cli.Stdin { - eg.Go(walkStdin) - } else { - // no explicit paths to process, so we only need to process root - pathsCh <- Cli.TreeRoot - close(pathsCh) + paths = []string{Cli.Stdin} + } else if len(paths) == 0 { + paths = []string{Cli.TreeRoot} } // create a filesystem walker - walker, err := walk.New(Cli.Walk, Cli.TreeRoot, pathsCh) + walker, err := walk.New(Cli.Walk, Cli.TreeRoot, paths) if err != nil { return fmt.Errorf("failed to create walker: %w", err) } diff --git a/cli/format_test.go b/cli/format_test.go index f5df561..b5547b8 100644 --- a/cli/format_test.go +++ b/cli/format_test.go @@ -573,54 +573,52 @@ func TestStdIn(t *testing.T) { // capture current cwd, so we can replace it after the test is finished cwd, err := os.Getwd() as.NoError(err) - t.Cleanup(func() { // return to the previous working directory as.NoError(os.Chdir(cwd)) }) tempDir := test.TempExamples(t) - configPath := filepath.Join(tempDir, "/treefmt.toml") - // change working directory to temp root - as.NoError(os.Chdir(tempDir)) - - // basic config - cfg := config.Config{ - Formatters: map[string]*config.Formatter{ - "echo": { - Command: "echo", - Includes: []string{"*"}, - }, - }, - } - test.WriteConfig(t, configPath, cfg) - - // swap out stdin + // capture current stdin and replace it on test cleanup prevStdIn := os.Stdin - stdin, err := os.CreateTemp("", "stdin") - as.NoError(err) - - os.Stdin = stdin - t.Cleanup(func() { os.Stdin = prevStdIn - _ = os.Remove(stdin.Name()) }) - go func() { - _, err := stdin.WriteString(`treefmt.toml -elm/elm.json -go/main.go -`) - as.NoError(err, "failed to write to stdin") - as.NoError(stdin.Sync()) - _, _ = stdin.Seek(0, 0) - }() + // write a new file + contents := `{ foo, ... }: "hello"` + os.Stdin = test.TempFile(t, "", "stdin", &contents) - _, err = cmd(t, "-C", tempDir, "--stdin") + outPath := "foo/bar/test.nix" + _, err = cmd(t, "-C", tempDir, "--allow-missing-formatter", "--stdin", outPath) as.NoError(err) - assertStats(t, as, 3, 3, 3, 0) + assertStats(t, as, 1, 1, 1, 1) + + // the nix formatters should have reduced the example to the following + as.Equal(`{ ...}: "hello" +`, string(test.ReadFile(t, outPath))) + + // overwrite an existing file + contents = ` +| col1 | col2 | +| ---- | ---- | +| nice | fits | +| oh no! | it's ugly | +` + os.Stdin = test.TempFile(t, "", "stdin", &contents) + + outPath = "haskell/CHANGELOG.md" + out, err := cmd(t, "-C", tempDir, "--allow-missing-formatter", "--stdin", outPath, "-vv") + println(out) + as.NoError(err) + assertStats(t, as, 1, 1, 1, 1) + + as.Equal(`| col1 | col2 | +| ------ | --------- | +| nice | fits | +| oh no! | it's ugly | +`, string(test.ReadFile(t, outPath))) } func TestDeterministicOrderingInPipeline(t *testing.T) { diff --git a/cli/helpers_test.go b/cli/helpers_test.go index fed4c4d..cfa31a7 100644 --- a/cli/helpers_test.go +++ b/cli/helpers_test.go @@ -4,7 +4,6 @@ import ( "fmt" "io" "os" - "path/filepath" "testing" "github.com/charmbracelet/log" @@ -42,7 +41,7 @@ func cmd(t *testing.T, args ...string) ([]byte, error) { } tempDir := t.TempDir() - tempOut := test.TempFile(t, filepath.Join(tempDir, "combined_output")) + tempOut := test.TempFile(t, tempDir, "combined_output", nil) // capture standard outputs before swapping them stdout := os.Stdout diff --git a/config/config_test.go b/config/config_test.go index a99f6f8..078b0fb 100644 --- a/config/config_test.go +++ b/config/config_test.go @@ -65,7 +65,7 @@ func TestReadConfigFile(t *testing.T) { deadnix, ok := cfg.Formatters["deadnix"] as.True(ok, "deadnix formatter not found") as.Equal("deadnix", deadnix.Command) - as.Nil(deadnix.Options) + as.Equal([]string{"-e"}, deadnix.Options) as.Equal([]string{"*.nix"}, deadnix.Includes) as.Nil(deadnix.Excludes) as.Equal(2, deadnix.Priority) diff --git a/docs/usage.md b/docs/usage.md index c477a8f..f2b9552 100644 --- a/docs/usage.md +++ b/docs/usage.md @@ -13,22 +13,23 @@ Arguments: [ ...] Paths to format. Defaults to formatting the whole tree. Flags: - -h, --help Show context-sensitive help. - --allow-missing-formatter Do not exit with error if a configured formatter is missing. - -C, --working-directory="." Run as if treefmt was started in the specified working directory instead of the current working directory. - --no-cache Ignore the evaluation cache entirely. Useful for CI. - -c, --clear-cache Reset the evaluation cache. Use in case the cache is not precise enough. - --config-file="./treefmt.toml" The config file to use. - --fail-on-change Exit with error if any changes were made. Useful for CI. - --formatters=FORMATTERS,... Specify formatters to apply. Defaults to all formatters. - --tree-root="." The root directory from which treefmt will start walking the filesystem. - --walk="auto" The method used to traverse the files within --tree-root. Currently supports 'auto', 'git' or 'filesystem'. - -v, --verbose Set the verbosity of logs e.g. -vv ($LOG_LEVEL). - -V, --version Print version. - -i, --init Create a new treefmt.toml. - -u, --on-unmatched=warn Log paths that did not match any formatters at the specified log level, with fatal exiting the process with an error. Possible values are . - --stdin Format the context passed in via stdin. - --cpu-profile=STRING The file into which a cpu profile will be written. + -h, --help Show context-sensitive help. + --allow-missing-formatter Do not exit with error if a configured formatter is missing. + -C, --working-directory="." Run as if treefmt was started in the specified working directory instead of the current working directory. + --no-cache Ignore the evaluation cache entirely. Useful for CI. + -c, --clear-cache Reset the evaluation cache. Use in case the cache is not precise enough. + --config-file=STRING Load the config file from the given path (defaults to searching upwards for treefmt.toml). + --fail-on-change Exit with error if any changes were made. Useful for CI. + -f, --formatters=FORMATTERS,... Specify formatters to apply. Defaults to all formatters. + --tree-root=STRING The root directory from which treefmt will start walking the filesystem (defaults to the directory containing the config file). + --tree-root-file=STRING File to search for to find the project root (if --tree-root is not passed). + --walk="auto" The method used to traverse the files within --tree-root. Currently supports 'auto', 'git' or 'filesystem'. + -v, --verbose Set the verbosity of logs e.g. -vv ($LOG_LEVEL). + -V, --version Print version. + -i, --init Create a new treefmt.toml. + -u, --on-unmatched=warn Log paths that did not match any formatters at the specified log level, with fatal exiting the process with an error. Possible values are . + --stdin=STRING Format stdin, placing the output into the provided path. Formatters are matched based on the path's file extension. + --cpu-profile=STRING The file into which a cpu profile will be written. ``` ## Arguments @@ -102,9 +103,9 @@ Log paths that did not match any formatters at the specified log level, with fat [default: warn] -### `--stdin` +### `--stdin=STRING` -Format the context passed in via stdin. +Format stdin, placing the output into the provided path. Formatters are matched based on the path's file extension. ### `--cpu-profile` diff --git a/foo/bar.md b/foo/bar.md new file mode 100644 index 0000000..2be7c65 --- /dev/null +++ b/foo/bar.md @@ -0,0 +1 @@ +# hello world diff --git a/test/examples/treefmt.toml b/test/examples/treefmt.toml index 8acc9eb..064a58f 100644 --- a/test/examples/treefmt.toml +++ b/test/examples/treefmt.toml @@ -35,6 +35,7 @@ priority = 1 [formatter.deadnix] command = "deadnix" +options = ["-e"] includes = ["*.nix"] priority = 2 diff --git a/test/temp.go b/test/temp.go index dbe6f50..6db8f65 100644 --- a/test/temp.go +++ b/test/temp.go @@ -1,6 +1,7 @@ package test import ( + "io" "os" "testing" @@ -29,15 +30,36 @@ func TempExamples(t *testing.T) string { return tempDir } -func TempFile(t *testing.T, path string) *os.File { +func TempFile(t *testing.T, dir string, pattern string, contents *string) *os.File { t.Helper() - file, err := os.Create(path) - if err != nil { - t.Fatalf("failed to create temporary file: %v", err) + + file, err := os.CreateTemp(dir, pattern) + require.NoError(t, err, "failed to create temp file") + + if contents == nil { + return file } + + _, err = file.WriteString(*contents) + require.NoError(t, err, "failed to write contents to temp file") + require.NoError(t, file.Close(), "failed to close temp file") + + file, err = os.Open(file.Name()) + require.NoError(t, err, "failed to open temp file") + return file } +func ReadFile(t *testing.T, path string) []byte { + f, err := os.Open(path) + require.NoError(t, err, "failed to open file") + defer f.Close() + + bytes, err := io.ReadAll(f) + require.NoError(t, err, "failed to read file") + return bytes +} + func RecreateSymlink(t *testing.T, path string) error { t.Helper() src, err := os.Readlink(path) diff --git a/walk/filesystem.go b/walk/filesystem.go index c98390f..3029709 100644 --- a/walk/filesystem.go +++ b/walk/filesystem.go @@ -7,8 +7,8 @@ import ( ) type filesystemWalker struct { - root string - pathsCh chan string + root string + paths []string } func (f filesystemWalker) Root() string { @@ -34,7 +34,7 @@ func (f filesystemWalker) Walk(_ context.Context, fn WalkFunc) error { return fn(&file, err) } - for path := range f.pathsCh { + for _, path := range f.paths { if err := filepath.Walk(path, walkFn); err != nil { return err } @@ -43,6 +43,6 @@ func (f filesystemWalker) Walk(_ context.Context, fn WalkFunc) error { return nil } -func NewFilesystem(root string, paths chan string) (Walker, error) { +func NewFilesystem(root string, paths []string) (Walker, error) { return filesystemWalker{root, paths}, nil } diff --git a/walk/git.go b/walk/git.go index f5e60a9..79932cd 100644 --- a/walk/git.go +++ b/walk/git.go @@ -14,7 +14,7 @@ import ( type gitWalker struct { root string - paths chan string + paths []string repo *git.Repository } @@ -41,7 +41,7 @@ func (g *gitWalker) Walk(ctx context.Context, fn WalkFunc) error { // cache in-memory whether a path is present in the git index var cache map[string]bool - for path := range g.paths { + for _, path := range g.paths { if path == g.root { // we can just iterate the index entries @@ -116,7 +116,7 @@ func (g *gitWalker) Walk(ctx context.Context, fn WalkFunc) error { return nil } -func NewGit(root string, paths chan string) (Walker, error) { +func NewGit(root string, paths []string) (Walker, error) { repo, err := git.PlainOpen(root) if err != nil { return nil, fmt.Errorf("failed to open git repo: %w", err) diff --git a/walk/walker.go b/walk/walker.go index 6ff1091..0309094 100644 --- a/walk/walker.go +++ b/walk/walker.go @@ -31,24 +31,24 @@ type Walker interface { Walk(ctx context.Context, fn WalkFunc) error } -func New(walkerType Type, root string, pathsCh chan string) (Walker, error) { +func New(walkerType Type, root string, paths []string) (Walker, error) { switch walkerType { case Git: - return NewGit(root, pathsCh) + return NewGit(root, paths) case Auto: - return Detect(root, pathsCh) + return Detect(root, paths) case Filesystem: - return NewFilesystem(root, pathsCh) + return NewFilesystem(root, paths) default: return nil, fmt.Errorf("unknown walker type: %v", walkerType) } } -func Detect(root string, pathsCh chan string) (Walker, error) { +func Detect(root string, paths []string) (Walker, error) { // for now, we keep it simple and try git first, filesystem second - w, err := NewGit(root, pathsCh) + w, err := NewGit(root, paths) if err == nil { return w, err } - return NewFilesystem(root, pathsCh) + return NewFilesystem(root, paths) }