-
Notifications
You must be signed in to change notification settings - Fork 1.6k
[ty] Add a --quiet mode
#19233
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
[ty] Add a --quiet mode
#19233
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,19 +1,21 @@ | ||
| mod args; | ||
| mod logging; | ||
| mod printer; | ||
| mod python_version; | ||
| mod version; | ||
|
|
||
| pub use args::Cli; | ||
| use ty_static::EnvVars; | ||
|
|
||
| use std::io::{self, BufWriter, Write, stdout}; | ||
| use std::fmt::Write; | ||
| use std::process::{ExitCode, Termination}; | ||
|
|
||
| use anyhow::Result; | ||
| use std::sync::Mutex; | ||
|
|
||
| use crate::args::{CheckCommand, Command, TerminalColor}; | ||
| use crate::logging::setup_tracing; | ||
| use crate::printer::Printer; | ||
| use anyhow::{Context, anyhow}; | ||
| use clap::{CommandFactory, Parser}; | ||
| use colored::Colorize; | ||
|
|
@@ -25,7 +27,7 @@ use ruff_db::system::{OsSystem, SystemPath, SystemPathBuf}; | |
| use salsa::plumbing::ZalsaDatabase; | ||
| use ty_project::metadata::options::ProjectOptionsOverrides; | ||
| use ty_project::watch::ProjectWatcher; | ||
| use ty_project::{Db, DummyReporter, Reporter, watch}; | ||
| use ty_project::{Db, watch}; | ||
| use ty_project::{ProjectDatabase, ProjectMetadata}; | ||
| use ty_server::run_server; | ||
|
|
||
|
|
@@ -42,14 +44,16 @@ pub fn run() -> anyhow::Result<ExitStatus> { | |
| Command::Check(check_args) => run_check(check_args), | ||
| Command::Version => version().map(|()| ExitStatus::Success), | ||
| Command::GenerateShellCompletion { shell } => { | ||
| use std::io::stdout; | ||
|
|
||
| shell.generate(&mut Cli::command(), &mut stdout()); | ||
| Ok(ExitStatus::Success) | ||
| } | ||
| } | ||
| } | ||
|
|
||
| pub(crate) fn version() -> Result<()> { | ||
| let mut stdout = BufWriter::new(io::stdout().lock()); | ||
| let mut stdout = Printer::default().stream_for_requested_summary().lock(); | ||
| let version_info = crate::version::version(); | ||
| writeln!(stdout, "ty {}", &version_info)?; | ||
| Ok(()) | ||
|
|
@@ -62,6 +66,8 @@ fn run_check(args: CheckCommand) -> anyhow::Result<ExitStatus> { | |
| countme::enable(verbosity.is_trace()); | ||
| let _guard = setup_tracing(verbosity, args.color.unwrap_or_default())?; | ||
|
|
||
| let printer = Printer::default().with_verbosity(verbosity); | ||
|
|
||
| tracing::warn!( | ||
| "ty is pre-release software and not ready for production use. \ | ||
| Expect to encounter bugs, missing features, and fatal errors.", | ||
|
|
@@ -126,7 +132,8 @@ fn run_check(args: CheckCommand) -> anyhow::Result<ExitStatus> { | |
| } | ||
|
|
||
| let project_options_overrides = ProjectOptionsOverrides::new(config_file, options); | ||
| let (main_loop, main_loop_cancellation_token) = MainLoop::new(project_options_overrides); | ||
| let (main_loop, main_loop_cancellation_token) = | ||
| MainLoop::new(project_options_overrides, printer); | ||
|
|
||
| // Listen to Ctrl+C and abort the watch mode. | ||
| let main_loop_cancellation_token = Mutex::new(Some(main_loop_cancellation_token)); | ||
|
|
@@ -144,7 +151,7 @@ fn run_check(args: CheckCommand) -> anyhow::Result<ExitStatus> { | |
| main_loop.run(&mut db)? | ||
| }; | ||
|
|
||
| let mut stdout = stdout().lock(); | ||
| let mut stdout = printer.stream_for_requested_summary().lock(); | ||
| match std::env::var(EnvVars::TY_MEMORY_REPORT).as_deref() { | ||
| Ok("short") => write!(stdout, "{}", db.salsa_memory_dump().display_short())?, | ||
| Ok("mypy_primer") => write!(stdout, "{}", db.salsa_memory_dump().display_mypy_primer())?, | ||
|
|
@@ -195,12 +202,16 @@ struct MainLoop { | |
| /// The file system watcher, if running in watch mode. | ||
| watcher: Option<ProjectWatcher>, | ||
|
|
||
| /// Interface for displaying information to the user. | ||
| printer: Printer, | ||
|
|
||
| project_options_overrides: ProjectOptionsOverrides, | ||
| } | ||
|
|
||
| impl MainLoop { | ||
| fn new( | ||
| project_options_overrides: ProjectOptionsOverrides, | ||
| printer: Printer, | ||
| ) -> (Self, MainLoopCancellationToken) { | ||
| let (sender, receiver) = crossbeam_channel::bounded(10); | ||
|
|
||
|
|
@@ -210,6 +221,7 @@ impl MainLoop { | |
| receiver, | ||
| watcher: None, | ||
| project_options_overrides, | ||
| printer, | ||
| }, | ||
| MainLoopCancellationToken { sender }, | ||
| ) | ||
|
|
@@ -226,32 +238,24 @@ impl MainLoop { | |
|
|
||
| // Do not show progress bars with `--watch`, indicatif does not seem to | ||
| // handle cancelling independent progress bars very well. | ||
| self.run_with_progress::<DummyReporter>(db)?; | ||
| // TODO(zanieb): We can probably use `MultiProgress` to handle this case in the future. | ||
| self.printer = self.printer.with_no_progress(); | ||
| self.run(db)?; | ||
|
|
||
| Ok(ExitStatus::Success) | ||
| } | ||
|
|
||
| fn run(self, db: &mut ProjectDatabase) -> Result<ExitStatus> { | ||
| self.run_with_progress::<IndicatifReporter>(db) | ||
| } | ||
|
|
||
| fn run_with_progress<R>(mut self, db: &mut ProjectDatabase) -> Result<ExitStatus> | ||
| where | ||
| R: Reporter + Default + 'static, | ||
| { | ||
| self.sender.send(MainLoopMessage::CheckWorkspace).unwrap(); | ||
|
|
||
| let result = self.main_loop::<R>(db); | ||
| let result = self.main_loop(db); | ||
|
|
||
| tracing::debug!("Exiting main loop"); | ||
|
|
||
| result | ||
| } | ||
|
|
||
| fn main_loop<R>(&mut self, db: &mut ProjectDatabase) -> Result<ExitStatus> | ||
| where | ||
| R: Reporter + Default + 'static, | ||
| { | ||
| fn main_loop(mut self, db: &mut ProjectDatabase) -> Result<ExitStatus> { | ||
| // Schedule the first check. | ||
| tracing::debug!("Starting main loop"); | ||
|
|
||
|
|
@@ -267,7 +271,7 @@ impl MainLoop { | |
| // to prevent blocking the main loop here. | ||
| rayon::spawn(move || { | ||
| match salsa::Cancelled::catch(|| { | ||
| let mut reporter = R::default(); | ||
| let mut reporter = IndicatifReporter::from(self.printer); | ||
| db.check_with_reporter(&mut reporter) | ||
| }) { | ||
| Ok(result) => { | ||
|
|
@@ -302,10 +306,12 @@ impl MainLoop { | |
| return Ok(ExitStatus::Success); | ||
| } | ||
|
|
||
| let mut stdout = stdout().lock(); | ||
|
|
||
| if result.is_empty() { | ||
| writeln!(stdout, "{}", "All checks passed!".green().bold())?; | ||
| writeln!( | ||
| self.printer.stream_for_success_summary(), | ||
| "{}", | ||
| "All checks passed!".green().bold() | ||
| )?; | ||
|
|
||
| if self.watcher.is_none() { | ||
| return Ok(ExitStatus::Success); | ||
|
|
@@ -314,14 +320,19 @@ impl MainLoop { | |
| let mut max_severity = Severity::Info; | ||
| let diagnostics_count = result.len(); | ||
|
|
||
| let mut stdout = self.printer.stream_for_details().lock(); | ||
| for diagnostic in result { | ||
| 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() { | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We should move this check outside the
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Oh, never mind. It's in here because of the
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yep haha, that broke the tests and I was like.. oh mhm |
||
| write!(stdout, "{}", diagnostic.display(db, &display_config))?; | ||
| } | ||
|
|
||
| max_severity = max_severity.max(diagnostic.severity()); | ||
| } | ||
|
|
||
| writeln!( | ||
| stdout, | ||
| self.printer.stream_for_failure_summary(), | ||
| "Found {} diagnostic{}", | ||
| diagnostics_count, | ||
| if diagnostics_count > 1 { "s" } else { "" } | ||
|
|
@@ -383,27 +394,53 @@ impl MainLoop { | |
| } | ||
|
|
||
| /// A progress reporter for `ty check`. | ||
| #[derive(Default)] | ||
| struct IndicatifReporter(Option<indicatif::ProgressBar>); | ||
| enum IndicatifReporter { | ||
| /// A constructed reporter that is not yet ready, contains the target for the progress bar. | ||
| Pending(indicatif::ProgressDrawTarget), | ||
| /// A reporter that is ready, containing a progress bar to report to. | ||
| /// | ||
| /// Initialization of the bar is deferred to [`ty_project::ProgressReporter::set_files`] so we | ||
| /// do not initialize the bar too early as it may take a while to collect the number of files to | ||
| /// process and we don't want to display an empty "0/0" bar. | ||
| Initialized(indicatif::ProgressBar), | ||
| } | ||
|
|
||
| impl From<Printer> for IndicatifReporter { | ||
| fn from(printer: Printer) -> Self { | ||
| Self::Pending(printer.progress_target()) | ||
| } | ||
| } | ||
|
|
||
| impl ty_project::Reporter for IndicatifReporter { | ||
| impl ty_project::ProgressReporter for IndicatifReporter { | ||
| fn set_files(&mut self, files: usize) { | ||
| let progress = indicatif::ProgressBar::new(files as u64); | ||
| progress.set_style( | ||
| let target = match std::mem::replace( | ||
| self, | ||
| IndicatifReporter::Pending(indicatif::ProgressDrawTarget::hidden()), | ||
| ) { | ||
| Self::Pending(target) => target, | ||
| Self::Initialized(_) => panic!("The progress reporter should only be initialized once"), | ||
| }; | ||
|
|
||
| let bar = indicatif::ProgressBar::with_draw_target(Some(files as u64), target); | ||
| bar.set_style( | ||
| indicatif::ProgressStyle::with_template( | ||
| "{msg:8.dim} {bar:60.green/dim} {pos}/{len} files", | ||
| ) | ||
| .unwrap() | ||
| .progress_chars("--"), | ||
| ); | ||
| progress.set_message("Checking"); | ||
|
|
||
| self.0 = Some(progress); | ||
| bar.set_message("Checking"); | ||
| *self = Self::Initialized(bar); | ||
| } | ||
|
|
||
| fn report_file(&self, _file: &ruff_db::files::File) { | ||
| if let Some(ref progress_bar) = self.0 { | ||
| progress_bar.inc(1); | ||
| match self { | ||
| IndicatifReporter::Initialized(progress_bar) => { | ||
| progress_bar.inc(1); | ||
| } | ||
| IndicatifReporter::Pending(_) => { | ||
| panic!("`report_file` called before `set_files`") | ||
| } | ||
| } | ||
| } | ||
| } | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -24,15 +24,32 @@ pub(crate) struct Verbosity { | |
| help = "Use verbose output (or `-vv` and `-vvv` for more verbose output)", | ||
| action = clap::ArgAction::Count, | ||
| global = true, | ||
| overrides_with = "quiet", | ||
| )] | ||
| verbose: u8, | ||
|
|
||
| #[arg( | ||
| long, | ||
| help = "Use quiet output", | ||
| action = clap::ArgAction::Count, | ||
| global = true, | ||
| overrides_with = "verbose", | ||
| )] | ||
| quiet: u8, | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'd prefer to keep this as a
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'd like to push back on that, I think we'll want a
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
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
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah, "silent" mode. It saves you from piping the command's output to |
||
| } | ||
|
|
||
| impl Verbosity { | ||
| /// Returns the verbosity level based on the number of `-v` flags. | ||
| /// Returns the verbosity level based on the number of `-v` and `-q` flags. | ||
| /// | ||
| /// Returns `None` if the user did not specify any verbosity flags. | ||
| pub(crate) fn level(&self) -> VerbosityLevel { | ||
| // `--quiet` and `--verbose` are mutually exclusive in Clap, so we can just check one first. | ||
| match self.quiet { | ||
| 0 => {} | ||
| _ => return VerbosityLevel::Quiet, | ||
| // TODO(zanieb): Add support for `-qq` with a "silent" mode | ||
| } | ||
|
|
||
| match self.verbose { | ||
| 0 => VerbosityLevel::Default, | ||
| 1 => VerbosityLevel::Verbose, | ||
|
|
@@ -42,9 +59,14 @@ impl Verbosity { | |
| } | ||
| } | ||
|
|
||
| #[derive(Debug, Copy, Clone, Eq, PartialEq, Ord, PartialOrd)] | ||
| #[derive(Debug, Copy, Clone, Eq, PartialEq, Ord, PartialOrd, Default)] | ||
| pub(crate) enum VerbosityLevel { | ||
| /// Quiet output. Only shows Ruff and ty events up to the [`ERROR`](tracing::Level::ERROR). | ||
| /// Silences output except for summary information. | ||
| Quiet, | ||
|
|
||
| /// Default output level. Only shows Ruff and ty events up to the [`WARN`](tracing::Level::WARN). | ||
| #[default] | ||
| Default, | ||
|
|
||
| /// Enables verbose output. Emits Ruff and ty events up to the [`INFO`](tracing::Level::INFO). | ||
|
|
@@ -62,6 +84,7 @@ pub(crate) enum VerbosityLevel { | |
| impl VerbosityLevel { | ||
| const fn level_filter(self) -> LevelFilter { | ||
| match self { | ||
| VerbosityLevel::Quiet => LevelFilter::ERROR, | ||
zanieb marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| VerbosityLevel::Default => LevelFilter::WARN, | ||
| VerbosityLevel::Verbose => LevelFilter::INFO, | ||
| VerbosityLevel::ExtraVerbose => LevelFilter::DEBUG, | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.