--pretty-er output by default#23408
Conversation
weswigham
left a comment
There was a problem hiding this comment.
Do we... Have a way we can test this? Oh! Maybe We could write a unit test that shells out to the built local tsc and baselines the output!
Locals pretty straightforward; you got any details on what terms/apps this causes pretty by default in now?
8c5b163 to
7fd1dda
Compare
|
I know the last time I did this (i.e. when I originally wrote Tests are Monday-Daniel-and-Wesley's problem. 😄 |
src/compiler/tsc.ts
Outdated
| function shouldBePretty(options: CompilerOptions) { | ||
| if ((typeof options.pretty === "undefined" && typeof options.diagnosticStyle === "undefined") || options.diagnosticStyle === DiagnosticStyle.Auto) { | ||
| return !!sys.writeOutputIsTty && sys.writeOutputIsTty(); | ||
| } |
src/compiler/commandLineParser.ts
Outdated
| pretty: DiagnosticStyle.Pretty, | ||
| simple: DiagnosticStyle.Simple, | ||
| }), | ||
| }, |
| } | ||
| } | ||
|
|
||
| function shouldBePretty(options: CompilerOptions) { |
There was a problem hiding this comment.
So now there are two CLI flags that can tell tsc it's pretty. What if the user provides tsc --pretty false --diagnosticStyle pretty or tsc --pretty true --diagnosticStyle simple? Should there be a warning for providing both?
src/compiler/commandLineParser.ts
Outdated
| category: Diagnostics.Command_line_Options, | ||
| description: Diagnostics.Stylize_errors_and_messages_using_color_and_context_experimental | ||
| }, | ||
| { |
There was a problem hiding this comment.
I would rather we do not add a new flag if we can. i think auto is just pretty === undefined.
if we really need a new value i would make pretty take boolean | "auto"
There was a problem hiding this comment.
boolean | "auto" | "simple" | "styled"? (Where boolean toggles between auto and styled).
src/compiler/tsc.ts
Outdated
| if ((typeof options.pretty === "undefined" && typeof options.diagnosticStyle === "undefined") || options.diagnosticStyle === DiagnosticStyle.Auto) { | ||
| return !!sys.writeOutputIsTty && sys.writeOutputIsTty(); | ||
| } | ||
| return options.diagnosticStyle === DiagnosticStyle.Pretty || options.pretty; |
mhegazy
left a comment
There was a problem hiding this comment.
Please 1. remove the extra flag, and 2. test on chakra
src/compiler/sys.ts
Outdated
| newLine: string; | ||
| useCaseSensitiveFileNames: boolean; | ||
| write(s: string): void; | ||
| writeOutputIsTty?(): boolean; |
src/compiler/tsc.ts
Outdated
| } | ||
|
|
||
| function shouldBePretty(options: CompilerOptions) { | ||
| if ((typeof options.pretty === "undefined")) { |
29ff745 to
25bb581
Compare
|
Tried it out. tsc.exe (the Chakra host-wrapped |
I guess this is because tsc.exe doesn't set the console mode on Windows to enable processing of Virtual Terminal Sequences ( (I think Node.js parses these sequences itself and then calls the corresponding APIs like Can tsc.exe be updated to set this console mode? Thanks! |
|
Thanks for the pointers, I might look into it @kpreisser! |
This pull request always makes TypeScript's output
--prettywhen the compiler can detect whether its "output device" is appropriate for more colorful output.If TypeScript's output is a psuedo-TTY, then it will enable today's prettified output and emit escape characters to the console; however, when piping to another file or program, it will automatically turned off.
Programs that want to specify the behavior can specifically set
--pretty, or the new--diagnosticStyleflag.This PR also affords us the room to give more variation in out emit modes. For example, with
--diagnosticStyle, we could have a more colorful simple/terse mode as requested in #10745.Fixes #10488.