Merge pull request #6875 from thaJeztah/optimize_formatter

cli/command/formatter: assorted fixes and cleanups

Author: vvoland

cli/command/container/formatter_stats.go

@@ -112,9 +112,14 @@ func NewStatsFormat(source, osType string) formatter.Format {
 	return formatter.Format(source)
 }
 
-// NewStats returns a new Stats entity and sets in it the given name
-func NewStats(container string) *Stats {
-	return &Stats{StatsEntry: StatsEntry{Container: container}}
+// NewStats returns a new Stats entity using the given ID, ID-prefix, or
+// name to resolve the container.
+func NewStats(idOrName string) *Stats {
+	// FIXME(thaJeztah): "idOrName" is used for fuzzy-matching the container, which can result in multiple stats for the same container.
+	// We should resolve the canonical ID once, then use that as reference
+	// to prevent duplicates. Various parts in the code compare Container
+	// against "ID" only (not considering "name" or "ID-prefix").
+	return &Stats{StatsEntry: StatsEntry{Container: idOrName}}
 }
 
 // statsFormatWrite renders the context for a list of containers statistics

cli/command/formatter/container.go

@@ -141,18 +141,28 @@ func (c *ContainerContext) ID() string {
 
 // Names returns a comma-separated string of the container's names, with their
 // slash (/) prefix stripped. Additional names for the container (related to the
-// legacy `--link` feature) are omitted.
+// legacy `--link` feature) are omitted when formatting "truncated".
 func (c *ContainerContext) Names() string {
-	names := StripNamePrefix(c.c.Names)
-	if c.trunc {
-		for _, name := range names {
-			if len(strings.Split(name, "/")) == 1 {
-				names = []string{name}
-				break
+	var b strings.Builder
+	for i, n := range c.c.Names {
+		name := strings.TrimPrefix(n, "/")
+		if c.trunc {
+			// When printing truncated, we only print a single name.
+			//
+			// Pick the first name that's not a legacy link (does not have
+			// slashes inside the name itself (e.g., "/other-container/link")).
+			// Normally this would be the first name found.
+			if strings.IndexByte(name, '/') == -1 {
+				return name
 			}
+			continue
+		}
+		if i > 0 {
+			b.WriteByte(',')
 		}
+		b.WriteString(name)
 	}
-	return strings.Join(names, ",")
+	return b.String()
 }
 
 // StripNamePrefix removes any "/" prefix from container names returned

cli/command/formatter/disk_usage.go

@@ -46,7 +46,6 @@ func (ctx *DiskUsageContext) startSubsection(format Format) (*template.Template,
 	ctx.buffer = &bytes.Buffer{}
 	ctx.header = ""
 	ctx.Format = format
-	ctx.preFormat()
 
 	return ctx.parseFormat()
 }
@@ -88,7 +87,6 @@ func (ctx *DiskUsageContext) Write() (err error) {
 		return ctx.verboseWrite()
 	}
 	ctx.buffer = &bytes.Buffer{}
-	ctx.preFormat()
 
 	tmpl, err := ctx.parseFormat()
 	if err != nil {
@@ -213,7 +211,6 @@ func (ctx *DiskUsageContext) verboseWrite() error {
 		return ctx.verboseWriteTable(duc)
 	}
 
-	ctx.preFormat()
 	tmpl, err := ctx.parseFormat()
 	if err != nil {
 		return err

cli/command/formatter/formatter.go

@@ -33,7 +33,7 @@ func (f Format) IsTable() bool {
 	return strings.HasPrefix(string(f), TableFormatKey)
 }
 
-// IsJSON returns true if the format is the json format
+// IsJSON returns true if the format is the JSON format
 func (f Format) IsJSON() bool {
 	return string(f) == JSONFormatKey
 }
@@ -43,6 +43,31 @@ func (f Format) Contains(sub string) bool {
 	return strings.Contains(string(f), sub)
 }
 
+// templateString pre-processes the format and returns it as a string
+// for templating.
+func (f Format) templateString() string {
+	out := string(f)
+	switch out {
+	case TableFormatKey:
+		// A bare "--format table" should already be handled before we
+		// hit this; a literal "table" here means a custom "table" format
+		// without template.
+		return ""
+	case JSONFormatKey:
+		// "--format json" only; not JSON formats ("--format '{{json .Field}}'").
+		return JSONFormat
+	}
+
+	// "--format 'table {{.Field}}\t{{.Field}}'" -> "{{.Field}}\t{{.Field}}"
+	if after, isTable := strings.CutPrefix(out, TableFormatKey); isTable {
+		out = after
+	}
+
+	out = strings.Trim(out, " ") // trim spaces, but preserve other whitespace.
+	out = strings.NewReplacer(`\t`, "\t", `\n`, "\n").Replace(out)
+	return out
+}
+
 // Context contains information required by the formatter to print the output as desired.
 type Context struct {
 	// Output is the output stream to which the formatted string is written.
@@ -53,49 +78,34 @@ type Context struct {
 	Trunc bool
 
 	// internal element
-	finalFormat string
-	header      any
-	buffer      *bytes.Buffer
-}
-
-func (c *Context) preFormat() {
-	c.finalFormat = string(c.Format)
-	// TODO: handle this in the Format type
-	switch {
-	case c.Format.IsTable():
-		c.finalFormat = c.finalFormat[len(TableFormatKey):]
-	case c.Format.IsJSON():
-		c.finalFormat = JSONFormat
-	}
-
-	c.finalFormat = strings.Trim(c.finalFormat, " ")
-	r := strings.NewReplacer(`\t`, "\t", `\n`, "\n")
-	c.finalFormat = r.Replace(c.finalFormat)
+	header any
+	buffer *bytes.Buffer
 }
 
 func (c *Context) parseFormat() (*template.Template, error) {
-	tmpl, err := templates.Parse(c.finalFormat)
+	tmpl, err := templates.Parse(c.Format.templateString())
 	if err != nil {
 		return nil, fmt.Errorf("template parsing error: %w", err)
 	}
 	return tmpl, nil
 }
 
 func (c *Context) postFormat(tmpl *template.Template, subContext SubContext) {
-	if c.Output == nil {
-		c.Output = io.Discard
+	out := c.Output
+	if out == nil {
+		out = io.Discard
 	}
-	if c.Format.IsTable() {
-		t := tabwriter.NewWriter(c.Output, 10, 1, 3, ' ', 0)
-		buffer := bytes.NewBufferString("")
-		tmpl.Funcs(templates.HeaderFunctions).Execute(buffer, subContext.FullHeader())
-		buffer.WriteTo(t)
-		t.Write([]byte("\n"))
-		c.buffer.WriteTo(t)
-		t.Flush()
-	} else {
-		c.buffer.WriteTo(c.Output)
+	if !c.Format.IsTable() {
+		_, _ = c.buffer.WriteTo(out)
+		return
 	}
+
+	// Write column-headers and rows to the tab-writer buffer, then flush the output.
+	tw := tabwriter.NewWriter(out, 10, 1, 3, ' ', 0)
+	_ = tmpl.Funcs(templates.HeaderFunctions).Execute(tw, subContext.FullHeader())
+	_, _ = tw.Write([]byte{'\n'})
+	_, _ = c.buffer.WriteTo(tw)
+	_ = tw.Flush()
 }
 
 func (c *Context) contextFormat(tmpl *template.Template, subContext SubContext) error {
@@ -115,8 +125,6 @@ type SubFormat func(func(SubContext) error) error
 // Write the template to the buffer using this Context
 func (c *Context) Write(sub SubContext, f SubFormat) error {
 	c.buffer = &bytes.Buffer{}
-	c.preFormat()
-
 	tmpl, err := c.parseFormat()
 	if err != nil {
 		return err

cli/command/formatter/formatter_test.go

@@ -8,20 +8,75 @@ import (
 	"testing"
 
 	"gotest.tools/v3/assert"
+	is "gotest.tools/v3/assert/cmp"
 )
 
 func TestFormat(t *testing.T) {
-	f := Format("json")
-	assert.Assert(t, f.IsJSON())
-	assert.Assert(t, !f.IsTable())
-
-	f = Format("table")
-	assert.Assert(t, !f.IsJSON())
-	assert.Assert(t, f.IsTable())
+	tests := []struct {
+		doc      string
+		f        Format
+		isJSON   bool
+		isTable  bool
+		template string
+	}{
+		{
+			doc:      "json format",
+			f:        "json",
+			isJSON:   true,
+			isTable:  false,
+			template: JSONFormat,
+		},
+		{
+			doc:      "empty table format (no template)",
+			f:        "table",
+			isJSON:   false,
+			isTable:  true,
+			template: "",
+		},
+		{
+			doc:      "table with escaped tabs",
+			f:        "table {{.Field}}\\t{{.Field2}}",
+			isJSON:   false,
+			isTable:  true,
+			template: "{{.Field}}\t{{.Field2}}",
+		},
+		{
+			doc:      "table with raw string",
+			f:        `table {{.Field}}\t{{.Field2}}`,
+			isJSON:   false,
+			isTable:  true,
+			template: "{{.Field}}\t{{.Field2}}",
+		},
+		{
+			doc:      "other format",
+			f:        "other",
+			isJSON:   false,
+			isTable:  false,
+			template: "other",
+		},
+		{
+			doc:      "other with spaces",
+			f:        "   other   ",
+			isJSON:   false,
+			isTable:  false,
+			template: "other",
+		},
+		{
+			doc:      "other with newline preserved",
+			f:        "   other\n   ",
+			isJSON:   false,
+			isTable:  false,
+			template: "other\n",
+		},
+	}
 
-	f = Format("other")
-	assert.Assert(t, !f.IsJSON())
-	assert.Assert(t, !f.IsTable())
+	for _, tc := range tests {
+		t.Run(tc.doc, func(t *testing.T) {
+			assert.Check(t, is.Equal(tc.f.IsJSON(), tc.isJSON))
+			assert.Check(t, is.Equal(tc.f.IsTable(), tc.isTable))
+			assert.Check(t, is.Equal(tc.f.templateString(), tc.template))
+		})
+	}
 }
 
 type fakeSubContext struct {