diff --git a/cli/cli.go b/cli/cli.go index 243a8439..8210ac47 100644 --- a/cli/cli.go +++ b/cli/cli.go @@ -43,8 +43,8 @@ type Format struct { globalExcludes []glob.Glob filesCh chan *walk.File - formattedCh chan *walk.File - processedCh chan *walk.File + formattedCh chan *format.Task + processedCh chan *format.Task } func (f *Format) configureLogging() { diff --git a/cli/format.go b/cli/format.go index 584cece7..93593bbc 100644 --- a/cli/format.go +++ b/cli/format.go @@ -184,10 +184,10 @@ func (f *Format) Run() (err error) { f.filesCh = make(chan *walk.File, BatchSize*runtime.NumCPU()) // create a channel for files that have been formatted - f.formattedCh = make(chan *walk.File, cap(f.filesCh)) + f.formattedCh = make(chan *format.Task, cap(f.filesCh)) // create a channel for files that have been processed - f.processedCh = make(chan *walk.File, cap(f.filesCh)) + f.processedCh = make(chan *format.Task, cap(f.filesCh)) // start concurrent processing tasks in reverse order eg.Go(f.updateCache(ctx)) @@ -317,17 +317,21 @@ func (f *Format) applyFormatters(ctx context.Context) func() error { // asynchronously apply the sequence formatters to the batch fg.Go(func() error { - // iterate the formatters, applying them in sequence to the batch of tasks - // we get the formatters list from the first task since they have all the same formatters list - for _, f := range tasks[0].Formatters { - if err := f.Apply(ctx, tasks); err != nil { - return err + // Iterate the formatters, applying them in sequence to the batch of tasks. + // We get the formatter list from the first task since they have all the same formatters list. + formatters := tasks[0].Formatters + + var formatErrors []error + for idx := range formatters { + if err := formatters[idx].Apply(ctx, tasks); err != nil { + formatErrors = append(formatErrors, err) } } // pass each file to the formatted channel for _, task := range tasks { - f.formattedCh <- task.File + task.Errors = formatErrors + f.formattedCh <- task } return nil @@ -359,7 +363,9 @@ func (f *Format) applyFormatters(ctx context.Context) func() error { if format.PathMatches(file.RelPath, f.globalExcludes) { log.Debugf("path matched global excludes: %s", file.RelPath) // mark it as processed and continue to the next - f.formattedCh <- file + f.formattedCh <- &format.Task{ + File: file, + } continue } @@ -378,7 +384,9 @@ func (f *Format) applyFormatters(ctx context.Context) func() error { } log.Logf(f.OnUnmatched, "no formatter for path: %s", file.RelPath) // mark it as processed and continue to the next - f.formattedCh <- file + f.formattedCh <- &format.Task{ + File: file, + } } else { // record the match stats.Add(stats.Matched, 1) @@ -414,14 +422,15 @@ func (f *Format) detectFormatted(ctx context.Context) func() error { // detect ctx cancellation case <-ctx.Done(): return ctx.Err() - // take the next file that has been processed - case file, ok := <-f.formattedCh: + // take the next task that has been processed + case task, ok := <-f.formattedCh: if !ok { // channel has been closed, no further files to process return nil } // check if the file has changed + file := task.File changed, newInfo, err := file.HasChanged() if err != nil { return err @@ -451,7 +460,7 @@ func (f *Format) detectFormatted(ctx context.Context) func() error { } // mark as processed - f.processedCh <- file + f.processedCh <- task } } } @@ -460,12 +469,16 @@ func (f *Format) detectFormatted(ctx context.Context) func() error { func (f *Format) updateCache(ctx context.Context) func() error { return func() error { // used to batch updates for more efficient txs - batch := make([]*walk.File, 0, BatchSize) + batch := make([]*format.Task, 0, BatchSize) // apply a batch processBatch := func() error { // pass the batch to the cache for updating - if err := cache.Update(batch); err != nil { + files := make([]*walk.File, len(batch)) + for idx := range batch { + files[idx] = batch[idx].File + } + if err := cache.Update(files); err != nil { return err } batch = batch[:0] @@ -486,12 +499,14 @@ func (f *Format) updateCache(ctx context.Context) func() error { case <-ctx.Done(): return ctx.Err() // respond to formatted files - case file, ok := <-f.processedCh: + case task, ok := <-f.processedCh: if !ok { // channel has been closed, no further files to process break LOOP } + file := task.File + if f.Stdin { // dump file into stdout f, err := os.Open(file.Path) @@ -508,11 +523,16 @@ func (f *Format) updateCache(ctx context.Context) func() error { continue } - // append to batch and process if we have enough - batch = append(batch, file) - if len(batch) == BatchSize { - if err := processBatch(); err != nil { - return err + // Append to batch and process if we have enough. + // We do not cache any files that were part of a pipeline in which one or more formatters failed. + // This is to ensure those files are re-processed in later invocations after the user has potentially + // resolved the issue, e.g. fixed a config problem. + if len(task.Errors) == 0 { + batch = append(batch, task) + if len(batch) == BatchSize { + if err := processBatch(); err != nil { + return err + } } } } diff --git a/docs/contributing.md b/docs/contributing.md index c642a98b..497c1bfd 100644 --- a/docs/contributing.md +++ b/docs/contributing.md @@ -37,8 +37,6 @@ updated `nix/packages/treefmt/gomod2nix.toml`. To sync it up, run `nix develop .#renovate -c gomod2nix:update`. - - ## Making changes If you want to introduce changes to the project, please follow these steps: diff --git a/format/formatter.go b/format/formatter.go index 58ea1894..4bf2effb 100644 --- a/format/formatter.go +++ b/format/formatter.go @@ -75,16 +75,15 @@ func (f *Formatter) Apply(ctx context.Context, tasks []*Task) error { f.log.Debugf("executing: %s", cmd.String()) if out, err := cmd.CombinedOutput(); err != nil { + f.log.Errorf("failed to apply with options '%v': %s", f.config.Options, err) if len(out) > 0 { _, _ = fmt.Fprintf(os.Stderr, "%s error:\n%s\n", f.name, out) } return fmt.Errorf("formatter '%s' with options '%v' failed to apply: %w", f.config.Command, f.config.Options, err) + } else { + f.log.Infof("%v file(s) processed in %v", len(tasks), time.Since(start)) } - // - - f.log.Infof("%v file(s) processed in %v", len(tasks), time.Since(start)) - return nil } diff --git a/format/task.go b/format/task.go index 062b4544..c7f7c0c6 100644 --- a/format/task.go +++ b/format/task.go @@ -11,6 +11,7 @@ type Task struct { File *walk.File Formatters []*Formatter BatchKey string + Errors []error } func NewTask(file *walk.File, formatters []*Formatter) Task {