Skip to content

internal/prompt: Confirm: don't wrap stdIn#7082

Open
thaJeztah wants to merge 1 commit into
docker:masterfrom
thaJeztah:prompt_cleans_step1
Open

internal/prompt: Confirm: don't wrap stdIn#7082
thaJeztah wants to merge 1 commit into
docker:masterfrom
thaJeztah:prompt_cleans_step1

Conversation

@thaJeztah

@thaJeztah thaJeztah commented Jul 7, 2026

Copy link
Copy Markdown
Member

This was a remnant from when PromptForConfirmation accepted a InStream; it was added in moby@30b8f08, which got left behind when cli@37ccc00 updated its signature to accept a plain io.Reader.

While updating, also update the comment to provide more context on the reason we're unconditionally using os.Stdin on Windows.

- What I did

- How I did it

- How to verify it

- Human readable description for the release notes

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

This was a remnant from when `PromptForConfirmation` accepted a `InStream`;
it was added in [moby@30b8f08], which got left behind when [cli@37ccc00]
updated its signature to accept a plain `io.Reader`.

While updating, also update the comment to provide more context on the reason
we're unconditionally using `os.Stdin` on Windows.

[moby@30b8f08]: moby/moby@30b8f08
[cli@37ccc00]: docker@37ccc00

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
@thaJeztah thaJeztah added this to the 29.7.0 milestone Jul 7, 2026
@thaJeztah thaJeztah added status/2-code-review kind/refactor PR's that refactor, or clean-up code labels Jul 7, 2026

@docker-agent docker-agent left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Assessment: 🟢 APPROVE

The change safely removes the streams.NewIn(os.Stdin) wrapper on Windows, replacing it with a bare os.Stdin. The resulting value is fed into bufio.NewScanner(in), which accepts any io.Reader — exactly what *os.File satisfies. The streams.NewIn wrapper previously added windowsconsole.NewAnsiReader processing, which caused interactive prompt hangs on Windows (moby/moby#14336, #14210, #17738). No imports are orphaned (streams is still used for DisableInputEcho). The expanded comment is accurate and helpful.

@codecov-commenter

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 0% with 1 line in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
internal/prompt/prompt.go 0.00% 1 Missing ⚠️

📢 Thoughts on this report? Let us know!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

kind/refactor PR's that refactor, or clean-up code status/2-code-review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants