From 357004b20d64089a1c3d5f1c8b185719c823502d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?M=C3=A1rton=20Boros?= Date: Fri, 21 Mar 2025 15:19:28 +0200 Subject: [PATCH] fix(cli): use logging infra instead of print(ln), do not mess up progress bars MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fixes #84 Change-Id: I3ae21bb5353d5d9ad592831526a48ae391f9843d Reviewed-on: https://cl.snix.dev/c/snix/+/30234 Tested-by: besadii Autosubmit: Márton Boros Reviewed-by: Florian Klink --- snix/cli/src/lib.rs | 22 +++++++++--------- snix/cli/src/main.rs | 47 ++++++++++++++++++++++++++++----------- snix/cli/src/repl.rs | 27 ++++++++++++++++------ snix/cli/tests/repl.rs | 3 ++- snix/eval/src/errors.rs | 9 +++++++- snix/eval/src/warnings.rs | 11 +++++++++ 6 files changed, 87 insertions(+), 32 deletions(-) diff --git a/snix/cli/src/lib.rs b/snix/cli/src/lib.rs index 3ff333195..0347d4bb4 100644 --- a/snix/cli/src/lib.rs +++ b/snix/cli/src/lib.rs @@ -82,7 +82,8 @@ pub struct EvalResult { /// Interprets the given code snippet, printing out warnings and errors and returning the result #[allow(clippy::too_many_arguments)] -pub fn evaluate( +pub fn evaluate( + stderr: &mut E, snix_store_io: Rc, code: &str, path: Option, @@ -126,13 +127,12 @@ pub fn evaluate( let source_map = eval_builder.source_map().clone(); let (result, globals) = { - let mut compiler_observer = - DisassemblingObserver::new(source_map.clone(), std::io::stderr()); + let mut compiler_observer = DisassemblingObserver::new(source_map.clone(), stderr.clone()); if args.dump_bytecode { eval_builder.set_compiler_observer(Some(&mut compiler_observer)); } - let mut runtime_observer = TracingObserver::new(std::io::stderr()); + let mut runtime_observer = TracingObserver::new(stderr.clone()); if args.trace_runtime { if args.trace_runtime_timing { runtime_observer.enable_timing() @@ -162,17 +162,17 @@ pub fn evaluate( if args.display_ast { if let Some(ref expr) = result.expr { - eprintln!("AST: {}", snix_eval::pretty_print_expr(expr)); + writeln!(stderr, "AST: {}", snix_eval::pretty_print_expr(expr)).unwrap(); } } for error in &result.errors { - error.fancy_format_stderr(); + error.fancy_format_write(stderr); } if !args.no_warnings { for warning in &result.warnings { - warning.fancy_format_stderr(&source_map); + warning.fancy_format_write(stderr, &source_map); } } @@ -212,8 +212,8 @@ impl InterpretResult { } } - pub fn finalize(self) -> bool { - print!("{}", self.output); + pub fn finalize(self, stderr: &mut E) -> bool { + write!(stderr, "{}", self.output).unwrap(); self.success } @@ -231,7 +231,8 @@ impl InterpretResult { /// evaluation succeeded. #[instrument(skip_all, fields(indicatif.pb_show=tracing::field::Empty))] #[allow(clippy::too_many_arguments)] -pub fn interpret( +pub fn interpret( + stderr: &mut E, snix_store_io: Rc, code: &str, path: Option, @@ -244,6 +245,7 @@ pub fn interpret( ) -> Result { let mut output = String::new(); let result = evaluate( + stderr, snix_store_io, code, path, diff --git a/snix/cli/src/main.rs b/snix/cli/src/main.rs index e7eaf3abd..3265d2fcb 100644 --- a/snix/cli/src/main.rs +++ b/snix/cli/src/main.rs @@ -6,6 +6,7 @@ use snix_cli::{init_io_handle, interpret, AllowIncomplete}; use snix_eval::observer::DisassemblingObserver; use snix_eval::EvalMode; use snix_glue::snix_store_io::SnixStoreIO; +use std::io::Write; use std::rc::Rc; use std::{fs, path::PathBuf}; @@ -14,7 +15,12 @@ static GLOBAL: MiMalloc = MiMalloc; /// Interpret the given code snippet, but only run the Svix compiler /// on it and return errors and warnings. -fn lint(code: &str, path: Option, args: &Args) -> bool { +fn lint( + stderr: &mut E, + code: &str, + path: Option, + args: &Args, +) -> bool { let mut eval_builder = snix_eval::Evaluation::builder_impure(); if args.strict { @@ -23,14 +29,18 @@ fn lint(code: &str, path: Option, args: &Args) -> bool { let source_map = eval_builder.source_map().clone(); - let mut compiler_observer = DisassemblingObserver::new(source_map.clone(), std::io::stderr()); + let mut compiler_observer = DisassemblingObserver::new(source_map.clone(), stderr.clone()); if args.dump_bytecode { eval_builder.set_compiler_observer(Some(&mut compiler_observer)); } if args.trace_runtime { - eprintln!("warning: --trace-runtime has no effect with --compile-only!"); + writeln!( + stderr, + "warning: --trace-runtime has no effect with --compile-only" + ) + .unwrap(); } let eval = eval_builder.build(); @@ -38,16 +48,16 @@ fn lint(code: &str, path: Option, args: &Args) -> bool { if args.display_ast { if let Some(ref expr) = result.expr { - eprintln!("AST: {}", snix_eval::pretty_print_expr(expr)); + writeln!(stderr, "AST: {}", snix_eval::pretty_print_expr(expr)).unwrap(); } } for error in &result.errors { - error.fancy_format_stderr(); + error.fancy_format_write(stderr); } for warning in &result.warnings { - warning.fancy_format_stderr(&source_map); + warning.fancy_format_write(stderr, &source_map); } // inform the caller about any errors @@ -57,18 +67,22 @@ fn lint(code: &str, path: Option, args: &Args) -> bool { fn main() { let args = Args::parse(); - snix_tracing::TracingBuilder::default() + let tracing_handle = snix_tracing::TracingBuilder::default() .enable_progressbar() .build() .expect("unable to set up tracing subscriber"); + let mut stdout = tracing_handle.get_stdout_writer(); + let mut stderr = tracing_handle.get_stderr_writer(); + let tokio_runtime = tokio::runtime::Runtime::new().expect("failed to setup tokio runtime"); let io_handle = init_io_handle(&tokio_runtime, &args); if let Some(file) = &args.script { - run_file(io_handle, file.clone(), &args) + run_file(&mut stdout, &mut stderr, io_handle, file.clone(), &args) } else if let Some(expr) = &args.expr { if !interpret( + &mut stderr, io_handle, expr, None, @@ -80,26 +94,33 @@ fn main() { None, ) .unwrap() - .finalize() + .finalize(&mut stdout) { std::process::exit(1); } } else { let mut repl = Repl::new(io_handle, &args); - repl.run() + repl.run(&mut stdout, &mut stderr) } } -fn run_file(io_handle: Rc, mut path: PathBuf, args: &Args) { +fn run_file( + stdout: &mut O, + stderr: &mut E, + io_handle: Rc, + mut path: PathBuf, + args: &Args, +) { if path.is_dir() { path.push("default.nix"); } let contents = fs::read_to_string(&path).expect("failed to read the input file"); let success = if args.compile_only { - lint(&contents, Some(path), args) + lint(stderr, &contents, Some(path), args) } else { interpret( + stderr, io_handle, &contents, Some(path), @@ -111,7 +132,7 @@ fn run_file(io_handle: Rc, mut path: PathBuf, args: &Args) { None, ) .unwrap() - .finalize() + .finalize(stdout) }; if !success { diff --git a/snix/cli/src/repl.rs b/snix/cli/src/repl.rs index 3fbdf826a..6db578a3d 100644 --- a/snix/cli/src/repl.rs +++ b/snix/cli/src/repl.rs @@ -1,3 +1,4 @@ +use std::io::Write; use std::path::PathBuf; use std::rc::Rc; @@ -74,8 +75,8 @@ pub struct CommandResult { } impl CommandResult { - pub fn finalize(self) -> bool { - print!("{}", self.output); + pub fn finalize(self, stdout: &mut E) -> bool { + write!(stdout, "{}", self.output).unwrap(); self.continue_ } @@ -111,9 +112,17 @@ impl<'a> Repl<'a> { } } - pub fn run(&mut self) { + pub fn run( + &mut self, + stdout: &mut O, + stderr: &mut E, + ) { if self.args.compile_only { - eprintln!("warning: `--compile-only` has no effect on REPL usage!"); + writeln!( + stderr, + "warning: `--compile-only` has no effect on REPL usage!" + ) + .unwrap(); } let history_path = match state_dir() { @@ -139,14 +148,14 @@ impl<'a> Repl<'a> { let readline = self.rl.readline(prompt); match readline { Ok(line) => { - if !self.send(line).finalize() { + if !self.send(stderr, line).finalize(stdout) { break; } } Err(ReadlineError::Interrupted) | Err(ReadlineError::Eof) => break, Err(err) => { - eprintln!("error: {}", err); + writeln!(stderr, "error: {}", err).unwrap(); break; } } @@ -159,7 +168,7 @@ impl<'a> Repl<'a> { /// Send a line of user input to the REPL. Returns a result indicating the output to show to the /// user, and whether or not to continue - pub fn send(&mut self, line: String) -> CommandResult { + pub fn send(&mut self, stderr: &mut E, line: String) -> CommandResult { if line.is_empty() { return CommandResult { output: String::new(), @@ -187,6 +196,7 @@ impl<'a> Repl<'a> { Ok(InterpretResult::empty_success(None)) } ReplCommand::Expr(input) => interpret( + stderr, Rc::clone(&self.io_handle), input, None, @@ -199,6 +209,7 @@ impl<'a> Repl<'a> { ), ReplCommand::Assign(Assignment { ident, value }) => { match evaluate( + stderr, Rc::clone(&self.io_handle), &value.to_string(), /* FIXME: don't re-parse */ None, @@ -218,6 +229,7 @@ impl<'a> Repl<'a> { } } ReplCommand::Explain(input) => interpret( + stderr, Rc::clone(&self.io_handle), input, None, @@ -229,6 +241,7 @@ impl<'a> Repl<'a> { Some(self.source_map.clone()), ), ReplCommand::Print(input) => interpret( + stderr, Rc::clone(&self.io_handle), input, None, diff --git a/snix/cli/tests/repl.rs b/snix/cli/tests/repl.rs index 28b4b86a8..ce89f4205 100644 --- a/snix/cli/tests/repl.rs +++ b/snix/cli/tests/repl.rs @@ -15,8 +15,9 @@ macro_rules! test_repl { OsString::from("nixpkgs=/tmp"), ]); let mut repl = snix_cli::Repl::new(init_io_handle(&tokio_runtime, &args), &args); + let mut buffer = std::io::Cursor::new(Vec::new()); $({ - let result = repl.send($send.into()); + let result = repl.send(&mut buffer, $send.into()); $expect.assert_eq(result.output()) ; })* diff --git a/snix/eval/src/errors.rs b/snix/eval/src/errors.rs index b152b3d95..5cc9c127e 100644 --- a/snix/eval/src/errors.rs +++ b/snix/eval/src/errors.rs @@ -1,5 +1,6 @@ use std::error; use std::io; +use std::io::Write; use std::path::PathBuf; use std::rc::Rc; use std::str::Utf8Error; @@ -150,7 +151,7 @@ to a missing value in the attribute set(s) included via `with`."# BytecodeError(Box), /// Given type can't be coerced to a string in the respective context - #[error("cannot ({}) coerce {from} to a string{}", + #[error("cannot ({}) coerce {from} to a string{}", (if .kind.strong { "strongly" } else { "weakly" }), (if *.from == "set" { ", missing a `__toString` or `outPath` attribute" @@ -634,6 +635,12 @@ impl Error { Emitter::stderr(ColorConfig::Auto, Some(&*self.source.codemap())).emit(&self.diagnostics()); } + /// Render a fancy, human-readable output of this error and print + /// it to a std::io::Write. + pub fn fancy_format_write(&self, stderr: &mut E) { + Emitter::new(Box::new(stderr), Some(&*self.source.codemap())).emit(&self.diagnostics()); + } + /// Create the optional span label displayed as an annotation on /// the underlined span of the error. fn span_label(&self) -> Option { diff --git a/snix/eval/src/warnings.rs b/snix/eval/src/warnings.rs index 33f1daecc..6bea1e15a 100644 --- a/snix/eval/src/warnings.rs +++ b/snix/eval/src/warnings.rs @@ -49,6 +49,17 @@ impl EvalWarning { .emit(&[self.diagnostic(source)]); } + /// Render a fancy, human-readable output of this warning and + /// print it to a std::io::Write. If rendered in a terminal that supports + /// colours and font styles, the output will include those. + pub fn fancy_format_write( + &self, + stderr: &mut E, + source: &SourceCode, + ) { + Emitter::new(Box::new(stderr), Some(&*source.codemap())).emit(&[self.diagnostic(source)]); + } + /// Create the optional span label displayed as an annotation on /// the underlined span of the warning. fn span_label(&self) -> Option {