fix(output): narrow rows for every column format, not just the three spelled exactly (#353)

internal/commands/config.go

@@ -20,6 +20,7 @@ import (
 	"github.com/Jamf-Concepts/jamf-cli/internal/config"
 	"github.com/Jamf-Concepts/jamf-cli/internal/exitcode"
 	"github.com/Jamf-Concepts/jamf-cli/internal/keychain"
+	"github.com/Jamf-Concepts/jamf-cli/internal/output"
 	"github.com/Jamf-Concepts/jamf-cli/internal/registry"
 )
 
@@ -98,25 +99,30 @@ type configProfileTableRow struct {
 // environment-id is the column. A tenant-scoped profile is not left unexplained
 // — auth-method already reads `platform`, and `config list -o json` still
 // carries tenant-id for anything parsing the output.
+//
+// The narrow shape is the default and the keep-set is named, rather than the
+// other way round, because the format string is not normalised anywhere: the
+// switch used to match "table", "csv" and "plain" exactly and return the wide
+// shape for everything else, so `config list -o Table` took the wide shape to
+// the table renderer and lost the column this function exists to guarantee.
+// See output.RendersStructureVerbatim.
 func listRowsForFormat(rows []configProfileRow, format string) any {
-	switch format {
-	case "table", "csv", "plain":
-		out := make([]configProfileTableRow, 0, len(rows))
-		for _, r := range rows {
-			out = append(out, configProfileTableRow{
-				Name:          r.Name,
-				URL:           r.URL,
-				AuthMethod:    r.AuthMethod,
-				EnvironmentID: r.EnvironmentID,
-				Default:       r.Default,
-				Status:        r.Status,
-				Healthy:       r.Healthy,
-			})
-		}
-		return out
-	default:
+	if output.RendersStructureVerbatim(format) {
 		return rows
 	}
+	out := make([]configProfileTableRow, 0, len(rows))
+	for _, r := range rows {
+		out = append(out, configProfileTableRow{
+			Name:          r.Name,
+			URL:           r.URL,
+			AuthMethod:    r.AuthMethod,
+			EnvironmentID: r.EnvironmentID,
+			Default:       r.Default,
+			Status:        r.Status,
+			Healthy:       r.Healthy,
+		})
+	}
+	return out
 }
 
 // activeProfileName returns the profile currently in effect: flag > env > default.

internal/commands/config_subcommands_test.go

@@ -733,3 +733,68 @@ func TestConfigAutoDefault_FirstProfile(t *testing.T) {
 		t.Errorf("DefaultProfile = %q, want %q (should not change when adding second profile)", reloaded.DefaultProfile, "first")
 	}
 }
+
+// TestConfigList_MisCasedFormatKeepsTheTableColumns is issue 353. The format
+// string is never normalised — output.New takes the --output value verbatim and
+// ResolveFormat returns it untouched — while Print's switch has no case for
+// "Table" and renders a table through its default arm. So a switch matching
+// "table" exactly handed the wide row type to a table renderer, and the wide
+// type is the one carrying omitempty on the fields that must always be columns.
+func TestConfigList_MisCasedFormatKeepsTheTableColumns(t *testing.T) {
+	// Three mis-casings and an unrecognised value, all of which Print renders
+	// as a table through its default arm.
+	for _, format := range []string{"Table", "TABLE", "Csv", "wibble"} {
+		t.Run(format, func(t *testing.T) {
+			out := runConfigList(t, format, configListScopeFixture)
+
+			if !strings.Contains(out, "ENVIRONMENT-ID") {
+				t.Errorf("-o %s renders a table with no ENVIRONMENT-ID column, so the scope of every platform profile is invisible:\n%s", format, out)
+			}
+			if !strings.Contains(out, "11111111-2222-3333-4444-555555555555") {
+				t.Errorf("-o %s drops the environment ID value:\n%s", format, out)
+			}
+			if !strings.Contains(out, "DEFAULT") {
+				t.Errorf("-o %s renders a table with no DEFAULT column:\n%s", format, out)
+			}
+		})
+	}
+}
+
+// json-multi is the one value that has to be narrowed while not rendering a
+// table: internal/commands/multi.go sets it as the capture format, so it writes
+// JSON on the wire and multi re-renders that JSON as a table afterwards. It is
+// therefore excluded from output.RendersStructureVerbatim, and this asserts the
+// consequence — the captured rows carry the column shape, not the wide one —
+// because a keep-set that included it would put the wide shape on a terminal by
+// way of `jamf-cli multi`.
+func TestConfigList_JSONMultiCapturesTheColumnShape(t *testing.T) {
+	out := runConfigList(t, "json-multi", configListScopeFixture)
+
+	var rows []map[string]any
+	if err := json.Unmarshal([]byte(out), &rows); err != nil {
+		t.Fatalf("json-multi output is not valid JSON: %v\n%s", err, out)
+	}
+	if len(rows) == 0 {
+		t.Fatal("json-multi captured no rows")
+	}
+	for _, r := range rows {
+		if _, ok := r["tenant-id"]; ok {
+			t.Errorf("json-multi carries tenant-id, so it took the wide row type: %v", r)
+		}
+		if _, ok := r["environment-id"]; !ok {
+			t.Errorf("json-multi row omits environment-id, so the column would vanish when multi renders it: %v", r)
+		}
+	}
+}
+
+// The keep-set is the point of the inversion, so it is asserted rather than
+// assumed: a format that renders a structure verbatim must still get the wide
+// row type, tenant-id included.
+func TestConfigList_StructuredFormatsStillGetTheWideRowType(t *testing.T) {
+	for _, format := range []string{"json", "yaml", "ndjson"} {
+		out := runConfigList(t, format, configListScopeFixture)
+		if !strings.Contains(out, "aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee") {
+			t.Errorf("-o %s dropped tenant-id, so the narrowing reached a structured format:\n%s", format, out)
+		}
+	}
+}

internal/commands/protect_helpers.go

@@ -8,24 +8,30 @@ import (
 	"io"
 	"os"
 
+	"github.com/Jamf-Concepts/jamf-cli/internal/output"
 	"github.com/Jamf-Concepts/jamf-cli/internal/protect"
 	"github.com/Jamf-Concepts/jamf-cli/internal/registry"
 	"gopkg.in/yaml.v3"
 )
 
-// printResult outputs a single item. For table/csv/plain, it uses the
-// flattened map for clean column output. For json/yaml, it outputs the full struct.
+// printResult outputs a single item. The column formats get the flattened map
+// for clean column output; json, yaml, ndjson, xml and raw get the full struct.
+//
+// The keep-set is named and the flattened shape is the default, rather than the
+// other way round, because the format string is not normalised: this used to
+// match "table", "csv" and "plain" exactly, so any other value — a mis-cased
+// -o Table, or the internal json-multi that means JSON on the wire and a table
+// on the screen — took the full struct to a table renderer. See
+// output.RendersStructureVerbatim.
 func printResult(out registry.OutputFormatter, item any, flattened map[string]any) error {
-	switch outputFmt {
-	case "table", "csv", "plain":
-		data, err := json.Marshal(flattened)
-		if err != nil {
-			return fmt.Errorf("marshalling output: %w", err)
-		}
-		return out.PrintRaw(data)
-	default:
+	if output.RendersStructureVerbatim(outputFmt) {
 		return protect.PrintOne(out, item)
 	}
+	data, err := json.Marshal(flattened)
+	if err != nil {
+		return fmt.Errorf("marshalling output: %w", err)
+	}
+	return out.PrintRaw(data)
 }
 
 // printExport outputs data as JSON (default) or YAML based on the global output format.

internal/commands/protect_helpers_test.go

@@ -5,7 +5,10 @@ package commands
 import (
 	"os"
 	"path/filepath"
+	"strings"
 	"testing"
+
+	"github.com/Jamf-Concepts/jamf-cli/internal/registry"
 )
 
 type testInput struct {
@@ -138,3 +141,71 @@ func TestWriteBase64File_Permissions(t *testing.T) {
 		})
 	}
 }
+
+// TestPrintResult_MisCasedFormatStillFlattens is issue 353 at this call site.
+// The switch matched "table", "csv" and "plain" exactly, so a mis-cased
+// -o Table — which Print renders as a table through its default arm — took the
+// full nested struct to a table renderer instead of the flattened map the
+// caller built for exactly that purpose.
+func TestPrintResult_MisCasedFormatStillFlattens(t *testing.T) {
+	type nested struct {
+		Name  string         `json:"name"`
+		Inner map[string]any `json:"inner"`
+	}
+	item := nested{Name: "plan-a", Inner: map[string]any{"deep": "value"}}
+	flattened := map[string]any{"name": "plan-a", "inner.deep": "value"}
+
+	old := outputFmt
+	defer func() { outputFmt = old }()
+
+	for _, format := range []string{"Table", "TABLE", "Csv", "json-multi", "wibble"} {
+		outputFmt = format
+		out := &captureRawFormatter{}
+		if err := printResult(out, item, flattened); err != nil {
+			t.Fatalf("printResult(-o %s): %v", format, err)
+		}
+		if !strings.Contains(string(out.raw), "inner.deep") {
+			t.Errorf("-o %s did not print the flattened map, so a column renderer got the nested struct: %s", format, out.raw)
+		}
+	}
+}
+
+// The keep-set still gets the full struct, which is what printResult narrows
+// away from.
+func TestPrintResult_StructuredFormatsGetTheFullStruct(t *testing.T) {
+	type nested struct {
+		Name  string         `json:"name"`
+		Inner map[string]any `json:"inner"`
+	}
+	item := nested{Name: "plan-a", Inner: map[string]any{"deep": "value"}}
+	flattened := map[string]any{"name": "plan-a", "inner.deep": "value"}
+
+	old := outputFmt
+	defer func() { outputFmt = old }()
+
+	for _, format := range []string{"json", "yaml", "ndjson", "xml", "raw"} {
+		outputFmt = format
+		out := &captureRawFormatter{}
+		if err := printResult(out, item, flattened); err != nil {
+			t.Fatalf("printResult(-o %s): %v", format, err)
+		}
+		if strings.Contains(string(out.raw), "inner.deep") {
+			t.Errorf("-o %s printed the flattened map, so the narrowing reached a structured format: %s", format, out.raw)
+		}
+		if !strings.Contains(string(out.raw), `"inner"`) {
+			t.Errorf("-o %s lost the nested struct: %s", format, out.raw)
+		}
+	}
+}
+
+// captureRawFormatter records what printResult writes. Only PrintRaw is
+// implemented; protect.PrintOne routes through it too.
+type captureRawFormatter struct {
+	registry.OutputFormatter
+	raw []byte
+}
+
+func (c *captureRawFormatter) PrintRaw(data []byte) error {
+	c.raw = data
+	return nil
+}

internal/output/output.go

@@ -204,6 +204,34 @@ func IsMachineRendered(format Format) bool {
 	return false
 }
 
+// RendersStructureVerbatim reports whether a format renders a value's own
+// structure rather than projecting it into columns.
+//
+// It exists so a command that narrows its row type for the column formats can
+// name the formats that keep the wide shape, instead of naming the column ones
+// and returning the wide shape for everything else. That polarity matters
+// because the format string is never normalised: New takes the --output value
+// verbatim and ResolveFormat returns it untouched, so "Table" is not
+// FormatTable, and Print's own switch has no case for it and renders a table
+// through the default arm. A command matching "table" exactly therefore handed
+// its wide shape to a table renderer, which is how `config list -o Table` came
+// to drop a column `config list -o table` shows (issue 353). Anything
+// unrecognised renders as a table, so the narrow shape is the matching one.
+//
+// FormatJSONMulti is deliberately absent: it means JSON on the wire and a
+// table on the screen (internal/commands/multi.go sets it as the capture
+// format, and Print has no case for it either), so keeping the wide shape for
+// it would put that shape back on a terminal by way of `jamf-cli multi`. The
+// generated selectTableColumns excludes it from its own keep-set for the same
+// reason, and this function is that set.
+func RendersStructureVerbatim(format string) bool {
+	switch Format(format) {
+	case FormatJSON, FormatYAML, FormatNDJSON, FormatXML, FormatRaw:
+		return true
+	}
+	return false
+}
+
 // Print outputs data in the configured format
 func (f *Formatter) Print(data any) error {
 	if rows, ok := data.([]map[string]any); ok {

internal/output/output_test.go

@@ -1837,3 +1837,29 @@ func TestPrintNDJSON_SelectProjection(t *testing.T) {
 		}
 	}
 }
+
+// TestRendersStructureVerbatim pins the keep-set, and pins the polarity that
+// the set exists to express: everything not named renders as columns, so
+// everything not named must take a command's narrowed row type. The default
+// arm is the one that was wrong at three call sites (issue 353).
+func TestRendersStructureVerbatim(t *testing.T) {
+	for _, format := range []string{"json", "yaml", "ndjson", "xml", "raw"} {
+		if !RendersStructureVerbatim(format) {
+			t.Errorf("%q is not in the keep-set, so a command would flatten a format that renders structure verbatim", format)
+		}
+	}
+
+	// Three column formats, the internal capture format, four mis-casings and
+	// an unrecognised value. Print renders a table for all but json-multi, and
+	// json-multi is JSON on the wire that multi re-renders as a table — so
+	// every one of them wants the column shape.
+	for _, format := range []string{
+		"table", "csv", "plain", "json-multi",
+		"Table", "TABLE", "JSON", "Yaml",
+		"wibble", "",
+	} {
+		if RendersStructureVerbatim(format) {
+			t.Errorf("%q is in the keep-set, so a command would hand its wide row type to a column renderer", format)
+		}
+	}
+}

internal/scope/scope.go

@@ -16,6 +16,7 @@ import (
 
 	"github.com/spf13/cobra"
 
+	"github.com/Jamf-Concepts/jamf-cli/internal/output"
 	"github.com/Jamf-Concepts/jamf-cli/internal/registry"
 )
 
@@ -303,28 +304,35 @@ func RemoveFromScope(s *ScopeXML, singularKey, section, flagName, name string) b
 	return removeNamedItem(readScopeItems(s, section, flagName), name)
 }
 
