fix: enable markdown syntax highlighting in macOS Terminal.app
Summary
print_markdown used to bail out whenever Stdout::is_terminal() returned false. On macOS Terminal.app that check is flaky, so the CLI fell back to literal markdown (no colored headers/bold/syntax highlighting). This change centralizes the decision in should_use_colored_output(), layering multiple signals so we still feed the response through bat whenever the terminal can actually understand ANSI.
Key changes:
- treat
NO_COLORas the top-level kill-switch (env_no_color()now returns true when the var is set) - add
should_use_colored_output()that:- returns false immediately if
NO_COLOR/TERM=dumbis present - returns true if
stdout().is_terminal()succeeds - otherwise inspects
TERM,TERM_PROGRAM, andCOLORTERMso macOS Terminal.app (and other color-capable terms such asxterm-256color) still get color even whenis_terminal()lies
- returns false immediately if
- reuse that helper in both
print_markdownandrender_text_no_newlinesso all CLI paths respect the same policy - switch
bat::PrettyPrinter::colored_output(!env_no_color())to line up with the inverted, correctedenv_no_color()semantics
Type of Change
- [ ] Feature
- [x] Bug fix
- [ ] Refactor / Code quality
- [ ] Performance improvement
- [ ] Documentation
- [ ] Tests
- [ ] Security fix
- [ ] Build / Release
- [ ] Other (specify below)
Testing
cargo fmtcargo build- Manual:
- Terminal.app interactive session (main vs branch) with markdown-heavy prompts
- Pipe to
catandcat -von both builds to validate the ANSI output behaviour NO_COLOR=1run for plain-text regression coverage
Related Issues
Relates to #4468
Screenshots/Demos (for UX changes)
Before:
After:
Email: [email protected]
Thank you for the contribution, @ConstantTime ! As we wait for the dev team to review, can you also fix the DCO check? You can follow instructions here to do so
@DOsinga @alexhancock @zanesq @jamadeo markdown PR for review!
thanks for your contribution. however I am still not sure what we're trying to fix here.
your example screenshot shows that if we go through a pipe, we will now be using colors. this is not what we want, we only want to use colors when we are displaying directly to the screen. when the output is redirected you want non-ansi code because you are probably going to programmatically process it.
thanks for your contribution. however I am still not sure what we're trying to fix here.
your example screenshot shows that if we go through a pipe, we will now be using colors. this is not what we want, we only want to use colors when we are displaying directly to the screen. when the output is redirected you want non-ansi code because you are probably going to programmatically process it.
The main fix we want is as described in the original bug,
goose CLI does not display markdown syntax highlighting when running in Terminal.app on macOS, despite the terminal supporting full color and formatting capabilities
Since I didn't get any followup items addressed on this issue, I'll be taking the hacktoberfest label off of this contribution in the interim.
I will also close this issue in the interim, and hopefully you @ConstantTime or another contributor are able to get your contribution in for something like this, knowing the direction the devs want as mentioned by @DOsinga .
Since I didn't get any followup items addressed on this issue, I'll be taking the hacktoberfest label off of this contribution in the interim.
I will also close this issue in the interim, and hopefully you @ConstantTime or another contributor are able to get your contribution in for something like this, knowing the direction the devs want as mentioned by @DOsinga .
oh sorry @taniandjerry. It's been a super busy week for me at office and I missed this thread somehow. Closing this PR makes sense as per your reasoning. Going through comments from @.DOsinga to understand the gap better.