Skip to content

fix(interpreter): close Oils word-split, globignore, dbracket, cd and alias gaps #2641

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
chaliy merged 1 commit into from
Oct 8, 2026
Merged

fix(interpreter): close Oils word-split, globignore, dbracket, cd and alias gaps #2641

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
44 changes: 39 additions & 5 deletions crates/bashkit/src/builtins/alias.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,22 +24,52 @@ impl Builtin for Alias {
return Ok(ExecResult::ok(String::new()));
};

if ctx.args.is_empty() {
// Leading options: `-p` lists, `--` ends them.
let mut args: &[String] = ctx.args;
let mut print_all = false;
while let Some(arg) = args.first() {
if arg == "--" {
args = &args[1..];
break;
}
if arg.len() < 2 || !arg.starts_with('-') {
break;
}
if arg.chars().skip(1).all(|c| c == 'p') {
print_all = true;
args = &args[1..];
continue;
}
let bad = arg.chars().nth(1).unwrap_or('-');
return Ok(ExecResult::err(
format!(
"bash: alias: -{bad}: invalid option\n\
alias: usage: alias [-p] [name[=value] ... ]\n"
),
2,
));
}

let mut print_listing = String::new();
if args.is_empty() || print_all {
// List all aliases
let mut sorted: Vec<_> = shell.aliases.iter().collect();
sorted.sort_by_key(|(k, _)| (*k).clone());
let mut output = String::new();
for (name, value) in sorted {
output.push_str(&format!("alias {name}={}\n", single_quote(value)));
}
return Ok(ExecResult::ok(output));
if args.is_empty() {
return Ok(ExecResult::ok(output));
}
print_listing = output;
}

let mut output = String::new();
let mut output = print_listing;
let mut exit_code = 0;
let mut stderr = String::new();

for arg in ctx.args {
for arg in args {
if let Some(eq_pos) = arg.find('=') {
// alias name=value — set directly
let name = &arg[..eq_pos];
Expand Down Expand Up @@ -92,7 +122,11 @@ impl Builtin for Unalias {
let mut exit_code = 0;
let mut stderr = String::new();

for arg in ctx.args {
let args = match ctx.args.first().map(String::as_str) {
Some("--") => &ctx.args[1..],
_ => ctx.args,
};
for arg in args {
if arg == "-a" {
shell.aliases.clear();
} else if shell.aliases.remove(arg.as_str()).is_none() {
Expand Down
201 changes: 160 additions & 41 deletions crates/bashkit/src/builtins/navigation.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,70 +8,189 @@ use crate::error::Result;
use crate::interpreter::ExecResult;

/// The cd builtin - change directory.
///
/// `cd [-L|-P [-e]] [-@] [dir]`. Paths resolve logically (`..` drops the
/// previous component, L-FS-001) but, as in bash, every component before a
/// `..` must exist. `-P` resolves symlinks. A relative `dir` that does not
/// start with `.` or `..` is looked up in `CDPATH` first; `cd -` and a
/// non-empty `CDPATH` hit print the new directory.
pub struct Cd;

#[async_trait]
impl Builtin for Cd {
async fn execute(&self, ctx: Context<'_>) -> Result<ExecResult> {
let Some(target) = ctx
.args
.first()
.map(|s| s.as_str())
.or_else(|| ctx.variables.get("HOME").map(|s| s.as_str()))
.or_else(|| ctx.env.get("HOME").map(|s| s.as_str()))
else {
let mut physical = false;
let mut operands: &[String] = ctx.args;
while let Some(arg) = operands.first() {
if arg == "--" {
operands = &operands[1..];
break;
}
if arg.len() < 2 || !arg.starts_with('-') {
break;
}
for ch in arg.chars().skip(1) {
match ch {
'L' => physical = false,
'P' => physical = true,
'e' | '@' => {}
_ => {
return Ok(ExecResult::err(
format!(
"bash: cd: -{ch}: invalid option\n\
cd: usage: cd [-L|[-P [-e]] [-@]] [dir]\n"
),
2,
));
}
}
}
operands = &operands[1..];
}
if operands.len() > 1 {
return Ok(ExecResult::err("bash: cd: too many arguments\n", 1));
}

let var = |name: &str| {
ctx.variables
.get(name)
.or_else(|| ctx.env.get(name))
.cloned()
};
let mut print_dir = false;
let target = match operands.first() {
Some(t) if t == "-" => {
print_dir = true;
match var("OLDPWD") {
Some(old) => old,
None => return Ok(ExecResult::err("bash: cd: OLDPWD not set\n", 1)),
}
}
Some(t) => t.clone(),
// Bare `cd` with HOME unset fails, as in bash.
return Ok(ExecResult::err("bash: cd: HOME not set\n", 1));
None => match var("HOME") {
Some(home) => home,
None => return Ok(ExecResult::err("bash: cd: HOME not set\n", 1)),
},
};
// `cd ""` stays put (bash 5.2).
if target.is_empty() {
return Ok(ExecResult::ok(""));
}

let new_path = if target.starts_with('/') {
PathBuf::from(target)
} else if target == "-" {
// Go to previous directory
ctx.variables
.get("OLDPWD")
.or_else(|| ctx.env.get("OLDPWD"))
.map(PathBuf::from)
.unwrap_or_else(|| ctx.cwd.clone())
let mut candidates: Vec<(PathBuf, bool)> = Vec::new();
let dot_relative = target == "."
|| target == ".."
|| target.starts_with("./")
|| target.starts_with("../");
if !target.starts_with('/')
&& !dot_relative
&& let Some(cdpath) = var("CDPATH")
{
for entry in cdpath.split(':') {
let base = if entry.is_empty() {
ctx.cwd.clone()
} else {
vfs_join(ctx.cwd, entry)
};
candidates.push((vfs_join(&base, &target), !entry.is_empty()));
}
}
let direct = if target.starts_with('/') {
PathBuf::from(&target)
} else {
vfs_join(ctx.cwd, target)
vfs_join(ctx.cwd, &target)
};
candidates.push((direct, false));

// Normalize the path
let normalized = normalize_path(&new_path);

// Check if directory exists
if ctx.fs.exists(&normalized).await? {
let metadata = ctx.fs.stat(&normalized).await?;
if metadata.file_type.is_dir() {
// Set OLDPWD before changing directory
let old_cwd = ctx.cwd.to_string_lossy().to_string();
ctx.variables.insert("OLDPWD".to_string(), old_cwd);
*ctx.cwd = normalized;
Ok(ExecResult::ok(""))
} else {
Ok(ExecResult::err(
format!("cd: {}: Not a directory\n", target),
1,
))
for (path, from_cdpath) in candidates {
let Some(mut resolved) = logical_dir(ctx.fs.as_ref(), &path).await else {
continue;
};
if physical {
match crate::fs::canonicalize(ctx.fs.as_ref(), &resolved).await {
Ok(real) => resolved = real,
Err(_) => continue,
}
}
let old_cwd = ctx.cwd.to_string_lossy().to_string();
ctx.variables.insert("OLDPWD".to_string(), old_cwd);
*ctx.cwd = resolved;
let out = if print_dir || from_cdpath {
format!("{}\n", ctx.cwd.to_string_lossy())
} else {
String::new()
};
return Ok(ExecResult::ok(out));
}

// Report why the direct path failed.
let path = if target.starts_with('/') {
PathBuf::from(&target)
} else {
Ok(ExecResult::err(
format!("cd: {}: No such file or directory\n", target),
1,
))
vfs_join(ctx.cwd, &target)
};
let normalized = normalize_path(&path);
let reason = match ctx.fs.stat(&normalized).await {
Ok(m) if !m.file_type.is_dir() => "Not a directory",
_ => "No such file or directory",
};
Ok(ExecResult::err(format!("cd: {target}: {reason}\n"), 1))
}
}

/// Resolve `path` logically: `.` drops, `..` pops the previous component,
/// but only after checking that what it pops is an existing directory
/// (bash: `cd nope/..` fails). Returns the normalized path when it names a
/// directory.
async fn logical_dir(fs: &dyn crate::fs::FileSystem, path: &std::path::Path) -> Option<PathBuf> {
use std::path::Component;
let mut out = PathBuf::from("/");
for comp in path.components() {
match comp {
Component::Normal(name) => out.push(name),
Component::ParentDir => {
if !fs.stat(&out).await.is_ok_and(|m| m.file_type.is_dir()) {
return None;
}
out.pop();
}
_ => {}
}
}
let out = normalize_path(&out);
fs.stat(&out)
.await
.is_ok_and(|m| m.file_type.is_dir())
.then_some(out)
}

/// The pwd builtin - print working directory.
///
/// `-P` prints the path with every symlink resolved; `-L` (default) the
/// logical one.
pub struct Pwd;

#[async_trait]
impl Builtin for Pwd {
async fn execute(&self, ctx: Context<'_>) -> Result<ExecResult> {
let cwd = ctx.cwd.to_string_lossy();
Ok(ExecResult::ok(format!("{}\n", cwd)))
let mut physical = false;
for arg in ctx.args {
match arg.as_str() {
"-P" => physical = true,
"-L" => physical = false,
"-LP" | "-PL" => physical = arg.ends_with('P'),
_ => {}
}
}
let cwd = if physical {
crate::fs::canonicalize(ctx.fs.as_ref(), ctx.cwd)
.await
.unwrap_or_else(|_| ctx.cwd.clone())
} else {
ctx.cwd.clone()
};
Ok(ExecResult::ok(format!("{}\n", cwd.to_string_lossy())))
}
}

Expand Down
Loading
Loading