-// OutputScope writes the scope to the output formatter. For table/csv/plain formats
-// it flattens the scope into rows; for json/yaml it outputs the full structure.
+// OutputScope writes the scope to the output formatter. The column formats get
+// the scope flattened into rows; json, yaml, ndjson, xml and raw get the full
+// structure.
+//
+// The keep-set is named and the flattened shape is the default, rather than the
+// other way round, because the format string is not normalised: this used to
+// match "table", "csv" and "plain" exactly, so any other value — a mis-cased
+// -o Table, or the internal json-multi that means JSON on the wire and a table
+// on the screen — took the nested structure to a table renderer. See
+// output.RendersStructureVerbatim.
 func OutputScope(out registry.OutputFormatter, s *ScopeXML, singularKey, format string) error {
-	switch format {
-	case "table", "csv", "plain":
-		rows := FlattenScope(s, singularKey)
-		if len(rows) == 0 {
-			fmt.Fprintln(os.Stderr, "Scope is empty")
-			return nil
-		}
-		data, err := json.Marshal(rows)
-		if err != nil {
-			return err
-		}
-		return out.PrintRaw(data)
-	default:
+	if output.RendersStructureVerbatim(format) {
 		data, err := json.Marshal(s)
 		if err != nil {
 			return err
 		}
 		return out.PrintRaw(data)
 	}
+
+	rows := FlattenScope(s, singularKey)
+	if len(rows) == 0 {
+		fmt.Fprintln(os.Stderr, "Scope is empty")
+		return nil
+	}
+	data, err := json.Marshal(rows)
+	if err != nil {
+		return err
+	}
+	return out.PrintRaw(data)
 }
 
 // FlattenScope converts a ScopeXML into a flat list of rows for table output.