fix(cli): use logging infra instead of print(ln), do not mess up progress bars

Fixes #84

Change-Id: I3ae21bb5353d5d9ad592831526a48ae391f9843d
Reviewed-on: https://cl.snix.dev/c/snix/+/30234
Tested-by: besadii
Autosubmit: Márton Boros <martonboros@gmail.com>
Reviewed-by: Florian Klink <flokli@flokli.de>

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<E: std::io::Write + Clone + Send>(
+    stderr: &mut E,
     snix_store_io: Rc<SnixStoreIO>,
     code: &str,
     path: Option<PathBuf>,
@@ -126,13 +127,12 @@ pub fn evaluate(
 
     let source_map = eval_builder.source_map().clone();
     let (result, globals) = {
-        let mut compiler_observer =
+        let mut compiler_observer = DisassemblingObserver::new(source_map.clone(), stderr.clone());
-            DisassemblingObserver::new(source_map.clone(), std::io::stderr());
         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 {
+    pub fn finalize<E: std::io::Write>(self, stderr: &mut E) -> bool {
-        print!("{}", self.output);
+        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<E: std::io::Write + Clone + Send>(
+    stderr: &mut E,
     snix_store_io: Rc<SnixStoreIO>,
     code: &str,
     path: Option<PathBuf>,
@@ -244,6 +245,7 @@ pub fn interpret(
 ) -> Result<InterpretResult, IncompleteInput> {
     let mut output = String::new();
     let result = evaluate(
+        stderr,
         snix_store_io,
         code,
         path,

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<PathBuf>, args: &Args) -> bool {
+fn lint<E: Write + Clone + Send>(
+    stderr: &mut E,
+    code: &str,
+    path: Option<PathBuf>,
+    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<PathBuf>, 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<PathBuf>, 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<PathBuf>, 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<SnixStoreIO>, mut path: PathBuf, args: &Args) {
+fn run_file<O: Write, E: Write + Clone + Send>(
+    stdout: &mut O,
+    stderr: &mut E,
+    io_handle: Rc<SnixStoreIO>,
+    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<SnixStoreIO>, mut path: PathBuf, args: &Args) {
             None,
         )
         .unwrap()
-        .finalize()
+        .finalize(stdout)
     };
 
     if !success {

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 {
+    pub fn finalize<E: Write>(self, stdout: &mut E) -> bool {
-        print!("{}", self.output);
+        write!(stdout, "{}", self.output).unwrap();
         self.continue_
     }
 
@@ -111,9 +112,17 @@ impl<'a> Repl<'a> {
         }
     }
 
-    pub fn run(&mut self) {
+    pub fn run<O: Write + Clone + Send, E: Write + Clone + Send>(
+        &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<E: Write + Clone + 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,

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())
                 ;
             })*

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<Error>),
 
     /// 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<E: Write + Send>(&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<String> {

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<E: std::io::Write + std::marker::Send>(
+        &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<String> {