-
Notifications
You must be signed in to change notification settings - Fork 1.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Better detection of terminals. #3611
Conversation
Will disable docker progress bars and buildpack’s coloured output in VSCode. Signed-off-by: David Gageot <[email protected]>
ae521da
to
03d5deb
Compare
Codecov Report
|
io.Writer | ||
} | ||
|
||
// If out is not a terminal, let's make sure pack doesn't output with colors |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Huh, I've never seen coloured output from pack!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lol
@@ -103,12 +102,8 @@ func isTerminal(w io.Writer) bool { | |||
return true |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is the function comment still current? Or could we just replace isTerminal
with a direct call to IsTerminal()
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm removing the comment. The test on ColoredWriteCloser
is needed here
"strings" | ||
|
||
"golang.org/x/crypto/ssh/terminal" | ||
"github.com/GoogleContainerTools/skaffold/pkg/skaffold/util" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would it be worth rewritting this code to use heroku/color
at some point?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's somewhere on my TODO...
|
||
// If out is not a terminal, let's make sure pack doesn't output with colors | ||
if _, isTerm := util.IsTerminal(out); !isTerm { | ||
color.Disable(true) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm guessing that pack
uses heroku/color
under the hood? Would be worth a comment here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok
Signed-off-by: David Gageot <[email protected]>
Will disable docker progress bars and buildpack’s coloured output in VSCode.
cc @jonjohnsonjr
Signed-off-by: David Gageot [email protected]