login: Add message about using PATs

[rumpl]

- What I did

Added a hint about using PATs

$ docker loginLog in with your Docker ID or email address to push and pull images from Docker Hub. If you don't have a Docker ID, head over to https://hub.docker.com/ to create one.You can log in with your password or a Personal Access Token (PAT). Using a limited-scope PAT grants better security and is required for organizations using SSO. Learn more at https://docs.docker.com/go/access-tokens/Username: 

- How I did it

- How to verify it

- Description for the changelog

- A picture of a cute animal (not mandatory but encouraged)

[codecov-commenter]

Codecov Report

Merging #4478 (9947f4b) into master (06de1f8) will decrease coverage by 0.43%.
The diff coverage is 0.00%.

❗ Current head 9947f4b differs from pull request most recent head 8d51f36. Consider uploading reports for the commit 8d51f36 to get more accurate results

Additional details and impacted files

@@ Coverage Diff @@## master #4478 +/- ##
==========================================
- Coverage 59.39% 58.97% -0.43% 
==========================================
Files 288 286 -2 Lines 24782 24779 -3 ==========================================
- Hits 14720 14614 -106 - Misses 9175 9278 +103 
Partials 887 887 

[vvoland]

0 should also be supported. It feels more natural for an env variable.

How about using strconv.ParseBool withh fallback to true (hints enabled by default) if err != nil?

[rumpl]

Maybe we should also handle "no", like yaml does? I'm joking, this check comes from compose-cli.

I kinda like only one possible value here

[rumpl]

Another nice possibility is to flip the check, have a DOCKER_NO_CLI_HINTS and only check that the variable is defined.

Examples

• cli/cli/streams/in.go

  Line 33 in 06de1f8

  if!i.isTerminal||os.Getenv("NORAW") !="" {

• cli/cli/debug/debug.go

  Line 25 in 06de1f8

  returnos.Getenv("DEBUG") !=""

• cli/cli-plugins/plugin/plugin.go

  Line 36 in 06de1f8

  ifos.Getenv("DOCKER_CLI_PLUGIN_USE_DIAL_STDIO") !="" {

[vvoland]

Personally I'd prefer DOCKER_NO_CLI_HINTS.

[thaJeztah]

DOCKER_DISABLE_CLI_HINTS would be an option.

I think we came with DOCKER_CLI_HINTS in a similar fashion as DOCKER_BUILDKIT; initially that was "opt-in", but the intent was to make that "opt-out" after incubation.

DOCKER_CLI_HINTS (and DOCKER_BUILDKIT) is more agnostic w.r.t. it being enable/disable. And can start with "opt-in" (DOCKER_CLI_HINTS=1) and when it's the default, "opt-out" (DOCKER_CLI_HINTS=0), or for situations where the config would disable it and you wanted to enable it for a specific call (through env var), as DOCKER_DISABLE_CLI_HINTS=false is kinda awkward because of the double negative.

w.r.t. accepted values, if we do look at value (beyond "non-empty"), I agree that we should use ParseBool if the value is non-empty, which would match other env-vars, such as DOCKER_BUILDKIT;

cli/cli/command/cli.go

Lines 164 to 171 in 06de1f8

	// use DOCKER_BUILDKIT env var value if set and not empty
	ifv:=os.Getenv("DOCKER_BUILDKIT"); v!="" {
	enabled, err:=strconv.ParseBool(v)
	iferr!=nil {
	returnfalse, errors.Wrap(err, "DOCKER_BUILDKIT environment variable expects boolean value")
	}
	returnenabled, nil
	}

[dvdksn]

+1 to ParseBool and DOCKER_NO_CLI_HINTS

[neersighted]

We already have DOCKER_CLI_HINTS in the wrapper (see docker/for-mac#6904 for evidence of people building on it); we might have to continue with that out of inertia, or at least propagate any changes we make here back to the Scout hints implementation (cc @eunomie)

[laurazard]

LGTM

[vvoland]

LGTM; it's beautiful now!

[thaJeztah]

We probably can't / shouldn't change the existing (old) hint, but perhaps we should use cli.Err() here (so that it can be redirected)

[thaJeztah]

Perhaps use a string-literal for this (back-tick)

[rumpl]

Didn’t use them because we don’t want newlines

[neersighted]

I'm going to merge this as-is, given no further discussion around the env var, and the desire to stay consistent (for now) with what was already done in the CLI wrapper for Scout.