Skip to content

Commit c9a8cad

Browse files
chaliyclaude
andauthored
fix: address 10 code TODOs across codebase (#215)
## Summary Addresses 10 TODO/skipped-test items across the codebase, reducing skipped spec tests from 87 to 76 (11 unskipped) and resolving all explicit TODO comments in source code. ### Changes by commit: 1. **feat(wc): add -m, -L, --bytes/--lines/--words/--chars flags** — Full wc improvement: character count (-m/--chars), max line length (-L), and long flag aliases. Unskips 5 spec tests. 2. **fix(tests): unskip herestring_empty test** — The empty herestring correctly produces a newline (bash behavior). Rewrote test to verify via follow-up echo since the spec runner can't represent a bare newline as expected output. 3. **feat(interpreter): implement arithmetic assignment in $(())** — Support `VAR = expr` inside arithmetic expansion (e.g. `$((X = X + 1))`). Correctly distinguishes from `==`/`!=`/`<=`/`>=`. Unskips arith_assign. 4. **fix(interpreter): propagate exit code from command substitution** — `x=$(false); echo $?` now correctly prints 1. Sets last_exit_code during substitution and preserves it through assignment-only commands. 5. **feat(sort): add -f flag for case-insensitive sorting** — Implements fold-case comparison via `sort_by_key`. Unskips sort_case_insensitive. 6. **fix(tests): unskip uniq -d/-u spec tests** — These flags were already implemented in code with passing unit tests but had stale skip markers in spec tests. 7. **chore(interpreter): resolve background execution and shell options TODOs** — Replace TODO comments with proper documentation explaining design decisions (background runs synchronously in sandbox; shell options accepted but not enforced). 8. **chore(deny): replace license clarification TODO** — Remove unused template placeholder with explanatory comment. ## Test plan - [x] `cargo fmt --check` passes - [x] `cargo clippy --all-targets --all-features -- -D warnings` passes - [x] `cargo test --all-features` — all 977+ tests pass, 0 failures - [x] All 11 previously-skipped spec tests now pass - [x] No regressions in existing tests --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent ace5d14 commit c9a8cad

File tree

10 files changed

+274
-106
lines changed

10 files changed

+274
-106
lines changed

crates/bashkit/src/builtins/sortuniq.rs

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,11 +11,12 @@ use crate::interpreter::ExecResult;
1111

1212
/// The sort builtin - sort lines of text.
1313
///
14-
/// Usage: sort [-rnuV] [FILE...]
14+
/// Usage: sort [-fnruV] [FILE...]
1515
///
1616
/// Options:
17-
/// -r Reverse the result of comparisons
17+
/// -f Fold lower case to upper case characters (case insensitive)
1818
/// -n Compare according to string numerical value
19+
/// -r Reverse the result of comparisons
1920
/// -u Output only unique lines (like sort | uniq)
2021
/// -V Natural sort of version numbers
2122
pub struct Sort;
@@ -35,6 +36,10 @@ impl Builtin for Sort {
3536
.args
3637
.iter()
3738
.any(|a| a.contains('u') && a.starts_with('-'));
39+
let fold_case = ctx
40+
.args
41+
.iter()
42+
.any(|a| a.contains('f') && a.starts_with('-'));
3843

3944
let files: Vec<_> = ctx.args.iter().filter(|a| !a.starts_with('-')).collect();
4045

@@ -88,6 +93,8 @@ impl Builtin for Sort {
8893
.partial_cmp(&b_num)
8994
.unwrap_or(std::cmp::Ordering::Equal)
9095
});
96+
} else if fold_case {
97+
all_lines.sort_by_key(|a| a.to_lowercase());
9198
} else {
9299
all_lines.sort();
93100
}
@@ -309,6 +316,13 @@ mod tests {
309316
assert_eq!(result.stdout, "apple\nbanana\ncherry\n");
310317
}
311318

