Skip to content
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

Stop defaulting to color output on non-TTY #116

Merged
merged 2 commits into from
Jul 7, 2021

Conversation

ribasushi
Copy link
Contributor

This is arguably a breaking change, but there really is no legitimate case
for output like this: https://circleci.com/gh/filecoin-project/lotus/189136

The change as implemented respects any explicit setting the user might have
provided, and also keeps coloration in the URL case, in order to minimize
the potential fallout

This is arguably a breaking change, but there really is no legitimate case
for output like this: https://circleci.com/gh/filecoin-project/lotus/189136

The change as implemented respects any explicit setting the user might have
provided, and also keeps coloration in the URL case, in order to minimize
the potential fallout
@ipfs ipfs deleted a comment from welcome bot Jul 4, 2021
Copy link
Member

@Stebalien Stebalien left a comment

Choose a reason for hiding this comment

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

But you're breaking my WORKFLOW! How will I 2>&1 | tee -a lotus.log and less -R lotus.log?

setup.go Show resolved Hide resolved
@ribasushi
Copy link
Contributor Author

How will I 2>&1 | tee -a lotus.log and less -R lotus.log?

Actually... I am glad you asked! As written the code would not allow it. With the latest commit you now can:

 GOLOG_LOG_FMT=color ... 2>&1 | tee -a lotus.log

@ribasushi
Copy link
Contributor Author

@magik6k @aschmahmann any objections to this or do I just :shipit: ?

@ribasushi ribasushi merged commit 2c2975b into master Jul 7, 2021
@ribasushi ribasushi deleted the feat/no_unexpected_color branch July 7, 2021 08:53
setup.go Show resolved Hide resolved
setup.go Show resolved Hide resolved
@aschmahmann aschmahmann mentioned this pull request Aug 23, 2021
62 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants