Skip to content

Commit 40de798

Browse files
committed
feat(rust-cli): wire IFS word splitting into for-loops and read builtin
Two key POSIX §2.6.5 integration points for $IFS word splitting: 1. **for-loop word list** — `for x in $var; do ... done` now expands each word in the `in` clause, then applies `ifs_split` using the current `$IFS` (defaulting to space/tab/newline). This means `ITEMS="a b c"; for x in $ITEMS; do mkdir $x; done` creates three directories instead of one named "a b c". 2. **read builtin** — `read a b c` now splits stdin input by `$IFS` and assigns fields to named variables. The last variable receives the remainder (POSIX behaviour). The parser now collects multiple variable names instead of just one. `read` with no args still defaults to `$REPLY`. The existing `ifs_split()` function in posix_builtins.rs (which had full POSIX semantics — whitespace/non-whitespace IFS handling, empty IFS preventing splitting, leading/trailing delimiter handling) was already tested but not wired into any execution path. Now it is. Tests: 8 new tests covering default IFS, custom IFS (comma-separated), empty IFS, newline splitting, literal word lists, and multi-variable read parsing. Test results: 729 passing, 0 failing, 14 ignored (up from 721). https://claude.ai/code/session_01EMHrh5Jq32pb98KXoSKLA4
1 parent 7695b83 commit 40de798

3 files changed

Lines changed: 210 additions & 17 deletions

File tree

impl/rust-cli/src/executable.rs

Lines changed: 54 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -674,9 +674,7 @@ impl ExecutableCommand for Command {
674674
Ok(ExecutionResult::ExternalCommand { exit_code: 1 })
675675
}
676676

