From 83b1ec83c9967467aef61ccc2b8ab6f3dbf9bcd0 Mon Sep 17 00:00:00 2001 From: Erich Gubler Date: Wed, 3 May 2023 17:12:16 -0400 Subject: [PATCH] feat(rm)!: use arg. spans for I/O errors (#8964) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit # Description Currently, error spans for I/O errors in an `rm` invocation always point to the `rm` argument. This isn't ideal, because the user loses context as to which “target” actually had a problem: ![image](https://user-images.githubusercontent.com/658538/235723366-50db727e-9ba2-4d16-afc6-6a2406c584e0.png) Shadow the existing `span` variable in outer scope in `rm`'s implementation for the errors that may be detected while handling I/O results. This is desired, because all failures from this point are target-specific, and pointing at the argument that generated the target instead is better. The end user should now see this: ![image](https://user-images.githubusercontent.com/658538/235724345-1d2e98e0-6b20-4bf5-b8a2-8b4368cdfb05.png) # User-Facing Changes * When `rm` encounters I/O errors, their spans now point to the “target” argument associated with the error, rather than the `rm` token. # Tests + Formatting No tests currently cover this. I'm open to adding tests, but adding as follow-up sounds better ATM, since this wasn't covered before. # After Submitting Nothing needs to be done here, AFAIK. No I/O errors are currently demonstrated in official docs, though maybe they should be? --- crates/nu-command/src/filesystem/rm.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/nu-command/src/filesystem/rm.rs b/crates/nu-command/src/filesystem/rm.rs index 126492aa96..a4ca55449e 100644 --- a/crates/nu-command/src/filesystem/rm.rs +++ b/crates/nu-command/src/filesystem/rm.rs @@ -318,8 +318,8 @@ fn rm( } all_targets - .into_keys() - .map(move |f| { + .into_iter() + .map(move |(f, span)| { let is_empty = || match f.read_dir() { Ok(mut p) => p.next().is_none(), Err(_) => false,