From f04db2a7a3814cbf582715e97cfec96dc388eb30 Mon Sep 17 00:00:00 2001
From: 132ikl <132@ikl.sh>
Date: Mon, 3 Feb 2025 09:55:54 -0500
Subject: [PATCH] Swap additional context and label in I/O errors (#14954)

<!--
if this PR closes one or more issues, you can automatically link the PR
with
them by using one of the [*linking
keywords*](https://docs.github.com/en/issues/tracking-your-work-with-issues/linking-a-pull-request-to-an-issue#linking-a-pull-request-to-an-issue-using-a-keyword),
e.g.
- this PR should close #xxxx
- fixes #xxxx

you can also mention related issues, PRs or discussions!
-->

# Description
<!--
Thank you for improving Nushell. Please, check our [contributing
guide](../CONTRIBUTING.md) and talk to the core team before making major
changes.

Description of your pull request goes here. **Provide examples and/or
screenshots** if your changes affect the user experience.
-->

Tweaks the error style for I/O errors introduced #14927. Moves the
additional context to below the text that says "I/O error", and always
shows the error kind in the label.

Additional context|Before PR|After PR
:-:|:-:|:-:

yes|![image](https://github.com/user-attachments/assets/df4f2e28-fdf5-4693-b60c-255d019af25f)
|
![image](https://github.com/user-attachments/assets/5915e9d0-78d4-49a6-b495-502d0c6444fa)
no|
![image](https://github.com/user-attachments/assets/e4ecaada-ec8c-4940-b08a-bbfaa45083d5)
|
![image](https://github.com/user-attachments/assets/467163d8-ab39-47f0-a74f-e2effe2fe6af)



# User-Facing Changes
<!-- List of all changes that impact the user experience here. This
helps us keep track of breaking changes. -->

N/A, as this is a follow-up to #14927 which has not been included in a
release

# Tests + Formatting
<!--
Don't forget to add tests that cover your changes.

Make sure you've run and fixed any issues with these commands:

- `cargo fmt --all -- --check` to check standard code formatting (`cargo
fmt --all` applies these changes)
- `cargo clippy --workspace -- -D warnings -D clippy::unwrap_used` to
check that you're using the standard code style
- `cargo test --workspace` to check that all tests pass (on Windows make
sure to [enable developer
mode](https://learn.microsoft.com/en-us/windows/apps/get-started/developer-mode-features-and-debugging))
- `cargo run -- -c "use toolkit.nu; toolkit test stdlib"` to run the
tests for the standard library

> **Note**
> from `nushell` you can also use the `toolkit` as follows
> ```bash
> use toolkit.nu # or use an `env_change` hook to activate it
automatically
> toolkit check pr
> ```
-->
N/A

# After Submitting
<!-- If your PR had any user-facing changes, update [the
documentation](https://github.com/nushell/nushell.github.io) after the
PR is merged, if necessary. This will help us keep the docs up to date.
-->
N/A

---------

Co-authored-by: Piepmatz <git+github@cptpiepmatz.de>
---
 crates/nu-command/src/filesystem/open.rs      | 17 ++++++-----
 .../nu-protocol/src/errors/shell_error/io.rs  | 28 ++++++++++++-------
 2 files changed, 28 insertions(+), 17 deletions(-)

diff --git a/crates/nu-command/src/filesystem/open.rs b/crates/nu-command/src/filesystem/open.rs
index a2a63f180b..f1ea73c6c8 100644
--- a/crates/nu-command/src/filesystem/open.rs
+++ b/crates/nu-command/src/filesystem/open.rs
@@ -118,13 +118,16 @@ impl Command for Open {
                     #[cfg(unix)]
                     let err = {
                         let mut err = err;
-                        err.additional_context = Some(match path.metadata() {
-                            Ok(md) => format!(
-                                "The permissions of {:o} does not allow access for this user",
-                                md.permissions().mode() & 0o0777
-                            ),
-                            Err(e) => e.to_string(),
-                        });
+                        err.additional_context = Some(
+                            match path.metadata() {
+                                Ok(md) => format!(
+                                    "The permissions of {:o} does not allow access for this user",
+                                    md.permissions().mode() & 0o0777
+                                ),
+                                Err(e) => e.to_string(),
+                            }
+                            .into(),
+                        );
                         err
                     };
 
diff --git a/crates/nu-protocol/src/errors/shell_error/io.rs b/crates/nu-protocol/src/errors/shell_error/io.rs
index 88072ec45e..83836bff73 100644
--- a/crates/nu-protocol/src/errors/shell_error/io.rs
+++ b/crates/nu-protocol/src/errors/shell_error/io.rs
@@ -115,7 +115,7 @@ pub struct IoError {
     ///
     /// Only set this field if it adds meaningful context.
     /// If [`ErrorKind`] already contains all the necessary information, leave this as [`None`].
-    pub additional_context: Option<String>,
+    pub additional_context: Option<AdditionalContext>,
 
     /// The precise location in the Rust code where the error originated.
     ///
@@ -139,6 +139,16 @@ pub enum ErrorKind {
     IsADirectory,
 }
 
+#[derive(Debug, Clone, PartialEq, Eq, Error, Diagnostic)]
+#[error("{0}")]
+pub struct AdditionalContext(String);
+
+impl From<String> for AdditionalContext {
+    fn from(value: String) -> Self {
+        AdditionalContext(value)
+    }
+}
+
 impl IoError {
     /// Creates a new [`IoError`] with the given kind, span, and optional path.
     ///
@@ -209,7 +219,7 @@ impl IoError {
             kind: kind.into(),
             span,
             path,
-            additional_context: Some(additional_context.to_string()),
+            additional_context: Some(additional_context.to_string().into()),
             location: None,
         }
     }
@@ -249,7 +259,7 @@ impl IoError {
             kind: kind.into(),
             span: Span::unknown(),
             path: None,
-            additional_context: Some(additional_context.to_string()),
+            additional_context: Some(additional_context.to_string().into()),
             location: Some(location.to_string()),
         }
     }
@@ -283,7 +293,7 @@ impl IoError {
             kind: kind.into(),
             span: Span::unknown(),
             path: path.into(),
-            additional_context: Some(additional_context.to_string()),
+            additional_context: Some(additional_context.to_string().into()),
             location: Some(location.to_string()),
         }
     }
@@ -370,16 +380,14 @@ impl Diagnostic for IoError {
             (true, Some(location)) => SourceSpan::new(0.into(), location.len()),
         };
 
-        let label = match self.additional_context.as_ref() {
-            Some(ctx) => format!("{ctx}\n{}", self.kind),
-            None => self.kind.to_string(),
-        };
-        let label = LabeledSpan::new_with_span(Some(label), span);
+        let label = LabeledSpan::new_with_span(Some(self.kind.to_string()), span);
         Some(Box::new(std::iter::once(label)))
     }
 
     fn diagnostic_source(&self) -> Option<&dyn Diagnostic> {
-        Some(&self.kind as &dyn Diagnostic)
+        self.additional_context
+            .as_ref()
+            .map(|ctx| ctx as &dyn Diagnostic)
     }
 
     fn source_code(&self) -> Option<&dyn miette::SourceCode> {