Merge pull request #4067 from laurazard/size-flag-ps

Don't automatically request size if `--size` was explicitly set to `false`

cli/command/container/list.go

@@ -19,6 +19,7 @@ import (
 type psOptions struct {
 	quiet       bool
 	size        bool
+	sizeChanged bool
 	all         bool
 	noTrunc     bool
 	nLatest     bool
@@ -36,6 +37,7 @@ func NewPsCommand(dockerCli command.Cli) *cobra.Command {
 		Short: "List containers",
 		Args:  cli.NoArgs,
 		RunE: func(cmd *cobra.Command, args []string) error {
+			options.sizeChanged = cmd.Flags().Changed("size")
 			return runPs(dockerCli, &options)
 		},
 		Annotations: map[string]string{
@@ -78,13 +80,8 @@ func buildContainerListOptions(opts *psOptions) (*types.ContainerListOptions, er
 		options.Limit = 1
 	}
 
-	if !opts.quiet && !options.Size && len(opts.format) > 0 {
+	// always validate template when `--format` is used, for consistency
-		// The --size option isn't set, but .Size may be used in the template.
+	if len(opts.format) > 0 {
-		// Parse and execute the given template to detect if the .Size field is
-		// used. If it is, then automatically enable the --size option. See #24696
-		//
-		// Only requesting container size information when needed is an optimization,
-		// because calculating the size is a costly operation.
 		tmpl, err := templates.NewParse("", opts.format)
 		if err != nil {
 			return nil, errors.Wrap(err, "failed to parse template")
@@ -98,10 +95,21 @@ func buildContainerListOptions(opts *psOptions) (*types.ContainerListOptions, er
 			return nil, errors.Wrap(err, "failed to execute template")
 		}
 
+		// if `size` was not explicitly set to false (with `--size=false`)
+		// and `--quiet` is not set, request size if the template requires it
+		if !opts.quiet && !options.Size && !opts.sizeChanged {
+			// The --size option isn't set, but .Size may be used in the template.
+			// Parse and execute the given template to detect if the .Size field is
+			// used. If it is, then automatically enable the --size option. See #24696
+			//
+			// Only requesting container size information when needed is an optimization,
+			// because calculating the size is a costly operation.
+
 			if _, ok := optionsProcessor.FieldsUsed["Size"]; ok {
 				options.Size = true
 			}
 		}
+	}
 
 	return options, nil
 }

cli/command/container/list_test.go

@@ -231,15 +231,56 @@ func TestContainerListFormatTemplateWithArg(t *testing.T) {
 }
 
 func TestContainerListFormatSizeSetsOption(t *testing.T) {
+	tests := []struct {
+		doc, format, sizeFlag string
+		sizeExpected          bool
+	}{
+		{
+			doc:          "detect with all fields",
+			format:       `{{json .}}`,
+			sizeExpected: true,
+		},
+		{
+			doc:          "detect with explicit field",
+			format:       `{{.Size}}`,
+			sizeExpected: true,
+		},
+		{
+			doc:          "detect no size",
+			format:       `{{.Names}}`,
+			sizeExpected: false,
+		},
+		{
+			doc:          "override enable",
+			format:       `{{.Names}}`,
+			sizeFlag:     "true",
+			sizeExpected: true,
+		},
+		{
+			doc:          "override disable",
+			format:       `{{.Size}}`,
+			sizeFlag:     "false",
+			sizeExpected: false,
+		},
+	}
+
+	for _, tc := range tests {
+		tc := tc
+		t.Run(tc.doc, func(t *testing.T) {
 			cli := test.NewFakeCli(&fakeClient{
 				containerListFunc: func(options types.ContainerListOptions) ([]types.Container, error) {
-			assert.Check(t, options.Size)
+					assert.Check(t, is.Equal(options.Size, tc.sizeExpected))
 					return []types.Container{}, nil
 				},
 			})
 			cmd := newListCommand(cli)
-	cmd.Flags().Set("format", `{{.Size}}`)
+			cmd.Flags().Set("format", tc.format)
+			if tc.sizeFlag != "" {
+				cmd.Flags().Set("size", tc.sizeFlag)
+			}
 			assert.NilError(t, cmd.Execute())
+		})
+	}
 }
 
 func TestContainerListWithConfigFormat(t *testing.T) {