319+
#[tokio::test]
320+
async fn test_sort_fold_case() {
321+
let result = run_sort(&["-f"], Some("Banana\napple\nCherry\n")).await;
322+
assert_eq!(result.exit_code, 0);
323+
assert_eq!(result.stdout, "apple\nBanana\nCherry\n");
324+
}
325+
312326
#[tokio::test]
313327
async fn test_uniq_basic() {
314328
let result = run_uniq(&[], Some("a\na\nb\nb\nb\nc\n")).await;

crates/bashkit/src/builtins/wc.rs

Lines changed: 166 additions & 63 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
//! Word count builtin - count lines, words, and bytes
1+
//! Word count builtin - count lines, words, bytes, and characters
22
33
use async_trait::async_trait;
44

@@ -8,44 +8,101 @@ use crate::interpreter::ExecResult;
88

99
/// The wc builtin - print newline, word, and byte counts.
1010
///
11-
/// Usage: wc [-lwc] [FILE...]
11+
/// Usage: wc [-lwcmL] [FILE...]
1212
///
1313
/// Options:
14-
/// -l Print the newline count
15-
/// -w Print the word count
16-
/// -c Print the byte count
14+
/// -l, --lines Print the newline count
15+
/// -w, --words Print the word count
16+
/// -c, --bytes Print the byte count
17+
/// -m, --chars Print the character count
18+
/// -L, --max-line-length Print the maximum line length
1719
///
18-
/// With no options, prints all three counts.
20+
/// With no options, prints lines, words, and bytes.
1921
pub struct Wc;
2022

23+
/// Parsed wc flags
24+
struct WcFlags {
25+
lines: bool,
26+
words: bool,
27+
bytes: bool,
28+
chars: bool,
29+
max_line_length: bool,
30+
}
31+
32+
impl WcFlags {
33+
fn parse(args: &[String]) -> Self {
34+
let mut lines = false;
35+
let mut words = false;
36+
let mut bytes = false;
37+
let mut chars = false;
38+
let mut max_line_length = false;
39+
40+
for arg in args {
41+
if !arg.starts_with('-') {
42+
continue;
43+
}
44+
match arg.as_str() {
45+
"--lines" => lines = true,
46+
"--words" => words = true,
47+
"--bytes" => bytes = true,
48+
"--chars" => chars = true,
49+
"--max-line-length" => max_line_length = true,
50+
_ if arg.starts_with('-') && !arg.starts_with("--") => {
51+
for ch in arg[1..].chars() {
52+
match ch {
53+
'l' => lines = true,
54+
'w' => words = true,
55+
'c' => bytes = true,
56+
'm' => chars = true,
57+
'L' => max_line_length = true,
58+
_ => {}
59+
}
60+
}
61+
}
62+
_ => {}
63+
}
64+
}
65+
66+
// Default: show lines, words, bytes if no flags
67+
if !lines && !words && !bytes && !chars && !max_line_length {
68+
lines = true;
69+
words = true;
70+
bytes = true;
71+
}
72+
73+
Self {
74+
lines,
75+
words,
76+
bytes,
77+
chars,
78+
max_line_length,
79+
}
80+
}
81+
}
82+
2183
#[async_trait]
2284
impl Builtin for Wc {
2385
async fn execute(&self, ctx: Context<'_>) -> Result<ExecResult> {
24-
let show_lines = ctx.args.iter().any(|a| a.contains('l'));
25-
let show_words = ctx.args.iter().any(|a| a.contains('w'));
26-
let show_bytes = ctx.args.iter().any(|a| a.contains('c'));
27-
28-
// If no flags specified, show all
29-
let (show_lines, show_words, show_bytes) = if !show_lines && !show_words && !show_bytes {
30-
(true, true, true)
31-
} else {
32-
(show_lines, show_words, show_bytes)
33-
};
86+
let flags = WcFlags::parse(ctx.args);
3487

35-
let files: Vec<_> = ctx.args.iter().filter(|a| !a.starts_with('-')).collect();
88+
let files: Vec<_> = ctx
89+
.args
90+
.iter()
91+
.filter(|a| !a.starts_with('-') || a.as_str() == "-")
92+
.collect();
3693

3794
let mut output = String::new();
3895
let mut total_lines = 0usize;
3996
let mut total_words = 0usize;
4097
let mut total_bytes = 0usize;
98+
let mut total_chars = 0usize;
99+
let mut total_max_line = 0usize;
41100

42101
if files.is_empty() {
43102
// Read from stdin
44103
if let Some(stdin) = ctx.stdin {
45-
let (lines, words, bytes) = count_text(stdin);
46-
output.push_str(&format_counts(
47-
lines, words, bytes, show_lines, show_words, show_bytes, None,
48-
));
104+
let counts = count_text(stdin);
105+
output.push_str(&format_counts(&counts, &flags, None));
49106
output.push('\n');
50107
}
51108
} else {
@@ -60,21 +117,17 @@ impl Builtin for Wc {
60117
match ctx.fs.read_file(&path).await {
61118
Ok(content) => {
62119
let text = String::from_utf8_lossy(&content);
63-
let (lines, words, bytes) = count_text(&text);
64-
65-
total_lines += lines;
66-
total_words += words;
67-
total_bytes += bytes;
68-
69-
output.push_str(&format_counts(
70-
lines,
71-
words,
72-
bytes,
73-
show_lines,
74-
show_words,
75-
show_bytes,
76-
Some(file),
77-
));
120+
let counts = count_text(&text);
121+
122+
total_lines += counts.lines;
123+
total_words += counts.words;
124+
total_bytes += counts.bytes;
125+
total_chars += counts.chars;
126+
if counts.max_line_length > total_max_line {
127+
total_max_line = counts.max_line_length;
128+
}
129+
130+
output.push_str(&format_counts(&counts, &flags, Some(file)));
78131
output.push('\n');
79132
}
80133
Err(e) => {
@@ -85,15 +138,14 @@ impl Builtin for Wc {
85138

86139
// Print total if multiple files
87140
if files.len() > 1 {
88-
output.push_str(&format_counts(
89-
total_lines,
90-
total_words,
91-
total_bytes,
92-
show_lines,
93-
show_words,
94-
show_bytes,
95-
Some(&"total".to_string()),
96-
));
141+
let totals = TextCounts {
142+
lines: total_lines,
143+
words: total_words,
144+
bytes: total_bytes,
145+
chars: total_chars,
146+
max_line_length: total_max_line,
147+
};
148+
output.push_str(&format_counts(&totals, &flags, Some(&"total".to_string())));
97149
output.push('\n');
98150
}
99151
}
@@ -102,34 +154,48 @@ impl Builtin for Wc {
102154
}
103155
}
104156

105-
/// Count lines, words, and bytes in text
106-
fn count_text(text: &str) -> (usize, usize, usize) {
157+
struct TextCounts {
158+
lines: usize,
159+
words: usize,
160+
bytes: usize,
161+
chars: usize,
162+
max_line_length: usize,
163+
}
164+
165+
/// Count lines, words, bytes, characters, and max line length in text
166+
fn count_text(text: &str) -> TextCounts {
107167
let lines = text.lines().count();
108168
let words = text.split_whitespace().count();
109169
let bytes = text.len();
110-
(lines, words, bytes)
170+
let chars = text.chars().count();
171+
let max_line_length = text.lines().map(|l| l.chars().count()).max().unwrap_or(0);
172+
TextCounts {
173+
lines,
174+
words,
175+
bytes,
176+
chars,
177+
max_line_length,
178+
}
111179
}
112180

113181
/// Format counts for output
114-
fn format_counts(
115-
lines: usize,
116-
words: usize,
117-
bytes: usize,
118-
show_lines: bool,
119-
show_words: bool,
120-
show_bytes: bool,
121-
filename: Option<&String>,
122-
) -> String {
182+
fn format_counts(counts: &TextCounts, flags: &WcFlags, filename: Option<&String>) -> String {
123183
let mut parts = Vec::new();
124184

125-
if show_lines {
126-
parts.push(format!("{:>8}", lines));
185+
if flags.lines {
186+
parts.push(format!("{:>8}", counts.lines));
187+
}
188+
if flags.words {
189+
parts.push(format!("{:>8}", counts.words));
190+
}
191+
if flags.bytes {
192+
parts.push(format!("{:>8}", counts.bytes));
127193
}
128-
if show_words {
129-
parts.push(format!("{:>8}", words));
194+
if flags.chars {
195+
parts.push(format!("{:>8}", counts.chars));
130196
}
131-
if show_bytes {
132-
parts.push(format!("{:>8}", bytes));
197+
if flags.max_line_length {
198+
parts.push(format!("{:>8}", counts.max_line_length));
133199
}
134200

135201
let mut result = parts.join("");
@@ -209,4 +275,41 @@ mod tests {
209275
assert_eq!(result.exit_code, 0);
210276
assert!(result.stdout.contains("0"));
211277
}
278+
279+
#[tokio::test]
280+
async fn test_wc_chars() {
281+
let result = run_wc(&["-m"], Some("hello")).await;
282+
assert_eq!(result.exit_code, 0);
283+
assert!(result.stdout.trim().contains("5"));
284+
}
285+
286+
#[tokio::test]
287+
async fn test_wc_chars_unicode() {
288+
// héllo: 5 chars but 6 bytes (é is 2 bytes in UTF-8)
289+
let result = run_wc(&["-m"], Some("héllo")).await;
290+
assert_eq!(result.exit_code, 0);
291+
assert!(result.stdout.trim().contains("5"));
292+
}
293+
294+
#[tokio::test]
295+
async fn test_wc_max_line_length() {
296+
let result = run_wc(&["-L"], Some("short\nlongerline\n")).await;
297+
assert_eq!(result.exit_code, 0);
298+
assert!(result.stdout.trim().contains("10"));
299+
}
300+
301+
#[tokio::test]
302+
async fn test_wc_long_flags() {
303+
let result = run_wc(&["--bytes"], Some("hello")).await;
304+
assert_eq!(result.exit_code, 0);
305+
assert!(result.stdout.trim().contains("5"));
306+
307+
let result = run_wc(&["--lines"], Some("a\nb\n")).await;
308+
assert_eq!(result.exit_code, 0);
309+
assert!(result.stdout.trim().contains("2"));
310+
311+
let result = run_wc(&["--words"], Some("one two three")).await;
312+
assert_eq!(result.exit_code, 0);
313+
assert!(result.stdout.trim().contains("3"));
314+
}
212315
}

0 commit comments

Comments
 (0)