rust-lang / rust-lang/rust

rustdoc: Fix output coloring for doctest building & running

Open
#148,868 1 comment 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

A-doctests C-bug C-tracking-issue T-rustdoc
Dominant language
Rust
Stars
119k
Forks
16.1k
PR merge metrics
PR metrics pending

Description

Follow-up to PR https://github.com/rust-lang/rust/pull/148834 and issue https://github.com/rust-lang/rust/issues/148749.

  • Add a regression test for issue https://github.com/rust-lang/rust/issues/148749
    • we want to check that --color=auto (the default) is respected when rendering doctest build errors
    • we can't test that directly because the output streams are obviously redirected into golden files *.{stdout,stderr} by compiletest which isn't a terminal, so --color=auto becomes indistinguishable from --color=never
    • therefore I was thinking of utilizing CLICOLOR_FORCE=1 to indirectly detect auto working
    • however that's blocked on https://github.com/rust-lang/rust/issues/148864 unless we find another solution
    • ideally we could use compiletest's SVG test mechanism but it's unclear if it'll work out of the box for us (like rendering the STDOUT as an SVG, not STDERR)
    • if worst comes to the worst we'll have to persist the ANSI codes until compiletest is patched
    • the test would look something like:
      // EXPLAINER
      // issue: <https://github.com/rust-lang/rust/issues/148749>
      //@ compile-flags: --test --color=auto --error-format=human
      //@ rustc-env:CLICOLOR_FORCE=1
      //@ normalize-stdout: "finished in \d+\.\d+s" -> "finished in $$TIME"
      
      //! ```
      //! undefined
      //! ```
      
  • Also run "color capability detection" for lexically / syntactically invalid doctests
    • if we fail to lex or parse the doctest, we construct a dummy doctest for which we currently unconditionally set supports_color to false for no apparent reason
    • that means we don't print syntax error diagnostics with colors by default which is bad
  • In doctest::make::parse_source, run "color capability detection" on STDOUT by default not STDERR since we / libtest are outputting these diagnostics to STDOUT by default (as part of its normal output)
    • see also https://github.com/rust-lang/rust/pull/148834#discussion_r2515957544
    • this means rustdoc doctest.rs --test | cat still outputs colors STDOUT despite us piping STDOUT to a "non-terminal"
    • it also means that rustdoc doctest.rs --test 2>/dev/null outputs no colors to STDOUT despite STDOUT being a terminal
    • the current behavior is only correct for --no-capture (https://github.com/rust-lang/rust/issues/148116): rustdoc doctest.rs --test --no-capture -Zunstable-options 2>/dev/null correctly disables colors for STDERR where the diagnostics land
    • I don't think if rustdoc_options.no_capture { /* query STDOUT */ } else { /* query STDERR */ } is sufficient
    • that's partly because there's also --test-args --no-capture (i.e., libtest's non-capturing mode)
      • I don't actually know how they differ and why rustdoc has its own non-capturing mode
      • not only do we want to query STDERR if !no_capture, we basically also want to query STDERR if --test-args --no-capture (however scanning libtest's arguments would be super janky) so rustdoc doctest.rs --test --test-args --no-capture 2>/dev/null correctly suppresses colors for STDERR
    • do we actually want / need to query both output streams in certain cases? Maybe
    • cc https://github.com/rust-lang/rust/issues/148864#issuecomment-3522037322

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start with doctest::make::parse_source and the linked PR 148834, then review issue 148749 and the compiletest golden-file behavior. Add regression coverage for --color=auto and address color capability detection for invalid doctests and the appropriate output stream. Done means doctest diagnostics respect terminal and color settings in the described cases.

Written by the indexing model from the issue text.

Assessment

Tech stack
rust
Domain
compilers, testing-qa
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Stale
Clarity
Needs clarification
Newbie friendliness
28/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.