From 6d708bf4ec8b3d17443100987de2a25fc2bd0597 Mon Sep 17 00:00:00 2001 From: Nicolas De Loof Date: Thu, 20 Aug 2026 14:06:52 +0200 Subject: [PATCH] cli: display.Mode always resolves to the mode actually rendered The display.Mode package global was left as "auto" on the most common path (auto mode with an interactive or redirected terminal), so readers like `up`'s NavigationMenu gate saw an unresolved value; that gate also compared against the string literal "plain" instead of the constant. applyDisplayMode wrote a mode that selectEventProcessor could then contradict (--ansi always with a redirected stderr rendered Plain while Mode said tty). selectEventProcessor becomes the single resolution point: every branch assigns the mode matching the renderer it returns, so after command setup the global never holds ModeAuto. applyDisplayMode loses its redundant writes and its misleading name (now applyAnsiMode); the contract is documented on the Mode variable, and the existing selectEventProcessor tests now pin the resolved mode for every branch. Side effect, deliberate: with stderr redirected (auto -> plain rendering) the navigation menu is no longer offered; it used to slip through because Mode still read "auto". Part of #14074 (section E). Signed-off-by: Nicolas De Loof --- cmd/compose/compose.go | 22 ++++++++++------------ cmd/compose/compose_progress_test.go | 16 ++++++++++++++++ cmd/compose/up.go | 2 +- cmd/display/mode.go | 8 +++++++- 4 files changed, 34 insertions(+), 14 deletions(-) diff --git a/cmd/compose/compose.go b/cmd/compose/compose.go index 58e519ad9cb..52daddcb364 100644 --- a/cmd/compose/compose.go +++ b/cmd/compose/compose.go @@ -481,7 +481,7 @@ func RootCommand(dockerCli command.Cli, backendOptions *BackendOptions) *cobra.C if err != nil { return err } - applyDisplayMode(dockerCli, ansi) + applyAnsiMode(dockerCli, ansi) detached, _ := cmd.Flags().GetBool("detach") ep, err := selectEventProcessor(dockerCli, opts.Progress, ansi, detached) @@ -619,22 +619,16 @@ func resolveAnsiMode(cmd *cobra.Command, ansi string, noAnsi bool) (string, erro return ansi, nil } -// applyDisplayMode configures ANSI output and the progress display mode, -// honoring the NO_COLOR convention (https://no-color.org). -func applyDisplayMode(dockerCli command.Cli, ansi string) { +// applyAnsiMode configures ANSI output, honoring the NO_COLOR convention +// (https://no-color.org). The progress display mode is resolved separately, +// by selectEventProcessor. +func applyAnsiMode(dockerCli command.Cli, ansi string) { formatter.SetANSIMode(dockerCli, ansi) if noColor, ok := os.LookupEnv("NO_COLOR"); ok && noColor != "" { display.NoColor() formatter.SetANSIMode(dockerCli, formatter.Never) } - - switch ansi { - case "never": - display.Mode = display.ModePlain - case "always": - display.Mode = display.ModeTTY - } } // normalizeProjectOptions handles the deprecated --workdir flag and makes @@ -689,7 +683,9 @@ func stdinfo(dockerCli command.Cli) io.Writer { return dockerCli.Err() } -// selectEventProcessor picks the EventProcessor for Compose progress rendering. +// selectEventProcessor picks the EventProcessor for Compose progress rendering, +// and resolves display.Mode to the mode actually rendered: every branch assigns +// it, so after command setup the global never holds ModeAuto. // // In auto mode we probe Err() (not Out()) because the renderer writes to stderr; // probing stdout would force plain mode whenever stdout is redirected (e.g. @@ -702,8 +698,10 @@ func selectEventProcessor(dockerCli command.Cli, progress, ansi string, detached display.Mode = display.ModePlain return display.Plain(dockerCli.Err()), nil case dockerCli.Err().IsTerminal(): + display.Mode = display.ModeTTY return display.Full(dockerCli.Err(), stdinfo(dockerCli), detached), nil default: + display.Mode = display.ModePlain return display.Plain(dockerCli.Err()), nil } case display.ModeTTY: diff --git a/cmd/compose/compose_progress_test.go b/cmd/compose/compose_progress_test.go index c94760ec66c..5756782f113 100644 --- a/cmd/compose/compose_progress_test.go +++ b/cmd/compose/compose_progress_test.go @@ -86,18 +86,21 @@ func TestSelectEventProcessor_AutoMode(t *testing.T) { errIsTTY bool ansi string wantType string + wantMode string }{ { name: "stderr TTY, stdout piped -> Full", errIsTTY: true, ansi: "auto", wantType: "*display.ttyWriter", + wantMode: display.ModeTTY, }, { name: "stderr piped, stdout TTY -> Plain (do not fall back to stdout)", outIsTTY: true, ansi: "auto", wantType: "*display.plainWriter", + wantMode: display.ModePlain, }, { name: "both TTY -> Full", @@ -105,11 +108,13 @@ func TestSelectEventProcessor_AutoMode(t *testing.T) { errIsTTY: true, ansi: "auto", wantType: "*display.ttyWriter", + wantMode: display.ModeTTY, }, { name: "both piped -> Plain", ansi: "auto", wantType: "*display.plainWriter", + wantMode: display.ModePlain, }, { name: "ansi never forces Plain even when stderr is TTY", @@ -117,6 +122,7 @@ func TestSelectEventProcessor_AutoMode(t *testing.T) { errIsTTY: true, ansi: "never", wantType: "*display.plainWriter", + wantMode: display.ModePlain, }, } @@ -128,6 +134,8 @@ func TestSelectEventProcessor_AutoMode(t *testing.T) { ep, err := selectEventProcessor(cli, "", tc.ansi, false) assert.NilError(t, err) assert.Equal(t, fmt.Sprintf("%T", ep), tc.wantType) + // the global must hold the mode actually rendered, never ModeAuto + assert.Equal(t, display.Mode, tc.wantMode) }) } } @@ -138,12 +146,14 @@ func TestSelectEventProcessor_ExplicitMode(t *testing.T) { progress string ansi string wantType string + wantMode string wantErrText string }{ { name: "progress=tty forces Full regardless of streams", progress: display.ModeTTY, ansi: "auto", + wantMode: display.ModeTTY, wantType: "*display.ttyWriter", }, { @@ -156,6 +166,7 @@ func TestSelectEventProcessor_ExplicitMode(t *testing.T) { name: "progress=plain forces Plain", progress: display.ModePlain, ansi: "auto", + wantMode: display.ModePlain, wantType: "*display.plainWriter", }, { @@ -168,18 +179,21 @@ func TestSelectEventProcessor_ExplicitMode(t *testing.T) { name: "progress=quiet returns Quiet", progress: display.ModeQuiet, ansi: "auto", + wantMode: display.ModeQuiet, wantType: "*display.quiet", }, { name: `progress="none" aliases to Quiet`, progress: "none", ansi: "auto", + wantMode: display.ModeQuiet, wantType: "*display.quiet", }, { name: "progress=json returns JSON", progress: display.ModeJSON, ansi: "auto", + wantMode: display.ModeJSON, wantType: "*display.jsonWriter", }, { @@ -204,6 +218,8 @@ func TestSelectEventProcessor_ExplicitMode(t *testing.T) { } assert.NilError(t, err) assert.Equal(t, fmt.Sprintf("%T", ep), tc.wantType) + // the global must hold the mode actually rendered, never ModeAuto + assert.Equal(t, display.Mode, tc.wantMode) }) } } diff --git a/cmd/compose/up.go b/cmd/compose/up.go index 14e071ecbba..e69e455e27c 100644 --- a/cmd/compose/up.go +++ b/cmd/compose/up.go @@ -345,7 +345,7 @@ func runUp( WaitTimeout: timeout, Watch: upOptions.watch, Services: services, - NavigationMenu: upOptions.navigationMenu && display.Mode != "plain" && dockerCli.In().IsTerminal(), + NavigationMenu: upOptions.navigationMenu && display.Mode != display.ModePlain && dockerCli.In().IsTerminal(), }, }) } diff --git a/cmd/display/mode.go b/cmd/display/mode.go index d66777b472c..c8da90a21ca 100644 --- a/cmd/display/mode.go +++ b/cmd/display/mode.go @@ -16,7 +16,13 @@ package display -// Mode define how progress should be rendered, either as ModePlain or ModeTTY +// Mode is the effective progress rendering mode for the current command. +// +// It starts as ModeAuto and is resolved during command setup by +// selectEventProcessor (cmd/compose), which assigns the mode matching the +// renderer it returns — so code running after setup never observes ModeAuto. +// The only other writers are the `--quiet` flags of `run` and `build`, which +// force ModeQuiet from their PreRun hooks. var Mode = ModeAuto const (