Clean up error display (#1699)

src/error.rs

@@ -219,219 +219,88 @@ impl<'src> ColorDisplay for Error<'src> {
   fn fmt(&self, f: &mut Formatter, color: Color) -> fmt::Result {
     use Error::*;
 
-    write!(
+    let error = color.error().paint("error");
-      f,
+    let message = color.message().prefix();
-      "{}: {}",
+    write!(f, "{error}: {message}")?;
-      color.error().paint("error"),
-      color.message().prefix()
-    )?;
 
     match self {
-      ArgumentCountMismatch {
+      ArgumentCountMismatch { recipe, found, min, max, .. } => {
-        recipe,
+        let count = Count("argument", *found);
-        found,
-        min,
-        max,
-        ..
-      } => {
         if min == max {
           let expected = min;
-          write!(
+          let only = if expected < found { "only " } else { "" };
-            f,
+          write!(f, "Recipe `{recipe}` got {found} {count} but {only}takes {expected}")?;
-            "Recipe `{recipe}` got {found} {} but {}takes {expected}",
-            Count("argument", *found),
-            if expected < found { "only " } else { "" }
-          )?;
         } else if found < min {
-          write!(
+          write!(f, "Recipe `{recipe}` got {found} {count} but takes at least {min}")?;
-            f,
-            "Recipe `{recipe}` got {found} {} but takes at least {min}",
-            Count("argument", *found)
-          )?;
         } else if found > max {
-          write!(
+          write!(f, "Recipe `{recipe}` got {found} {count} but takes at most {max}")?;
-            f,
-            "Recipe `{recipe}` got {found} {} but takes at most {max}",
-            Count("argument", *found)
-          )?;
         }
       }
       Backtick { output_error, .. } => match output_error {
-        OutputError::Code(code) => {
+        OutputError::Code(code) => write!(f, "Backtick failed with exit code {code}")?,
-          write!(f, "Backtick failed with exit code {code}")?;
+        OutputError::Signal(signal) => write!(f, "Backtick was terminated by signal {signal}")?,
+        OutputError::Unknown => write!(f, "Backtick failed for an unknown reason")?,
+        OutputError::Io(io_error) => match io_error.kind() {
+            io::ErrorKind::NotFound => write!(f, "Backtick could not be run because just could not find the shell:\n{io_error}"),
+            io::ErrorKind::PermissionDenied => write!(f, "Backtick could not be run because just could not run the shell:\n{io_error}"),
+            _ => write!(f, "Backtick could not be run because of an IO error while launching the shell:\n{io_error}"),
+          }?,
+        OutputError::Utf8(utf8_error) => write!(f, "Backtick succeeded but stdout was not utf8: {utf8_error}")?,
       }
-        OutputError::Signal(signal) => {
+      ChooserInvoke { shell_binary, shell_arguments, chooser, io_error} => {
-          write!(f, "Backtick was terminated by signal {signal}")?;
+        let chooser = chooser.to_string_lossy();
-        }
+        write!(f, "Chooser `{shell_binary} {shell_arguments} {chooser}` invocation failed: {io_error}")?;
-        OutputError::Unknown => {
-          write!(f, "Backtick failed for an unknown reason")?;
-        }
-        OutputError::Io(io_error) => {
-          match io_error.kind() {
-            io::ErrorKind::NotFound => write!(
-              f,
-              "Backtick could not be run because just could not find the shell:\n{io_error}"
-            ),
-            io::ErrorKind::PermissionDenied => write!(
-              f,
-              "Backtick could not be run because just could not run the shell:\n{io_error}"
-            ),
-            _ => write!(
-              f,
-              "Backtick could not be run because of an IO error while launching the shell:\n{io_error}"
-            ),
-          }?;
-        }
-        OutputError::Utf8(utf8_error) => {
-          write!(
-            f,
-            "Backtick succeeded but stdout was not utf8: {utf8_error}"
-          )?;
-        }
-      },
-      ChooserInvoke {
-        shell_binary,
-        shell_arguments,
-        chooser,
-        io_error,
-      } => {
-        write!(
-          f,
-          "Chooser `{shell_binary} {shell_arguments} {}` invocation failed: {io_error}",
-          chooser.to_string_lossy(),
-        )?;
       }
       ChooserRead { chooser, io_error } => {
-        write!(
+        let chooser = chooser.to_string_lossy();
-          f,
+        write!(f, "Failed to read output from chooser `{chooser}`: {io_error}")?;
-          "Failed to read output from chooser `{}`: {io_error}",
-          chooser.to_string_lossy()
-        )?;
       }
       ChooserStatus { chooser, status } => {
-        write!(
+        let chooser = chooser.to_string_lossy();
-          f,
+        write!(f, "Chooser `{chooser}` failed: {status}")?;
-          "Chooser `{}` failed: {status}",
-          chooser.to_string_lossy()
-        )?;
       }
       ChooserWrite { chooser, io_error } => {
-        write!(
+        let chooser = chooser.to_string_lossy();
-          f,
+        write!(f, "Failed to write to chooser `{chooser}`: {io_error}")?;
-          "Failed to write to chooser `{}`: {io_error}",
-          chooser.to_string_lossy()
-        )?;
       }
       CircularInclude { current, include } => {
-        write!(
+        let include = include.display();
-          f,
+        let current = current.display();
-          "Include `{}` in `{}` is a circular include", include.display(), current.display()
+        write!(f, "Include `{include}` in `{current}` is a circular include")?;
-        )?;
+      }
-      },
+      Code { recipe, line_number, code, .. } => {
-      Code {
-        recipe,
-        line_number,
-        code,
-        ..
-      } => {
         if let Some(n) = line_number {
-          write!(
+          write!(f, "Recipe `{recipe}` failed on line {n} with exit code {code}")?;
-            f,
-            "Recipe `{recipe}` failed on line {n} with exit code {code}"
-          )?;
         } else {
           write!(f, "Recipe `{recipe}` failed with exit code {code}")?;
         }
       }
-      CommandInvoke {
+      CommandInvoke { binary, arguments, io_error } => {
-        binary,
+        let cmd = format_cmd(binary, arguments);
-        arguments,
+        write!(f, "Failed to invoke {cmd}: {io_error}")?;
-        io_error,
-      } => {
-        write!(
-          f,
-          "Failed to invoke {}: {io_error}",
-          iter::once(binary)
-            .chain(arguments)
-            .map(|value| Enclosure::tick(value.to_string_lossy()).to_string())
-            .collect::<Vec<String>>()
-            .join(" "),
-        )?;
       }
-      CommandStatus {
+      CommandStatus { binary, arguments, status} => {
-        binary,
+        let cmd = format_cmd(binary, arguments);
-        arguments,
+        write!(f, "Command {cmd} failed: {status}")?;
-        status,
-      } => {
-        write!(
-          f,
-          "Command {} failed: {status}",
-          iter::once(binary)
-            .chain(arguments)
-            .map(|value| Enclosure::tick(value.to_string_lossy()).to_string())
-            .collect::<Vec<String>>()
-            .join(" "),
-        )?;
       }
       Compile { compile_error } => Display::fmt(compile_error, f)?,
       Config { config_error } => Display::fmt(config_error, f)?,
-      Cygpath {
+      Cygpath { recipe, output_error} => match output_error {
-        recipe,
+        OutputError::Code(code) => write!(f, "Cygpath failed with exit code {code} while translating recipe `{recipe}` shebang interpreter path")?,
-        output_error,
+        OutputError::Signal(signal) => write!(f, "Cygpath terminated by signal {signal} while translating recipe `{recipe}` shebang interpreter path")?,
-      } => match output_error {
+        OutputError::Unknown => write!(f, "Cygpath experienced an unknown failure while translating recipe `{recipe}` shebang interpreter path")?,
-        OutputError::Code(code) => {
-          write!(
-            f,
-            "Cygpath failed with exit code {code} while translating recipe `{recipe}` shebang interpreter \
-             path"
-          )?;
-        }
-        OutputError::Signal(signal) => {
-          write!(
-            f,
-            "Cygpath terminated by signal {signal} while translating recipe `{recipe}` shebang interpreter \
-             path"
-          )?;
-        }
-        OutputError::Unknown => {
-          write!(
-            f,
-            "Cygpath experienced an unknown failure while translating recipe `{recipe}` shebang \
-             interpreter path"
-          )?;
-        }
         OutputError::Io(io_error) => {
           match io_error.kind() {
-            io::ErrorKind::NotFound => write!(
+            io::ErrorKind::NotFound => write!(f, "Could not find `cygpath` executable to translate recipe `{recipe}` shebang interpreter path:\n{io_error}"),
-              f,
+            io::ErrorKind::PermissionDenied => write!(f, "Could not run `cygpath` executable to translate recipe `{recipe}` shebang interpreter path:\n{io_error}"),
-              "Could not find `cygpath` executable to translate recipe `{recipe}` shebang interpreter \
-               path:\n{io_error}"
-            ),
-            io::ErrorKind::PermissionDenied => write!(
-              f,
-              "Could not run `cygpath` executable to translate recipe `{recipe}` shebang interpreter \
-               path:\n{io_error}"
-            ),
             _ => write!(f, "Could not run `cygpath` executable:\n{io_error}"),
           }?;
         }
-        OutputError::Utf8(utf8_error) => {
+        OutputError::Utf8(utf8_error) => write!(f, "Cygpath successfully translated recipe `{recipe}` shebang interpreter path, but output was not utf8: {utf8_error}")?,
-          write!(
-            f,
-            "Cygpath successfully translated recipe `{recipe}` shebang interpreter path, but output was \
-             not utf8: {utf8_error}"
-          )?;
       }
-      },
+      DefaultRecipeRequiresArguments { recipe, min_arguments} => {
-      DefaultRecipeRequiresArguments {
+        let count = Count("argument", *min_arguments);
-        recipe,
+        write!(f, "Recipe `{recipe}` cannot be used as default recipe since it requires at least {min_arguments} {count}.")?;
-        min_arguments,
-      } => {
-        write!(
-          f,
-          "Recipe `{recipe}` cannot be used as default recipe since it requires at least {min_arguments} {}.",
-          Count("argument", *min_arguments),
-        )?;
       }
       Dotenv { dotenv_error } => {
         write!(f, "Failed to load environment file: {dotenv_error}")?;
@@ -440,21 +309,16 @@ impl<'src> ColorDisplay for Error<'src> {
         write!(f, "Failed to dump JSON to stdout: {serde_json_error}")?;
       }
       EditorInvoke { editor, io_error } => {
-        write!(
+        let editor = editor.to_string_lossy();
-          f,
+        write!(f, "Editor `{editor}` invocation failed: {io_error}")?;
-          "Editor `{}` invocation failed: {io_error}",
-          editor.to_string_lossy(),
-        )?;
       }
       EditorStatus { editor, status } => {
-        write!(f, "Editor `{}` failed: {status}", editor.to_string_lossy(),)?;
+        let editor = editor.to_string_lossy();
+        write!(f, "Editor `{editor}` failed: {status}")?;
       }
-      EvalUnknownVariable {
+      EvalUnknownVariable { variable, suggestion} => {
-        variable,
-        suggestion,
-      } => {
         write!(f, "Justfile does not contain variable `{variable}`.")?;
-        if let Some(suggestion) = *suggestion {
+        if let Some(suggestion) = suggestion {
           write!(f, "\n{suggestion}")?;
         }
       }
@@ -462,154 +326,83 @@ impl<'src> ColorDisplay for Error<'src> {
         write!(f, "Formatted justfile differs from original.")?;
       }
       FunctionCall { function, message } => {
-        write!(
+        let function = function.lexeme();
-          f,
+        write!(f, "Call to function `{function}` failed: {message}")?;
-          "Call to function `{}` failed: {message}",
+      }
-          function.lexeme()
+      IncludeMissingPath { file: justfile, line } => {
-        )?;
+        let line = line.ordinal();
+        let justfile = justfile.display();
+        write!(f, "!include directive on line {line} of `{justfile}` has no argument")?;
       }
-      IncludeMissingPath {
-        file: justfile, line
-      } => {
-
-        write!(
-          f,
-          "!include directive on line {} of `{}` has no argument",
-          line.ordinal(),
-          justfile.display(),
-        )?;
-
-      },
       InitExists { justfile } => {
         write!(f, "Justfile `{}` already exists", justfile.display())?;
       }
       Internal { message } => {
-        write!(
+        write!(f, "Internal runtime error, this may indicate a bug in just: {message} \
-          f,
+                   consider filing an issue: https://github.com/casey/just/issues/new")?;
-          "Internal runtime error, this may indicate a bug in just: {message} \
-           consider filing an issue: https://github.com/casey/just/issues/new"
-        )?;
       }
       InvalidDirective { line } => {
         write!(f, "Invalid directive: {line}")?;
       }
       Io { recipe, io_error } => {
         match io_error.kind() {
-          io::ErrorKind::NotFound => write!(
+          io::ErrorKind::NotFound => write!(f, "Recipe `{recipe}` could not be run because just could not find the shell: {io_error}"),
-            f,
+          io::ErrorKind::PermissionDenied => write!(f, "Recipe `{recipe}` could not be run because just could not run the shell: {io_error}"),
-            "Recipe `{recipe}` could not be run because just could not find the shell: {io_error}"
+          _ => write!(f, "Recipe `{recipe}` could not be run because of an IO error while launching the shell: {io_error}"),
-          ),
-          io::ErrorKind::PermissionDenied => write!(
-            f,
-            "Recipe `{recipe}` could not be run because just could not run the shell: {io_error}"
-          ),
-          _ => write!(
-            f,
-            "Recipe `{recipe}` could not be run because of an IO error while launching the shell: {io_error}"
-          ),
         }?;
       }
       Load { io_error, path } => {
-        write!(
+        let path = path.display();
-          f,
+        write!(f, "Failed to read justfile at `{path}`: {io_error}")?;
-          "Failed to read justfile at `{}`: {io_error}",
-          path.display()
-        )?;
-      }
-      NoChoosableRecipes => {
-        write!(f, "Justfile contains no choosable recipes.")?;
-      }
-      NoRecipes => {
-        write!(f, "Justfile contains no recipes.")?;
-      }
-      RegexCompile { source } => {
-        write!(f, "{source}")?;
       }
+      NoChoosableRecipes => write!(f, "Justfile contains no choosable recipes.")?,
+      NoRecipes => write!(f, "Justfile contains no recipes.")?,
+      RegexCompile { source } => write!(f, "{source}")?,
       Search { search_error } => Display::fmt(search_error, f)?,
-      Shebang {
+      Shebang { recipe, command, argument, io_error} => {
-        recipe,
-        command,
-        argument,
-        io_error,
-      } => {
         if let Some(argument) = argument {
-          write!(
+          write!(f, "Recipe `{recipe}` with shebang `#!{command} {argument}` execution error: {io_error}")?;
-            f,
-            "Recipe `{recipe}` with shebang `#!{command} {argument}` execution error: {io_error}",
-          )?;
         } else {
-          write!(
+          write!(f, "Recipe `{recipe}` with shebang `#!{command}` execution error: {io_error}")?;
-            f,
-            "Recipe `{recipe}` with shebang `#!{command}` execution error: {io_error}",
-          )?;
         }
       }
-      Signal {
+      Signal { recipe, line_number, signal } => {
-        recipe,
-        line_number,
-        signal,
-      } => {
         if let Some(n) = line_number {
-          write!(
+          write!(f, "Recipe `{recipe}` was terminated on line {n} by signal {signal}")?;
-            f,
-            "Recipe `{recipe}` was terminated on line {n} by signal {signal}",
-          )?;
         } else {
           write!(f, "Recipe `{recipe}` was terminated by signal {signal}")?;
         }
       }
-      TmpdirIo { recipe, io_error } => write!(
+      TmpdirIo { recipe, io_error } => {
-        f,
+        write!(f, "Recipe `{recipe}` could not be run because of an IO error while trying to create a temporary \
-        "Recipe `{recipe}` could not be run because of an IO error while trying to create a temporary \
+                   directory or write a file to that directory`:{io_error}")?;
-         directory or write a file to that directory`:{io_error}",
+      }
-      )?,
+      Unknown { recipe, line_number} => {
-      Unknown {
-        recipe,
-        line_number,
-      } => {
         if let Some(n) = line_number {
-          write!(
+          write!(f, "Recipe `{recipe}` failed on line {n} for an unknown reason")?;
-            f,
-            "Recipe `{recipe}` failed on line {n} for an unknown reason",
-          )?;
         } else {
           write!(f, "Recipe `{recipe}` failed for an unknown reason")?;
         }
       }
       UnknownOverrides { overrides } => {
-        write!(
+        let count = Count("Variable", overrides.len());
-          f,
+        let overrides = List::and_ticked(overrides);
-          "{} {} overridden on the command line but not present in justfile",
+        write!(f, "{count} {overrides} overridden on the command line but not present in justfile")?;
-          Count("Variable", overrides.len()),
-          List::and_ticked(overrides),
-        )?;
       }
-      UnknownRecipes {
+      UnknownRecipes { recipes, suggestion } => {
-        recipes,
+        let count = Count("recipe", recipes.len());
-        suggestion,
+        let recipes = List::or_ticked(recipes);
-      } => {
+        write!(f, "Justfile does not contain {count} {recipes}.")?;
-        write!(
+        if let Some(suggestion) = suggestion {
-          f,
-          "Justfile does not contain {} {}.",
-          Count("recipe", recipes.len()),
-          List::or_ticked(recipes),
-        )?;
-        if let Some(suggestion) = *suggestion {
           write!(f, "\n{suggestion}")?;
         }
       }
       Unstable { message } => {
-        write!(
+        write!(f, "{message} Invoke `just` with the `--unstable` flag to enable unstable features.")?;
-          f,
-          "{message} Invoke `just` with the `--unstable` flag to enable unstable features."
-        )?;
       }
       WriteJustfile { justfile, io_error } => {
-        write!(
+        let justfile = justfile.display();
-          f,
+        write!(f, "Failed to write justfile to `{justfile}`: {io_error}")?;
-          "Failed to write justfile to `{}`: {io_error}",
-          justfile.display()
-        )?;
       }
     }
 
@@ -634,3 +427,11 @@ impl<'src> ColorDisplay for Error<'src> {
     Ok(())
   }
 }
+
+fn format_cmd(binary: &OsString, arguments: &Vec<OsString>) -> String {
+  iter::once(binary)
+    .chain(arguments)
+    .map(|value| Enclosure::tick(value.to_string_lossy()).to_string())
+    .collect::<Vec<String>>()
+    .join(" ")
+}

tests/edit.rs

@@ -82,7 +82,7 @@ fn status_error() {
   assert!(
     Regex::new("^error: Editor `exit-2` failed: exit (code|status): 2\n$")
       .unwrap()
-      .is_match(str::from_utf8(&output.stderr).unwrap(),)
+      .is_match(str::from_utf8(&output.stderr).unwrap())
   );
 
   assert_eq!(output.status.code().unwrap(), 2);

tests/fmt.rs

@@ -98,7 +98,7 @@ fn unstable_passed() {
     panic!("justfile failed with status: {}", output.status);
   }
 
-  assert_eq!(fs::read_to_string(&justfile).unwrap(), "x := 'hello'\n",);
+  assert_eq!(fs::read_to_string(&justfile).unwrap(), "x := 'hello'\n");
 }
 
 #[test]