mirror of
https://github.com/docker/compose.git
synced 2026-10-12 02:13:08 +00:00
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 <[email protected]>
This commit is contained in:
committed by
Guillaume Lours
parent
0d7d22585a
commit
cccb9f1f7e
+10
-12
@@ -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:
|
||||
|
||||
@@ -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)
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
+1
-1
@@ -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(),
|
||||
},
|
||||
})
|
||||
}
|
||||
|
||||
+7
-1
@@ -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 (
|
||||
|
||||
Reference in New Issue
Block a user