-
Notifications
You must be signed in to change notification settings - Fork 684
Implement CLI TTY/terminal width/coloring #1538
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
Conversation
...nes/reference/tsc/commandLine/show-help-with-ExitStatus.DiagnosticsPresent_OutputsSkipped.js
Outdated
Show resolved
Hide resolved
one of: none, commonjs, amd, system, umd, es6/es2015, es2020, es2022, esnext, node16, node18, nodenext, preserve | ||
default: undefined | ||
[94m --lib [39mSpecify a set of bundled library declaration files that describe the target runtime environment . | ||
one or more: es5, es6/es2015, es7/es2016, es2017, es2018, es2019, es2020, es2021, es2022, es2023, es2024, es next, dom, dom.iterable, dom.asynciterable, webworker, webworker.importscripts, webworker.itera ble, webworker.asynciterable, scripthost, es2015.core, es2015.collection, es2015.generator, es2 015.iterable, es2015.promise, es2015.proxy, es2015.reflect, es2015.symbol, es2015.symbol.wellkn own, es2016.array.include, es2016.intl, es2017.arraybuffer, es2017.date, es2017.object, es2017. sharedmemory, es2017.string, es2017.intl, es2017.typedarrays, es2018.asyncgenerator, es2018.asy nciterable/esnext.asynciterable, es2018.intl, es2018.promise, es2018.regexp, es2019.array, es20 19.object, es2019.string, es2019.symbol/esnext.symbol, es2019.intl, es2020.bigint/esnext.bigint , es2020.date, es2020.promise, es2020.sharedmemory, es2020.string, es2020.symbol.wellknown, es2 020.intl, es2020.number, es2021.promise, es2021.string, es2021.weakref/esnext.weakref, es2021.i ntl, es2022.array, es2022.error, es2022.intl, es2022.object, es2022.string, es2022.regexp, es20 23.array, es2023.collection, es2023.intl, es2024.arraybuffer, es2024.collection, es2024.object/ esnext.object, es2024.promise, es2024.regexp/esnext.regexp, es2024.sharedmemory, es2024.string/ esnext.string, esnext.array, esnext.collection, esnext.intl, esnext.disposable, esnext.promise, esnext.decorators, esnext.iterator, esnext.float16, decorators, decorators.legacy |
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.
This line looks weird, but it turns out that this is actually exactly what Strada does if you fix bugs in the test suite that prevent this code from being tested. I think we've never noticed that this was broken because most people's terminals would wrap the line.
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 now "fix" the bug here by adding the newline that was missing.
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 happy to leave it "broken", though, since it technically matches the old behavior. It would be nice if this code did word wrapping but that's another issue for another day.
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.
Pull Request Overview
This PR implements TTY/terminal width detection and coloring for the CLI help output. The key change is adding proper TTY detection to stop printing colorized output when not outputting to a TTY, along with implementing proper color support for help text formatting.
Key changes:
- Added TTY detection and terminal width querying capabilities to the system interface
- Implemented a
createColors
function with proper color code generation for different terminal environments - Updated help output formatting to use colors and terminal width-aware layout
Reviewed Changes
Copilot reviewed 12 out of 13 changed files in this pull request and generated 2 comments.
Show a summary per file
File | Description |
---|---|
internal/execute/system.go | Added new system interface methods for TTY detection, terminal width, and environment variables |
internal/execute/outputs.go | Implemented color system and updated help formatting to use colors and terminal width |
internal/core/tristate.go | Added IsUnknown() method for tristate values |
cmd/tsgo/sys.go | Implemented TTY detection and terminal width querying for the OS system |
internal/execute/testsys_test.go | Added test system support for TTY/terminal width simulation |
internal/execute/tsc_test.go | Updated test cases to use environment variables for terminal width testing |
internal/execute/tsctestrunner_test.go | Added environment variable support to test runner |
go.mod | Added golang.org/x/term dependency |
testdata/baselines/reference/*.js | Updated test baselines showing colorized help output |
Is there a way to bypass this - print pretty results in an environment that's detected as a TTY? @jakebailey |
Yes, If you're seeing this behave differently to tsc, please file an issue. |
TTY detection means we'll stop printing out pretty results by default without a TTY.
With that in place, we can implement
createColors
and do the colorization code for the rest of the CLI. I've implemented that, though it's likely not optimal. Not really a big deal given what I implemented is mainly code for--help
or similar.