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

make colored output the default #1928

Closed
wants to merge 2 commits into from
Closed

make colored output the default #1928

wants to merge 2 commits into from

Conversation

samchouse
Copy link
Contributor

This PR removes the --color flag that doesn't do anything and makes outputting colors from ran tasks the default. This PR makes it the default because that is what most people expect when switching to Turbo from NX or normal repos and loss of color shouldn't be the default.

Default:
image

--color flag:
image

--no-color flag:
image

With this PR:
image

@samchouse samchouse requested a review from a team as a code owner September 12, 2022 20:30
@vercel
Copy link

vercel bot commented Sep 12, 2022

@Xenfo is attempting to deploy a commit to the Vercel Team on Vercel.

A member of the Team first needs to authorize it.

Copy link
Contributor

@mehulkar mehulkar left a comment

Choose a reason for hiding this comment

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

This is generally what we want the end user experience to be, but we want to do it a different way:

  • We want to remove colors (and prefixes) from logs that are written to disk
  • We want to display colors (and prefixes) when printing to terminal / replay

If we just always add --color, it will not only invalidate existing caches, it will pollute the caches with the colors that may not be printable everywhere (e.g. CI systems).

Note: I'm still sort of new to this project, so I'm not 100% sure I understand correctly. @gsoltis can probably say more and whether this config change can still be accepted and without impacting the longer term solution! (See also conversation in #1349)

@gsoltis
Copy link
Contributor

gsoltis commented Sep 13, 2022

This looks like a confusion of two color settings:

  1. The color setting for turbo's output. For example, should turbo use color for task prefixes
  2. The color setting for tasks being run

The documentation for this flag definitely does not make this distinction, and it's somewhat unclear that there even ought to be a distinction here.

@mehulkar is correct in saying that we do need the ability to strip colors from logs appropriately before we can start logging them. This is unfortunately related to a large change that needs to come to log storage: moving towards just storing the raw output from the task, without any prefixes or caching information.

To move this forward, perhaps we can tackle the log replay aspect first. What about using the existing flags to control whether colors are stripped from log replay? This should initially be a no-op, since no colors are stored there, but paves the way to being able to store them safely.

@samchouse
Copy link
Contributor Author

Sure whatever you think is best, I'm willing to give it a go. I'm still a little confused though, are we trying to modify the flags to remove colors from the saved log files before we replay them?

@mehulkar
Copy link
Contributor

mehulkar commented Oct 5, 2022

@Xenfo you may be interested in #2126. It attempts--hopefully I got everything--to ensure that output from tasks that is stored on disk for caching does not include any output from turbo (i.e. no prefixes and no messages from turbo). It adds those in only during display to terminal.

I'd like to revisit this PR and what you're trying to solve after we release that (likely in 1.6)

@mehulkar mehulkar added the area: logging Improvements to logging label Jan 11, 2023
@samchouse samchouse closed this Jul 26, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: logging Improvements to logging
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants