fix(shell): expanded tilde for every brace-expansion element

Brace expansion joined its results with spaces and re-parsed them as a single word, so tilde-at-word-start only fired on the leading element. Now each brace element is expanded as its own word, so `~/project/{a,b}` expands both tildes instead of leaving a literal `~/project/b`.

Fixes #5819
This commit is contained in:
roboomp
2026-07-17 09:15:30 +00:00
parent 0f9fceeea4
commit 9e73821202
3 changed files with 132 additions and 27 deletions
+43
View File
@@ -2210,6 +2210,49 @@ mod tests {
let _ = std::fs::remove_dir_all(&tmp);
}
/// Regression test for issue #5819: `mkdir -p ~/proj/{a,b}` must create both
/// `a` and `b` under `$HOME/proj`. Brace expansion runs before tilde
/// expansion and previously left every element after the first with a
/// literal `~`, so `b` was created as `./~/proj/b` in the shell cwd instead.
#[tokio::test(flavor = "multi_thread")]
async fn uutils_mkdir_expands_tilde_for_every_brace_element() {
let base = std::env::temp_dir().join(format!("pi-mkdir-brace-{}", std::process::id()));
let home = base.join("home");
let cwd = base.join("cwd");
let _ = std::fs::remove_dir_all(&base);
std::fs::create_dir_all(&home).expect("home dir");
std::fs::create_dir_all(&cwd).expect("cwd dir");
let cwd_str = cwd.to_str().expect("utf8 cwd path");
let mut env = HashMap::new();
env.insert("HOME".to_string(), home.to_string_lossy().to_string());
let config = ShellConfig { session_env: Some(env), snapshot_path: None, minimizer: None };
let mut session = create_session(&config).await.expect("create_session");
session.shell.set_working_dir(cwd_str).expect("set cwd");
let mut params = session.shell.default_exec_params();
params.set_fd(OpenFiles::STDIN_FD, null_file().expect("null"));
params.set_fd(OpenFiles::STDOUT_FD, null_file().expect("null"));
params.set_fd(OpenFiles::STDERR_FD, null_file().expect("null"));
let source_info = SourceInfo::from("pi-natives:test");
let exec = session
.shell
.run_string("mkdir -p ~/proj/{a,b}", &source_info, &params)
.await
.expect("run_string");
assert!(matches!(exec.exit_code, ExecutionExitCode::Success), "exit {}", exit_code(&exec));
// Both elements' tildes expanded: dirs land under $HOME/proj.
assert!(home.join("proj/a").is_dir(), "~/proj/a not created under HOME");
assert!(home.join("proj/b").is_dir(), "~/proj/b not created under HOME");
// The buggy path created a literal `~` tree in the shell cwd.
assert!(!cwd.join("~").exists(), "literal ~ tree leaked into cwd");
assert!(!cwd.join("a").exists(), "unexpanded element leaked into cwd");
let _ = std::fs::remove_dir_all(&base);
}
/// `mkdir --help` and an invalid flag must be handled in-process: rendered
/// to the command streams and returned as an exit code. The upstream
/// `uumain` parser calls `std::process::exit`, which would terminate the
+85 -27
View File
@@ -626,30 +626,44 @@ impl<'a, SE: extensions::ShellExtensions> WordExpander<'a, SE> {
return Ok(Expansion::from(ExpansionPiece::Splittable(word.to_owned())));
}
// Apply brace expansion first, before anything else (not applicable to heredoc
// bodies).
let brace_expanded = self.brace_expand_if_needed(word)?;
// Apply brace expansion first, before anything else (not applicable to
// heredoc bodies). Each resulting element is an independent word: bash
// runs tilde/parameter/command/arithmetic expansion on EVERY element, so
// a tilde that begins any element (e.g. `~/{a,b}` -> `~/a` and `~/b`)
// must expand — not only the first. Parsing the space-joined result as a
// single word left every element after the first with a literal leading
// `~` (issue #5819).
let brace_expanded_words = self.brace_expand_words(word)?;
if tracing::enabled!(target: trace_categories::EXPANSION, tracing::Level::DEBUG)
&& brace_expanded != word
&& !(brace_expanded_words.len() == 1 && brace_expanded_words[0] == word)
{
tracing::debug!(target: trace_categories::EXPANSION, " => brace expanded to '{brace_expanded}'");
tracing::debug!(target: trace_categories::EXPANSION, " => brace expanded to {brace_expanded_words:?}");
}
// Expand: tildes, parameters, command substitutions, arithmetic.
let pieces = if self.heredoc_mode {
// Heredoc mode only affects top-level parsing (literal quotes); recursive
// expansion of parameter words (e.g., ${var:-"default"}) uses normal semantics.
self.heredoc_mode = false;
brush_parser::word::parse_heredoc(brace_expanded.as_ref(), &self.parser_options)?
} else {
brush_parser::word::parse(brace_expanded.as_ref(), &self.parser_options)?
};
// Expand each brace element separately (tildes, parameters, command
// substitutions, arithmetic), separating elements with a splittable
// space so downstream field splitting yields one field per element.
let mut expansions = vec![];
for piece in pieces {
let piece_expansion = self.expand_word_piece(piece.piece).await?;
expansions.push(piece_expansion);
for (index, element) in brace_expanded_words.iter().enumerate() {
if index > 0 {
expansions.push(Expansion::from(ExpansionPiece::Splittable(String::from(" "))));
}
let pieces = if self.heredoc_mode {
// Heredoc mode only affects top-level parsing (literal quotes);
// recursive expansion of parameter words (e.g., ${var:-"default"})
// uses normal semantics.
self.heredoc_mode = false;
brush_parser::word::parse_heredoc(element.as_ref(), &self.parser_options)?
} else {
brush_parser::word::parse(element.as_ref(), &self.parser_options)?
};
for piece in pieces {
let piece_expansion = self.expand_word_piece(piece.piece).await?;
expansions.push(piece_expansion);
}
}
let coalesced = coalesce_expansions(expansions);
@@ -695,7 +709,13 @@ impl<'a, SE: extensions::ShellExtensions> WordExpander<'a, SE> {
}
}
fn brace_expand_if_needed(&self, word: &'a str) -> Result<Cow<'a, str>, error::Error> {
/// Perform brace expansion on `word`, returning each expanded element as a
/// separate word. When brace expansion doesn't apply (disabled, no braces,
/// or a parse failure), the original word is returned as the sole element.
///
/// Empty brace elements (e.g. from `{,b}`) are returned as a quoted empty
/// string (`""`) so they survive as empty fields, matching bash.
fn brace_expand_words(&self, word: &'a str) -> Result<Vec<Cow<'a, str>>, error::Error> {
// We perform a non-authoritative check to see if the string *may* contain
// braces to expand. There may be false positives, but must be no false
// negatives.
@@ -703,28 +723,40 @@ impl<'a, SE: extensions::ShellExtensions> WordExpander<'a, SE> {
|| !self.shell.options().perform_brace_expansion
|| !may_contain_braces_to_expand(word)
{
return Ok(word.into());
return Ok(vec![word.into()]);
}
let parse_result = brush_parser::word::parse_brace_expansions(word, &self.parser_options);
if parse_result.is_err() {
tracing::error!("failed to parse for brace expansion: {parse_result:?}");
return Ok(word.into());
return Ok(vec![word.into()]);
}
let brace_expansion_pieces = parse_result?;
let Some(brace_expansion_pieces) = brace_expansion_pieces else {
return Ok(word.into());
return Ok(vec![word.into()]);
};
tracing::debug!(target: trace_categories::EXPANSION, "Brace expansion pieces: {brace_expansion_pieces:?}");
let result = braceexpansion::generate_and_combine_brace_expansions(brace_expansion_pieces)
let words = braceexpansion::generate_and_combine_brace_expansions(brace_expansion_pieces)
.into_iter()
.map(|s| if s.is_empty() { "\"\"".into() } else { s })
.join(" ");
.map(|s| if s.is_empty() { Cow::Borrowed("\"\"") } else { Cow::Owned(s) })
.collect();
Ok(result.into())
Ok(words)
}
/// Convenience wrapper over [`Self::brace_expand_words`] that joins the
/// expanded elements back into a single space-separated string.
#[cfg(test)]
fn brace_expand_if_needed(&self, word: &'a str) -> Result<Cow<'a, str>, error::Error> {
let mut words = self.brace_expand_words(word)?;
if words.len() == 1 {
Ok(words.pop().unwrap())
} else {
Ok(Cow::Owned(words.join(" ")))
}
}
/// Apply tilde-expansion, parameter expansion, command substitution, and
@@ -2082,6 +2114,32 @@ mod tests {
Ok(())
}
/// Regression test for issue #5819: a tilde that begins each element of a
/// brace expansion must expand independently. Brace expansion joins its
/// elements before the tilde/parameter/... pass, so `~/{a,b}` must yield
/// `<HOME>/a` and `<HOME>/b` — not `<HOME>/a` followed by a literal `~/b`.
#[tokio::test]
async fn test_tilde_expands_for_every_brace_element() -> Result<()> {
let mut shell = crate::shell::Shell::builder().build().await?;
shell
.env_mut()
.set_global("HOME", ShellVariable::new(ShellValue::String("/home/user".to_string())))?;
let params = shell.default_exec_params();
assert_eq!(
full_expand_and_split_word(&mut shell, &params, "~/project/{a,b}").await?,
vec!["/home/user/project/a", "/home/user/project/b"],
);
// A bare `~/{a,b}` (tilde immediately followed by the brace) must also
// expand on both elements.
assert_eq!(
full_expand_and_split_word(&mut shell, &params, "~/{a,b}").await?,
vec!["/home/user/a", "/home/user/b"],
);
Ok(())
}
#[tokio::test]
async fn test_field_splitting() -> Result<()> {
let mut shell = crate::shell::Shell::builder().build().await?;
+4
View File
@@ -2,6 +2,10 @@
## [Unreleased]
### Fixed
- Fixed `~` (tilde) not expanding for every element of a brace expansion in the bash tool, so `mkdir -p ~/project/{a,b}` now creates both `a` and `b` under `$HOME/project` instead of leaving a literal `~/project/b` in the working directory ([#5819](https://github.com/can1357/oh-my-pi/issues/5819)).
## [17.0.2] - 2026-07-17
### Fixed