feat: separate formatter config from formatter

Signed-off-by: Brian McGee <brian@bmcgee.ie>

internal/cli/format.go

@@ -64,22 +64,23 @@ func (f *Format) Run() error {
 		}
 	}
 
+	formatters := make(map[string]*format.Formatter)
+
 	// detect dependency cycles and broken dependencies
-	for name, formatter := range cfg.Formatters {
+	// todo dependency cycle detection
-		if formatter.Before != "" {
+	for name, config := range cfg.Formatters {
+		before := config.Before
+		if before != "" {
 			// check child formatter exists
-			_, ok := cfg.Formatters[formatter.Before]
+			_, ok := cfg.Formatters[before]
 			if !ok {
-				return fmt.Errorf(
+				return fmt.Errorf("formatter %v is before %v but config for %v was not found", name, before, before)
-					"formatter %v is before %v but config for %v was not found",
-					name, formatter.Before, formatter.Before,
-				)
 			}
 		}
 	}
 
 	// init formatters
-	for name, formatter := range cfg.Formatters {
+	for name, config := range cfg.Formatters {
 		if !includeFormatter(name) {
 			// remove this formatter
 			delete(cfg.Formatters, name)
@@ -87,23 +88,24 @@ func (f *Format) Run() error {
 			continue
 		}
 
-		err = formatter.Init(name, globalExcludes)
+		formatter, err := format.NewFormatter(name, config, globalExcludes)
 		if errors.Is(err, format.ErrFormatterNotFound) && Cli.AllowMissingFormatter {
 			l.Debugf("formatter not found: %v", name)
-			// remove this formatter
+			continue
-			delete(cfg.Formatters, name)
 		} else if err != nil {
 			return fmt.Errorf("%w: failed to initialise formatter: %v", err, name)
 		}
+
+		formatters[name] = formatter
 	}
 
 	// iterate the initialised formatters configuring parent/child relationships
-	for _, formatter := range cfg.Formatters {
+	for _, formatter := range formatters {
-		if formatter.Before != "" {
+		if formatter.Before() != "" {
-			child, ok := cfg.Formatters[formatter.Before]
+			child, ok := formatters[formatter.Before()]
 			if !ok {
 				// formatter has been filtered out by the user
-				formatter.Before = ""
+				formatter.ResetBefore()
 				continue
 			}
 			formatter.SetChild(child)
@@ -111,7 +113,7 @@ func (f *Format) Run() error {
 		}
 	}
 
-	if err = cache.Open(Cli.TreeRoot, Cli.ClearCache, cfg.Formatters); err != nil {
+	if err = cache.Open(Cli.TreeRoot, Cli.ClearCache, formatters); err != nil {
 		return err
 	}
 
@@ -124,8 +126,8 @@ func (f *Format) Run() error {
 	eg, ctx := errgroup.WithContext(ctx)
 
 	// start the formatters
-	for name := range cfg.Formatters {
+	for name := range formatters {
-		formatter := cfg.Formatters[name]
+		formatter := formatters[name]
 		eg.Go(func() error {
 			return formatter.Run(ctx)
 		})
@@ -180,7 +182,7 @@ func (f *Format) Run() error {
 	eg.Go(func() error {
 		// pass paths to each formatter
 		for path := range pathsCh {
-			for _, formatter := range cfg.Formatters {
+			for _, formatter := range formatters {
 				if formatter.Wants(path) {
 					formatter.Put(path)
 				}
@@ -188,7 +190,7 @@ func (f *Format) Run() error {
 		}
 
 		// indicate no more paths for each formatter
-		for _, formatter := range cfg.Formatters {
+		for _, formatter := range formatters {
 			if formatter.Parent() != nil {
 				// this formatter is not a root, it will be closed by a parent
 				continue
@@ -197,7 +199,7 @@ func (f *Format) Run() error {
 		}
 
 		// await completion
-		for _, formatter := range cfg.Formatters {
+		for _, formatter := range formatters {
 			formatter.AwaitCompletion()
 		}
 

internal/cli/format_test.go

@@ -20,7 +20,7 @@ func TestAllowMissingFormatter(t *testing.T) {
 	configPath := tempDir + "/treefmt.toml"
 
 	test.WriteConfig(t, configPath, format.Config{
-		Formatters: map[string]*format.Formatter{
+		Formatters: map[string]*format.FormatterConfig{
 			"foo-fmt": {
 				Command: "foo-fmt",
 			},
@@ -41,7 +41,7 @@ func TestSpecifyingFormatters(t *testing.T) {
 	configPath := tempDir + "/treefmt.toml"
 
 	test.WriteConfig(t, configPath, format.Config{
-		Formatters: map[string]*format.Formatter{
+		Formatters: map[string]*format.FormatterConfig{
 			"elm": {
 				Command:  "echo",
 				Includes: []string{"*.elm"},
@@ -90,7 +90,7 @@ func TestIncludesAndExcludes(t *testing.T) {
 
 	// test without any excludes
 	config := format.Config{
-		Formatters: map[string]*format.Formatter{
+		Formatters: map[string]*format.FormatterConfig{
 			"echo": {
 				Command:  "echo",
 				Includes: []string{"*"},
@@ -162,7 +162,7 @@ func TestCache(t *testing.T) {
 
 	// test without any excludes
 	config := format.Config{
-		Formatters: map[string]*format.Formatter{
+		Formatters: map[string]*format.FormatterConfig{
 			"echo": {
 				Command:  "echo",
 				Includes: []string{"*"},
@@ -197,7 +197,7 @@ func TestChangeWorkingDirectory(t *testing.T) {
 
 	// test without any excludes
 	config := format.Config{
-		Formatters: map[string]*format.Formatter{
+		Formatters: map[string]*format.FormatterConfig{
 			"echo": {
 				Command:  "echo",
 				Includes: []string{"*"},
@@ -222,7 +222,7 @@ func TestFailOnChange(t *testing.T) {
 
 	// test without any excludes
 	config := format.Config{
-		Formatters: map[string]*format.Formatter{
+		Formatters: map[string]*format.FormatterConfig{
 			"echo": {
 				Command:  "echo",
 				Includes: []string{"*"},
@@ -258,7 +258,7 @@ func TestBustCacheOnFormatterChange(t *testing.T) {
 
 	// start with 2 formatters
 	config := format.Config{
-		Formatters: map[string]*format.Formatter{
+		Formatters: map[string]*format.FormatterConfig{
 			"python": {
 				Command:  "black",
 				Includes: []string{"*.py"},
@@ -302,7 +302,7 @@ func TestBustCacheOnFormatterChange(t *testing.T) {
 	as.Contains(string(out), "0 files changed")
 
 	// add go formatter
-	config.Formatters["go"] = &format.Formatter{
+	config.Formatters["go"] = &format.FormatterConfig{
 		Command:  "gofmt",
 		Options:  []string{"-w"},
 		Includes: []string{"*.go"},
@@ -353,7 +353,7 @@ func TestOrderingFormatters(t *testing.T) {
 
 	// missing child
 	test.WriteConfig(t, configPath, format.Config{
-		Formatters: map[string]*format.Formatter{
+		Formatters: map[string]*format.FormatterConfig{
 			"hs-a": {
 				Command:  "echo",
 				Includes: []string{"*.hs"},
@@ -367,7 +367,7 @@ func TestOrderingFormatters(t *testing.T) {
 
 	// multiple roots
 	test.WriteConfig(t, configPath, format.Config{
-		Formatters: map[string]*format.Formatter{
+		Formatters: map[string]*format.FormatterConfig{
 			"hs-a": {
 				Command:  "echo",
 				Includes: []string{"*.hs"},

internal/format/config.go

@@ -8,7 +8,7 @@ type Config struct {
 		// Excludes is an optional list of glob patterns used to exclude certain files from all formatters.
 		Excludes []string
 	}
-	Formatters map[string]*Formatter `toml:"formatter"`
+	Formatters map[string]*FormatterConfig `toml:"formatter"`
 }
 
 // ReadConfigFile reads from path and unmarshals toml into a Config instance.

internal/format/format.go

@@ -14,8 +14,7 @@ import (
 // ErrFormatterNotFound is returned when the Command for a Formatter is not available.
 var ErrFormatterNotFound = errors.New("formatter not found")
 
-// Formatter represents a command which should be applied to a filesystem.
+type FormatterConfig struct {
-type Formatter struct {
 	// Command is the command invoke when applying this Formatter.
 	Command string
 	// Options are an optional list of args to be passed to Command.
@@ -26,11 +25,17 @@ type Formatter struct {
 	Excludes []string
 	// Before is the name of another formatter which must process a path after this one
 	Before string
+}
+
+// Formatter represents a command which should be applied to a filesystem.
+type Formatter struct {
+	name   string
+	config *FormatterConfig
 
-	name       string
 	log        *log.Logger
 	executable string // path to the executable described by Command
 
+	before string
 	parent *Formatter
 	child  *Formatter
 
@@ -49,24 +54,35 @@ type Formatter struct {
 	batchSize int
 }
 
+func (f *Formatter) Before() string {
+	return f.before
+}
+
+func (f *Formatter) ResetBefore() {
+	f.before = ""
+}
+
 // Executable returns the path to the executable defined by Command
 func (f *Formatter) Executable() string {
 	return f.executable
 }
 
-// Init is used to initialise internal state before this Formatter is ready to accept paths.
+// NewFormatter is used to create a new Formatter.
-func (f *Formatter) Init(name string, globalExcludes []glob.Glob) error {
+func NewFormatter(name string, config *FormatterConfig, globalExcludes []glob.Glob) (*Formatter, error) {
 	var err error
 
+	f := Formatter{}
 	// capture the name from the config file
 	f.name = name
+	f.config = config
+	f.before = config.Before
 
 	// test if the formatter is available
-	executable, err := exec.LookPath(f.Command)
+	executable, err := exec.LookPath(config.Command)
 	if errors.Is(err, exec.ErrNotFound) {
-		return ErrFormatterNotFound
+		return nil, ErrFormatterNotFound
 	} else if err != nil {
-		return err
+		return nil, err
 	}
 	f.executable = executable
 
@@ -77,18 +93,18 @@ func (f *Formatter) Init(name string, globalExcludes []glob.Glob) error {
 	f.inboxCh = make(chan string, f.batchSize)
 	f.completedCh = make(chan interface{}, 1)
 
-	f.includes, err = CompileGlobs(f.Includes)
+	f.includes, err = CompileGlobs(config.Includes)
 	if err != nil {
-		return fmt.Errorf("%w: formatter '%v' includes", err, f.name)
+		return nil, fmt.Errorf("%w: formatter '%v' includes", err, f.name)
 	}
 
-	f.excludes, err = CompileGlobs(f.Excludes)
+	f.excludes, err = CompileGlobs(config.Excludes)
 	if err != nil {
-		return fmt.Errorf("%w: formatter '%v' excludes", err, f.name)
+		return nil, fmt.Errorf("%w: formatter '%v' excludes", err, f.name)
 	}
 	f.excludes = append(f.excludes, globalExcludes...)
 
-	return nil
+	return &f, nil
 }
 
 func (f *Formatter) SetParent(formatter *Formatter) {
@@ -177,7 +193,7 @@ func (f *Formatter) apply(ctx context.Context) error {
 	}
 
 	// construct args, starting with config
-	args := f.Options
+	args := f.config.Options
 
 	// append each file path
 	for _, path := range f.batch {
@@ -186,7 +202,7 @@ func (f *Formatter) apply(ctx context.Context) error {
 
 	// execute
 	start := time.Now()
-	cmd := exec.CommandContext(ctx, f.Command, args...)
+	cmd := exec.CommandContext(ctx, f.config.Command, args...)
 
 	if out, err := cmd.CombinedOutput(); err != nil {
 		f.log.Debugf("\n%v", string(out))