Skip to content

cli/streams: assorted cleanups#6900

Open
thaJeztah wants to merge 4 commits intodocker:masterfrom
thaJeztah:cli_stream_cleanups
Open

cli/streams: assorted cleanups#6900
thaJeztah wants to merge 4 commits intodocker:masterfrom
thaJeztah:cli_stream_cleanups

Conversation

@thaJeztah
Copy link
Copy Markdown
Member

I'm working on some related changes, but let's clean this up first

cli/streams: (In|Out).SetRawTerminal: dry

Move the code to the commonStream type, which is where the actual
state field is kept.

cli/streams: move constructors to the start

It's more idiomatic to define the constructor before methods.

cli/streams: don't depend on embedding

Define explicit wrapper methods instead of depending on the embedded
commonStreams struct.

cli/streams: simplify CheckTty and add go:fix to inline it

This function is very specific to attaching to containers, and probably
helps clarity to inline it where used.

- Human readable description for the release notes

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

Move the code to the commonStream type, which is where the actual
state field is kept.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
It's more idiomatic to define the constructor before methods.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Define explicit wrapper methods instead of depending on the embedded
commonStreams struct.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
This function is very specific to attaching to containers, and probably
helps clarity to inline it where used.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
@thaJeztah thaJeztah added this to the 29.3.2 milestone Apr 1, 2026
@thaJeztah thaJeztah added status/2-code-review kind/refactor PR's that refactor, or clean-up code labels Apr 1, 2026
@codecov-commenter
Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 0% with 58 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
cli/streams/stream.go 0.00% 26 Missing ⚠️
cli/streams/in.go 0.00% 17 Missing ⚠️
cli/streams/out.go 0.00% 15 Missing ⚠️

📢 Thoughts on this report? Let us know!

Comment on lines -66 to -68
if runtime.GOOS == "windows" {
return errors.New(eText + ". If you are using mintty, try prefixing the command with 'winpty'")
}
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Shouldn't we keep this?

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