Repository navigation
Re-land text wrapping/color PR - #22831
Conversation
6a724da to
eba2fdc
Compare
|
Sorry I missed the conversation about the test breakage. Can you describe it a bit? (or link to the bug). I was under the impression machine callers just get formatted output in json. |
|
Well, the original change broke 160 internal benchmark tests because the output format changed. The benchmarks were using the timestamps in the verbose output, and there's no JSON version of that output that I know of. Perhaps they'd work better if working with the daemon mode, but that's not how they're constructed. |
There was a problem hiding this comment.
Some of the docs in this page needs updating
There was a problem hiding this comment.
Which ones are you referring to? I looked over those three comments and didn't see anything that wasn't correct. I updated the formatting of promptForCharInput, since it didn't match the prevailing style, but that was it.
There was a problem hiding this comment.
I elaborated on the details of the default values.
There was a problem hiding this comment.
Yes, that's still right.
…color to doctor and analysis messages. (flutter#22656) This turns on text wrapping for usage messages and status messages. When on a terminal, wraps to the width of the terminal. When writing to a non-terminal, wrap lines at a default column width (currently defined to be 100 chars). If --no-wrap is specified, then no wrapping occurs. If --wrap-column is specified, wraps to that column (if --wrap is on). Adds ANSI color to the doctor and analysis output on terminals. This is in this PR with the wrapping, since wrapping needs to know how to count visible characters in the presence of ANSI sequences. (This is just one more step towards re-implementing all of Curses for Flutter. :-)) Will not print ANSI sequences when sent to a non-terminal, or of --no-color is specified. Fixes ANSI color and bold sequences so that they can be combined (bold, colored text), and a small bug in indentation calculation for wrapping. Since wrapping is now turned on, also removed many redundant '\n's in the code.
eba2fdc to
4904254
Compare
… bold on ANSI terminals.
4904254 to
0bf46e5
Compare
0a49e1b to
12c33ff
Compare
3 participants
This attempts to re-land #22656.
There are two changes from the original:
I turned off wrapping completely when not sending output to a terminal. Previously I had defaulted to wrapping at and arbitrary 100 chars in that case, just to keep long messages from being too long, but that turns out the be a bad idea because there are tests that are relying on the specific form of the output. It's also pretty arbitrary, and mostly people sending output to a non-terminal will want unwrapped text.
I found a better way to terminate ANSI color/bold sequences, so that they can be embedded within each other without needed quite as complex a dance with removing redundant sequences.
As part of these changes, I removed the
Logger.supportsColorsetter so that the one source of truth for color support is inAnsiTerminal.supportsColor.