From 51aa66fef7fe5fc2bf8951f831cdd1a4d07245eb Mon Sep 17 00:00:00 2001 From: Devyn Cairns Date: Thu, 4 Apr 2024 05:23:10 -0700 Subject: [PATCH] Fix #12391: mkdir uses process startup directory instead of current script directory (#12394) # Description This fixes #12391. nushell/nushell@87c5f6e455 accidentally introduced a bug where the path was not being properly expanded according to the cwd. This makes both 'touch' and 'mkdir' use globs just like the rest of the commands to preserve tilde behavior while still expanding the paths properly. This doesn't actually expand the globs. Should it? # User-Facing Changes - Restore behavior of `mkdir`, `touch` - Help text now says they can take globs, but they won't actually expand them, maybe this should be changed # Tests + Formatting Regression tests added. # After Submitting This is severe enough and should be included in the point release. --- crates/nu-command/src/filesystem/touch.rs | 26 +++++++++++++++------- crates/nu-command/src/filesystem/umkdir.rs | 13 ++++++----- crates/nu-command/tests/commands/touch.rs | 13 +++++++++++ crates/nu-command/tests/commands/umkdir.rs | 14 ++++++++++++ 4 files changed, 52 insertions(+), 14 deletions(-) diff --git a/crates/nu-command/src/filesystem/touch.rs b/crates/nu-command/src/filesystem/touch.rs index f9e6cc10ed..f92e3ea4f7 100644 --- a/crates/nu-command/src/filesystem/touch.rs +++ b/crates/nu-command/src/filesystem/touch.rs @@ -1,8 +1,12 @@ use filetime::FileTime; -use nu_engine::command_prelude::*; +use nu_engine::{command_prelude::*, current_dir}; +use nu_path::expand_path_with; +use nu_protocol::NuGlob; use std::{fs::OpenOptions, path::Path, time::SystemTime}; +use super::util::get_rest_for_glob_pattern; + #[derive(Clone)] pub struct Touch; @@ -18,7 +22,11 @@ impl Command for Touch { fn signature(&self) -> Signature { Signature::build("touch") .input_output_types(vec![(Type::Nothing, Type::Nothing)]) - .rest("files", SyntaxShape::Filepath, "The file(s) to create.") + .rest( + "files", + SyntaxShape::OneOf(vec![SyntaxShape::GlobPattern, SyntaxShape::Filepath]), + "The file(s) to create." + ) .named( "reference", SyntaxShape::String, @@ -58,7 +66,7 @@ impl Command for Touch { let mut change_atime: bool = call.has_flag(engine_state, stack, "access")?; let reference: Option> = call.get_flag(engine_state, stack, "reference")?; let no_create: bool = call.has_flag(engine_state, stack, "no-create")?; - let files: Vec = call.rest(engine_state, stack, 0)?; + let files: Vec> = get_rest_for_glob_pattern(engine_state, stack, call, 0)?; if files.is_empty() { return Err(ShellError::MissingParameter { @@ -105,8 +113,10 @@ impl Command for Touch { })?; } - for (index, item) in files.into_iter().enumerate() { - let path = Path::new(&item); + let cwd = current_dir(engine_state, stack)?; + + for (index, glob) in files.into_iter().enumerate() { + let path = expand_path_with(glob.item.as_ref(), &cwd, glob.item.is_expand()); // If --no-create is passed and the file/dir does not exist there's nothing to do if no_create && !path.exists() { @@ -119,7 +129,7 @@ impl Command for Touch { .write(true) .create(true) .truncate(false) - .open(path) + .open(&path) { return Err(ShellError::CreateNotPossible { msg: format!("Failed to create file: {err}"), @@ -132,7 +142,7 @@ impl Command for Touch { } if change_mtime { - if let Err(err) = filetime::set_file_mtime(&item, FileTime::from_system_time(mtime)) + if let Err(err) = filetime::set_file_mtime(&path, FileTime::from_system_time(mtime)) { return Err(ShellError::ChangeModifiedTimeNotPossible { msg: format!("Failed to change the modified time: {err}"), @@ -145,7 +155,7 @@ impl Command for Touch { } if change_atime { - if let Err(err) = filetime::set_file_atime(&item, FileTime::from_system_time(atime)) + if let Err(err) = filetime::set_file_atime(&path, FileTime::from_system_time(atime)) { return Err(ShellError::ChangeAccessTimeNotPossible { msg: format!("Failed to change the access time: {err}"), diff --git a/crates/nu-command/src/filesystem/umkdir.rs b/crates/nu-command/src/filesystem/umkdir.rs index 949e6ba794..5b8d8c33cd 100644 --- a/crates/nu-command/src/filesystem/umkdir.rs +++ b/crates/nu-command/src/filesystem/umkdir.rs @@ -1,10 +1,11 @@ -use nu_engine::command_prelude::*; +use nu_engine::{command_prelude::*, current_dir}; -use std::path::PathBuf; use uu_mkdir::mkdir; #[cfg(not(windows))] use uucore::mode; +use super::util::get_rest_for_glob_pattern; + #[derive(Clone)] pub struct UMkdir; @@ -39,7 +40,7 @@ impl Command for UMkdir { .input_output_types(vec![(Type::Nothing, Type::Nothing)]) .rest( "rest", - SyntaxShape::Directory, + SyntaxShape::OneOf(vec![SyntaxShape::GlobPattern, SyntaxShape::Directory]), "The name(s) of the path(s) to create.", ) .switch( @@ -57,10 +58,10 @@ impl Command for UMkdir { call: &Call, _input: PipelineData, ) -> Result { - let mut directories = call - .rest::(engine_state, stack, 0)? + let cwd = current_dir(engine_state, stack)?; + let mut directories = get_rest_for_glob_pattern(engine_state, stack, call, 0)? .into_iter() - .map(PathBuf::from) + .map(|dir| nu_path::expand_path_with(dir.item.as_ref(), &cwd, dir.item.is_expand())) .peekable(); let is_verbose = call.has_flag(engine_state, stack, "verbose")?; diff --git a/crates/nu-command/tests/commands/touch.rs b/crates/nu-command/tests/commands/touch.rs index f5b3d6f611..d7f5cb875a 100644 --- a/crates/nu-command/tests/commands/touch.rs +++ b/crates/nu-command/tests/commands/touch.rs @@ -502,3 +502,16 @@ fn create_a_file_with_tilde() { assert!(files_exist_at(vec![Path::new("~tilde2")], dirs.test())); }) } + +#[test] +fn respects_cwd() { + Playground::setup("touch_respects_cwd", |dirs, _sandbox| { + nu!( + cwd: dirs.test(), + "mkdir 'dir'; cd 'dir'; touch 'i_will_be_created.txt'" + ); + + let path = dirs.test().join("dir/i_will_be_created.txt"); + assert!(path.exists()); + }) +} diff --git a/crates/nu-command/tests/commands/umkdir.rs b/crates/nu-command/tests/commands/umkdir.rs index ebb2b932ab..3ca1b3bdfb 100644 --- a/crates/nu-command/tests/commands/umkdir.rs +++ b/crates/nu-command/tests/commands/umkdir.rs @@ -123,6 +123,20 @@ fn creates_directory_three_dots_quotation_marks() { }) } +#[test] +fn respects_cwd() { + Playground::setup("mkdir_respects_cwd", |dirs, _| { + nu!( + cwd: dirs.test(), + "mkdir 'some_folder'; cd 'some_folder'; mkdir 'another/deeper_one'" + ); + + let expected = dirs.test().join("some_folder/another/deeper_one"); + + assert!(expected.exists()); + }) +} + #[cfg(not(windows))] #[test] fn mkdir_umask_permission() {