Merge be3646b87c into abb8e9b78a

cli/hints: add tests

cli/command/image/tree.go

@@ -14,6 +14,7 @@ import (
 	imagetypes "github.com/docker/docker/api/types/image"
 	"github.com/docker/docker/pkg/stringid"
 	"github.com/docker/go-units"
+	"github.com/fvbommel/sortorder"
 	"github.com/morikuni/aec"
 )
 
@@ -82,16 +83,46 @@ func runTree(ctx context.Context, dockerCLI command.Cli, opts treeOptions) error
 
 		details.ContentSize = units.HumanSizeWithPrecision(float64(totalContent), 3)
 
-		view.images = append(view.images, topImage{
-			Names:    img.RepoTags,
-			Details:  details,
-			Children: children,
-			created:  img.Created,
-		})
+		if len(img.RepoTags) == 0 {
+			// Untagged image
+			view.images = append(view.images, topImage{
+				Names:    img.RepoTags,
+				Details:  details,
+				Children: children,
+				created:  img.Created,
+			})
+		} else {
+			// Present images tagged under multiple names as separate images.
+			for _, n := range img.RepoTags {
+				view.images = append(view.images, topImage{
+					Names:    []string{n}, // Consider changing Names to be a single name for purpose of this presentation.
+					Details:  details,
+					Children: children,
+					created:  img.Created,
+				})
+			}
+		}
 	}
 
+	// Sort images alphabetically using natural-sort, with untagged images last.
 	sort.Slice(view.images, func(i, j int) bool {
-		return view.images[i].created > view.images[j].created
+		iUntagged, jUntagged := len(view.images[i].Names) == 0, len(view.images[j].Names) == 0
+		if iUntagged || jUntagged {
+			switch {
+			case iUntagged && jUntagged:
+				// Both untagged images; sort by created date (desc)
+				return view.images[i].created > view.images[j].created
+			case iUntagged:
+				// Sort untagged images last
+				return false
+			case jUntagged:
+				// Sort untagged images last
+				return true
+			}
+		}
+
+		// Sort alphabetically, ascending
+		return sortorder.NaturalLess(view.images[i].Names[0], view.images[j].Names[0])
 	})
 
 	return printImageTree(dockerCLI, view)

cli/command/registry/login.go

@@ -58,9 +58,9 @@ func NewLoginCommand(dockerCli command.Cli) *cobra.Command {
 	return cmd
 }
 
-func verifyloginOptions(dockerCli command.Cli, opts *loginOptions) error {
+func verifyLoginOptions(dockerCli command.Cli, opts *loginOptions) error {
 	if opts.password != "" {
-		fmt.Fprintln(dockerCli.Err(), "WARNING! Using --password via the CLI is insecure. Use --password-stdin.")
+		_, _ = fmt.Fprintln(dockerCli.Err(), "WARNING! Using --password via the CLI is insecure. Use --password-stdin.")
 		if opts.passwordStdin {
 			return errors.New("--password and --password-stdin are mutually exclusive")
 		}
@@ -83,7 +83,7 @@ func verifyloginOptions(dockerCli command.Cli, opts *loginOptions) error {
 }
 
 func runLogin(ctx context.Context, dockerCli command.Cli, opts loginOptions) error {
-	if err := verifyloginOptions(dockerCli, &opts); err != nil {
+	if err := verifyLoginOptions(dockerCli, &opts); err != nil {
 		return err
 	}
 	var (
@@ -174,7 +174,7 @@ func loginUser(ctx context.Context, dockerCli command.Cli, opts loginOptions, de
 		if !errors.Is(err, manager.ErrDeviceLoginStartFail) {
 			return response, err
 		}
-		fmt.Fprint(dockerCli.Err(), "Failed to start web-based login - falling back to command line login...\n\n")
+		_, _ = fmt.Fprint(dockerCli.Err(), "Failed to start web-based login - falling back to command line login...\n\n")
 	}
 
 	return loginWithUsernameAndPassword(ctx, dockerCli, opts, defaultUsername, serverAddress)

cli/command/utils.go

@@ -199,7 +199,7 @@ func PruneFilters(dockerCli Cli, pruneFilters filters.Args) filters.Args {
 // AddPlatformFlag adds `platform` to a set of flags for API version 1.32 and later.
 func AddPlatformFlag(flags *pflag.FlagSet, target *string) {
 	flags.StringVar(target, "platform", os.Getenv("DOCKER_DEFAULT_PLATFORM"), "Set platform if server is multi-platform capable")
-	flags.SetAnnotation("platform", "version", []string{"1.32"})
+	_ = flags.SetAnnotation("platform", "version", []string{"1.32"})
 }
 
 // ValidateOutputPath validates the output paths of the `export` and `save` commands.

cli/hints/hints.go

@@ -5,7 +5,9 @@ import (
 	"strconv"
 )
 
-// Enabled returns whether cli hints are enabled or not
+// Enabled returns whether cli hints are enabled or not. Hints are enabled by
+// default, but can be disabled through the "DOCKER_CLI_HINTS" environment
+// variable.
 func Enabled() bool {
 	if v := os.Getenv("DOCKER_CLI_HINTS"); v != "" {
 		enabled, err := strconv.ParseBool(v)

cli/hints/hints_test.go

@@ -0,0 +1,52 @@
+package hints
+
+import (
+	"testing"
+
+	"gotest.tools/v3/assert"
+)
+
+func TestEnabled(t *testing.T) {
+	tests := []struct {
+		doc      string
+		env      string
+		expected bool
+	}{
+		{
+			doc:      "default",
+			expected: true,
+		},
+		{
+			doc:      "DOCKER_CLI_HINTS=1",
+			env:      "1",
+			expected: true,
+		},
+		{
+			doc:      "DOCKER_CLI_HINTS=true",
+			env:      "true",
+			expected: true,
+		},
+		{
+			doc:      "DOCKER_CLI_HINTS=0",
+			env:      "0",
+			expected: false,
+		},
+		{
+			doc:      "DOCKER_CLI_HINTS=false",
+			env:      "false",
+			expected: false,
+		},
+		{
+			doc:      "DOCKER_CLI_HINTS=not-a-bool",
+			env:      "not-a-bool",
+			expected: true,
+		},
+	}
+
+	for _, tc := range tests {
+		t.Run(tc.doc, func(t *testing.T) {
+			t.Setenv("DOCKER_CLI_HINTS", tc.env)
+			assert.Equal(t, Enabled(), tc.expected)
+		})
+	}
+}