677-
Command::Read { var_name, prompt, redirects: _ } => {
678-
let expanded_var = crate::parser::expand_variables(var_name, state);
679-
677+
Command::Read { var_names, prompt, redirects: _ } => {
680678
if let Some(p) = prompt {
681679
let expanded_prompt = crate::parser::expand_variables(p, state);
682680
eprint!("{}", expanded_prompt);
@@ -691,7 +689,41 @@ impl ExecutableCommand for Command {
691689
}
692690
Ok(_) => {
693691
let value = input.trim_end_matches('\n').trim_end_matches('\r');
694-
state.set_variable(expanded_var, value.to_string());
692+
693+
// POSIX §2.21: split by IFS, assign fields to vars.
694+
// The last variable gets the remainder (including
695+
// any fields beyond the variable count).
696+
let ifs = state
697+
.get_variable("IFS")
698+
.map(|s| s.to_string())
699+
.unwrap_or_else(|| {
700+
crate::posix_builtins::DEFAULT_IFS.to_string()
701+
});
702+
703+
let fields = crate::posix_builtins::ifs_split(value, &ifs);
704+
705+
for (i, var_name) in var_names.iter().enumerate() {
706+
let expanded_var =
707+
crate::parser::expand_variables(var_name, state);
708+
if i == var_names.len() - 1 {
709+
// Last variable gets the remainder.
710+
// Rejoin un-consumed fields with a single
711+
// space (POSIX behaviour).
712+
let remainder = if i < fields.len() {
713+
fields[i..].join(" ")
714+
} else {
715+
String::new()
716+
};
717+
state.set_variable(expanded_var, remainder);
718+
} else {
719+
let field = fields
720+
.get(i)
721+
.cloned()
722+
.unwrap_or_default();
723+
state.set_variable(expanded_var, field);
724+
}
725+
}
726+
695727
Ok(ExecutionResult::Success)
696728
}
697729
Err(e) => Err(anyhow::anyhow!("read: {}", e)),
@@ -891,11 +923,24 @@ impl ExecutableCommand for Command {
891923
Command::ForLoop { var, words, body } => {
892924
let mut last_result = ExecutionResult::Success;
893925

894-
for word in words {
895-
// Expand variables in the word
896-
let expanded = crate::parser::expand_variables(word, state);
926+
// POSIX §2.6.5: expand each word, then apply IFS splitting
927+
// to produce the actual iteration list.
928+
let ifs = state
929+
.get_variable("IFS")
930+
.map(|s| s.to_string())
931+
.unwrap_or_else(|| crate::posix_builtins::DEFAULT_IFS.to_string());
932+
933+
let expanded_words: Vec<String> = words
934+
.iter()
935+
.flat_map(|word| {
936+
let expanded = crate::parser::expand_variables(word, state);
937+
crate::posix_builtins::ifs_split(&expanded, &ifs)
938+
})
939+
.collect();
940+
941+
for word in &expanded_words {
897942
// Set loop variable
898-
state.set_variable(var.clone(), expanded);
943+
state.set_variable(var.clone(), word.clone());
899944

900945
// Execute body
901946
last_result = execute_block(body, state)?;
@@ -1282,7 +1327,7 @@ impl ExecutableCommand for Command {
12821327
Command::Echo { args, .. } => format!("echo {}", args.join(" ")),
12831328
Command::True => "true".to_string(),
12841329
Command::False => "false".to_string(),
1285-
Command::Read { var_name, .. } => format!("read {}", var_name),
1330+
Command::Read { var_names, .. } => format!("read {}", var_names.join(" ")),
12861331
Command::Source { file } => format!("source {}", file),
12871332
Command::Set { args } => format!("set {}", args.join(" ")),
12881333
Command::Unset { name } => format!("unset {}", name),

impl/rust-cli/src/parser.rs

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -294,9 +294,9 @@ pub enum Command {
294294
True,
295295
/// false: always returns exit code 1
296296
False,
297-
/// read: read a line from stdin into a variable
297+
/// read: read a line from stdin, split by IFS, assign to variables
298298
Read {
299-
var_name: String,
299+
var_names: Vec<String>,
300300
prompt: Option<String>,
301301
redirects: Vec<Redirection>,
302302
},
@@ -3446,23 +3446,23 @@ fn parse_base_command(cmd: &str, args: Vec<String>, redirects: Vec<Redirection>,
34463446
":" => Ok(Command::True), // POSIX no-op, same as true
34473447

34483448
"read" => {
3449-
// read [-p prompt] var_name
3450-
let mut var_name = String::new();
3449+
// read [-p prompt] var1 [var2 ...]
3450+
let mut var_names: Vec<String> = Vec::new();
34513451
let mut prompt = None;
34523452
let mut i = 0;
34533453
while i < args.len() {
34543454
if args[i] == "-p" && i + 1 < args.len() {
34553455
prompt = Some(args[i + 1].clone());
34563456
i += 2;
34573457
} else {
3458-
var_name = args[i].clone();
3458+
var_names.push(args[i].clone());
34593459
i += 1;
34603460
}
34613461
}
3462-
if var_name.is_empty() {
3463-
var_name = "REPLY".to_string();
3462+
if var_names.is_empty() {
3463+
var_names.push("REPLY".to_string());
34643464
}
3465-
Ok(Command::Read { var_name, prompt, redirects })
3465+
Ok(Command::Read { var_names, prompt, redirects })
34663466
}
34673467

34683468
"source" | "." => {
Lines changed: 148 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,148 @@
1+
// SPDX-License-Identifier: PMPL-1.0-or-later
2+
//! Tests for IFS word splitting in for-loops and related contexts.
3+
//!
4+
//! POSIX §2.6.5 says that after parameter expansion, the result of
5+
//! unquoted expansions is split into fields using `$IFS`. This affects
6+
//! the `for ... in $words` word list (each expanded word is split) and
7+
//! the `read` builtin (input is split across named variables).
8+
9+
use anyhow::Result;
10+
use tempfile::tempdir;
11+
use vsh::executable::ExecutableCommand;
12+
use vsh::parser::parse_command;
13+
use vsh::state::ShellState;
14+
15+
// -------------------------------------------------------------------------
16+
// IFS splitting in for-loops
17+
// -------------------------------------------------------------------------
18+
19+
#[test]
20+
fn for_loop_splits_variable_by_default_ifs() -> Result<()> {
21+
let temp = tempdir()?;
22+
let mut state = ShellState::new(temp.path().to_str().unwrap())?;
23+
24+
state.set_variable("ITEMS", "alpha beta gamma");
25+
26+
parse_command("for x in $ITEMS; do mkdir $x; done")?
27+
.execute(&mut state)?;
28+
29+
assert!(state.resolve_path("alpha").exists());
30+
assert!(state.resolve_path("beta").exists());
31+
assert!(state.resolve_path("gamma").exists());
32+
Ok(())
33+
}
34+
35+
#[test]
36+
fn for_loop_splits_variable_by_custom_ifs() -> Result<()> {
37+
let temp = tempdir()?;
38+
let mut state = ShellState::new(temp.path().to_str().unwrap())?;
39+
40+
state.set_variable("CSV", "one,two,three");
41+
state.set_variable("IFS", ",");
42+
43+
parse_command("for x in $CSV; do mkdir $x; done")?
44+
.execute(&mut state)?;
45+
46+
assert!(state.resolve_path("one").exists());
47+
assert!(state.resolve_path("two").exists());
48+
assert!(state.resolve_path("three").exists());
49+
Ok(())
50+
}
51+
52+
#[test]
53+
fn for_loop_with_empty_ifs_does_not_split() -> Result<()> {
54+
let temp = tempdir()?;
55+
let mut state = ShellState::new(temp.path().to_str().unwrap())?;
56+
57+
state.set_variable("WORDS", "hello world");
58+
state.set_variable("IFS", "");
59+
60+
// With empty IFS, the entire value is one word.
61+
// The for-loop body runs once and creates one dir whose name
62+
// contains a space.
63+
parse_command("for x in $WORDS; do mkdir $x; done")?
64+
.execute(&mut state)?;
65+
66+
// "hello world" stays as one token — but mkdir may receive it as
67+
// two words due to the shell's argument tokenization. The key test
68+
// is that IFS="" prevents the for-loop from splitting the expansion
69+
// into two iterations.
70+
//
71+
// Verify we did NOT get separate "hello" and "world" directories.
72+
assert!(!state.resolve_path("hello").exists());
73+
assert!(!state.resolve_path("world").exists());
74+
Ok(())
75+
}
76+
77+
#[test]
78+
fn for_loop_splits_newlines_in_variable() -> Result<()> {
79+
let temp = tempdir()?;
80+
let mut state = ShellState::new(temp.path().to_str().unwrap())?;
81+
82+
// Newline is part of default IFS
83+
state.set_variable("LINES", "line1\nline2\nline3");
84+
85+
parse_command("for x in $LINES; do mkdir $x; done")?
86+
.execute(&mut state)?;
87+
88+
assert!(state.resolve_path("line1").exists());
89+
assert!(state.resolve_path("line2").exists());
90+
assert!(state.resolve_path("line3").exists());
91+
Ok(())
92+
}
93+
94+
#[test]
95+
fn for_loop_literal_words_not_affected_by_ifs() -> Result<()> {
96+
let temp = tempdir()?;
97+
let mut state = ShellState::new(temp.path().to_str().unwrap())?;
98+
99+
// Literal words in the for-list are already split by the parser
100+
// at whitespace boundaries. IFS affects EXPANSION splitting, not
101+
// literal splitting.
102+
parse_command("for x in one two three; do mkdir $x; done")?
103+
.execute(&mut state)?;
104+
105+
assert!(state.resolve_path("one").exists());
106+
assert!(state.resolve_path("two").exists());
107+
assert!(state.resolve_path("three").exists());
108+
Ok(())
109+
}
110+
111+
// -------------------------------------------------------------------------
112+
// read builtin: IFS splitting + multiple variables
113+
// -------------------------------------------------------------------------
114+
115+
// Note: The `read` builtin reads from stdin, which we can't easily mock in
116+
// integration tests. We test the parser changes and IFS logic indirectly
117+
// through the unit tests in posix_builtins.rs and by verifying the Command
118+
// variant is parsed correctly.
119+
120+
#[test]
121+
fn read_parses_multiple_variables() -> Result<()> {
122+
let cmd = parse_command("read a b c")?;
123+
let desc = format!("{:?}", cmd);
124+
// Should have three variable names
125+
assert!(desc.contains("var_names"));
126+
assert!(desc.contains("\"a\""));
127+
assert!(desc.contains("\"b\""));
128+
assert!(desc.contains("\"c\""));
129+
Ok(())
130+
}
131+
132+
#[test]
133+
fn read_defaults_to_reply() -> Result<()> {
134+
let cmd = parse_command("read")?;
135+
let desc = format!("{:?}", cmd);
136+
assert!(desc.contains("REPLY"));
137+
Ok(())
138+
}
139+
140+
#[test]
141+
fn read_with_prompt_and_vars() -> Result<()> {
142+
let cmd = parse_command("read -p 'Enter: ' first last")?;
143+
let desc = format!("{:?}", cmd);
144+
assert!(desc.contains("\"first\""));
145+
assert!(desc.contains("\"last\""));
146+
assert!(desc.contains("Enter:"));
147+
Ok(())
148+
}

0 commit comments

Comments
 (0)