[ty] Add a `--quiet` mode by zanieb · Pull Request #19233 · astral-sh/ruff

zanieb · 2025-07-09T12:30:30Z

Adds a --quiet flag which silences diagnostic, warning logs, and messages like "all checks passed" while retaining summary messages that indicate problems, e.g., the number of diagnostics.

I'm a bit on the fence regarding filtering out warning logs, because it can omit important details, e.g., the message that a fatal diagnostic was encountered. Let's discuss that in #19233 (comment)

The implementation recycles the Printer abstraction used in uv, which is intended to replace all direct usage of std::io::stdout. See #19233 (comment)

I ended up futzing with the progress bar more than I probably should have to ensure it was also using the printer, but it doesn't seem like a big deal. See #19233 (comment)

Closes astral-sh/ty#772

crates/ty/src/lib.rs

zanieb · 2025-07-09T14:09:56Z

crates/ty/src/printer.rs

+use crate::logging::VerbosityLevel;
+
+#[derive(Debug, Clone, Copy, PartialEq, Eq, Default)]
+pub(crate) struct Printer {


The Printer abstraction is lifted from uv, but is a little different here because we reuse the existing VerbosityLevel enum, support locking the stream, and don't need stderr support yet.

zanieb · 2025-07-09T14:10:27Z

crates/ty/src/printer.rs

+    /// Return the [`Stdout`] stream for general messages.
+    pub(crate) fn stdout(self) -> Stdout {


Initially I wrote a stderr handler too, but we don't actually need it here yet.

zanieb · 2025-07-09T14:13:05Z

crates/ty/src/printer.rs

+        match self.status {
+            StreamStatus::Enabled => self.lock = Some(std::io::stdout().lock()),
+            StreamStatus::Disabled => self.lock = None,
+        }
+        self


We don't support locking in uv's printer, but in ty we lock stdout for the hot loop of printing diagnostics which seems reasonable and worth retaining.

zanieb · 2025-07-09T14:13:32Z

crates/ty_project/src/lib.rs


 /// A progress reporter.
-pub trait Reporter: Send + Sync {
+pub trait ProgressReporter: Send + Sync {


I just renamed this for clarity, as I initially thought this was used to report more than progress.

zanieb · 2025-07-09T15:27:55Z

crates/ty/src/printer.rs

+    /// Return the [`Stdout`] stream for an important message.
+    pub(crate) fn stdout_important(self) -> Stdout {


I don't love this name, but it's a simple way to handle things for now. I could see us doing things like stream_for_summary, stream_for_diagnostics, etc. in the future?

I'm inclined to go with stream_for_summary as those are the main messages that should still be written when -q is used

Works for me.

The reason I didn't do this initially is because stream_for_summary doesn't really fit the use case for something like the version command

(I'll come up with some names though)

crates/ty/src/lib.rs

zanieb · 2025-07-09T15:32:17Z

crates/ty/src/lib.rs

 /// A progress reporter for `ty check`.
-#[derive(Default)]
-struct IndicatifReporter(Option<indicatif::ProgressBar>);
+struct IndicatifReporter {


I got a bit in the weeds here handling the progress bar. We have handling for it in uv, and I thought it'd be reasonable to copy over. Arguably, we could just cut scope here and continue to use the DummyReporter, but I think it's nice to consolidate the logic for showing and hiding output in the main binary.

github-actions · 2025-07-09T15:32:20Z

`mypy_primer` results

No ecosystem changes detected ✅
No memory usage changes detected ✅

crates/ty/src/logging.rs

MichaReiser · 2025-07-09T20:16:17Z

I plan on reviewing this tomorrow morning

MichaReiser

Thanks. There are three changes we should make before landing but this otherwise looks good to me.

We should avoid rendering the diagnostics when running in --quiet because that's a lot of unnecessary work otherwise
Disallow -qq (and variance) until we actually do support other quiet modes
I think there's one summary message that now gets omitted that should be rendered as part of --quiet

crates/ty/src/lib.rs

MichaReiser · 2025-07-10T06:15:32Z

crates/ty/src/logging.rs

+        global = true,
+        overrides_with = "verbose",
+    )]
+    quiet: u8,


I'd prefer to keep this as a bool flag. It seems more confusing to users if they can use -qq but it doesn't change the behavior in anyway

I'd like to push back on that, I think we'll want a --qq silent mode to match uv and this is forwards compatible. You can also provide -vvvvvvv and it doesn't do anything more.

I think we'll want a --qq silent mode

What does this do? Does it silence everything?

I don't feel too strongly. I just think both approaches are forward compatible and I'm not entirely sure yet if we indeed want -qq in the future (because the use case isn't clear to me not because I'm oposed to it)

Yeah, "silent" mode.

It saves you from piping the command's output to /dev/null which is error prone in various shells (as we've discovered in our install scripts).

crates/ty/src/lib.rs

MichaReiser · 2025-07-10T06:19:03Z

crates/ty/src/printer.rs

+    /// Return the [`Stdout`] stream for an important message.
+    pub(crate) fn stdout_important(self) -> Stdout {


I'm inclined to go with stream_for_summary as those are the main messages that should still be written when -q is used

crates/ty/src/printer.rs

MichaReiser · 2025-07-10T14:15:56Z

crates/ty/src/lib.rs

-                                write!(stdout, "{}", diagnostic.display(db, &display_config))?;
+                                // Only render diagnostics if they're going to be displayed, since doing
+                                // so is expensive.
+                                if stdout.is_enabled() {


We should move this check outside the for loop as the result is always the same for all diagnostics.

Oh, never mind. It's in here because of the max_severity check below

Yep haha, that broke the tests and I was like.. oh mhm

MichaReiser

Thank you

…re_help * 'main' of https://github.com/astral-sh/ruff: Add simple integration tests for all output formats (astral-sh#19265) [`flake8-return`] Fix false-positive for variables used inside nested functions in `RET504` (astral-sh#18433) [ty] Add a `--quiet` mode (astral-sh#19233) Treat form feed as valid whitespace before a line continuation (astral-sh#19220) [ty] Make `check_file` a salsa query (astral-sh#19255) [ty] Consolidate submodule resolving code between `types.rs` and `ide_support.rs` (astral-sh#19256) [ty] Remove countme from salsa-structs (astral-sh#19257) [ty] Improve and document equivalence for module-literal types (astral-sh#19243) [ty] Optimize protocol subtyping by removing expensive and unnecessary equivalence check from the top of `Type::has_relation_to()` (astral-sh#19230) [ty] Ecosystem analyzer: parallelize, fix race condition (astral-sh#19252) [ty] Add completion kind to playground (astral-sh#19251) [ty] Deploy ecosystem diff to Cloudflare pages (astral-sh#19234) [ty] Add semantic token provider to playground (astral-sh#19232)

This matches uv's behavior. Briefly discussed at #19233 (comment) I think the most useful case is to avoid piping to `/dev/null` which hard to do properly in a cross-platform script.

zanieb added the ty Multi-file analysis & type inference label Jul 9, 2025

zanieb changed the title ~~Add a --quiet mode~~ [ty] Add a --quiet mode Jul 9, 2025