Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
104 changes: 66 additions & 38 deletions codex-rs/core/src/bash.rs
Original file line number Diff line number Diff line change
Expand Up @@ -138,26 +138,12 @@ fn parse_plain_command_from_node(cmd: tree_sitter::Node, src: &str) -> Option<Ve
words.push(child.utf8_text(src.as_bytes()).ok()?.to_owned());
}
"string" => {
if child.child_count() == 3
&& child.child(0)?.kind() == "\""
&& child.child(1)?.kind() == "string_content"
&& child.child(2)?.kind() == "\""
{
words.push(child.child(1)?.utf8_text(src.as_bytes()).ok()?.to_owned());
} else {
return None;
}
let parsed = parse_double_quoted_string(child, src)?;
words.push(parsed);
}
"raw_string" => {
let raw_string = child.utf8_text(src.as_bytes()).ok()?;
let stripped = raw_string
.strip_prefix('\'')
.and_then(|s| s.strip_suffix('\''));
if let Some(s) = stripped {
words.push(s.to_owned());
} else {
return None;
}
let parsed = parse_raw_string(child, src)?;
words.push(parsed);
}
"concatenation" => {
// Handle concatenated arguments like -g"*.py"
Expand All @@ -170,28 +156,12 @@ fn parse_plain_command_from_node(cmd: tree_sitter::Node, src: &str) -> Option<Ve
.push_str(part.utf8_text(src.as_bytes()).ok()?.to_owned().as_str());
}
"string" => {
if part.child_count() == 3
&& part.child(0)?.kind() == "\""
&& part.child(1)?.kind() == "string_content"
&& part.child(2)?.kind() == "\""
{
concatenated.push_str(
part.child(1)?
.utf8_text(src.as_bytes())
.ok()?
.to_owned()
.as_str(),
);
} else {
return None;
}
let parsed = parse_double_quoted_string(part, src)?;
concatenated.push_str(&parsed);
}
"raw_string" => {
let raw_string = part.utf8_text(src.as_bytes()).ok()?;
let stripped = raw_string
.strip_prefix('\'')
.and_then(|s| s.strip_suffix('\''))?;
concatenated.push_str(stripped);
let parsed = parse_raw_string(part, src)?;
concatenated.push_str(&parsed);
}
_ => return None,
}
Expand All @@ -207,9 +177,38 @@ fn parse_plain_command_from_node(cmd: tree_sitter::Node, src: &str) -> Option<Ve
Some(words)
}

fn parse_double_quoted_string(node: Node, src: &str) -> Option<String> {
if node.kind() != "string" {
return None;
}
let mut cursor = node.walk();
for part in node.named_children(&mut cursor) {
if part.kind() != "string_content" {
return None;
}
}
let raw = node.utf8_text(src.as_bytes()).ok()?;
let stripped = raw
.strip_prefix('"')
.and_then(|text| text.strip_suffix('"'))?;
Some(stripped.to_string())
}

fn parse_raw_string(node: Node, src: &str) -> Option<String> {
if node.kind() != "raw_string" {
return None;
}
let raw_string = node.utf8_text(src.as_bytes()).ok()?;
let stripped = raw_string
.strip_prefix('\'')
.and_then(|s| s.strip_suffix('\''));
stripped.map(str::to_owned)
}

#[cfg(test)]
mod tests {
use super::*;
use pretty_assertions::assert_eq;

fn parse_seq(src: &str) -> Option<Vec<Vec<String>>> {
let tree = try_parse_shell(src)?;
Expand Down Expand Up @@ -250,6 +249,35 @@ mod tests {
);
}

#[test]
fn accepts_double_quoted_strings_with_newlines() {
let cmds = parse_seq("git commit -m \"line1\nline2\"").unwrap();
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

parse_double_quoted_string() could use more tests. I would like to verify that things like env vars embedded in double quoted strings are rejected.

I would also like to see if this covers examples like:

"/usr"'/'"local"/bin

where mixed quote types are used.

assert_eq!(
cmds,
vec![vec![
"git".to_string(),
"commit".to_string(),
"-m".to_string(),
"line1\nline2".to_string(),
]]
);
}

#[test]
fn accepts_mixed_quote_concatenation() {
let cmds = parse_seq("echo \"/usr\"'/'\"local\"/bin").unwrap();
assert_eq!(
cmds,
vec![vec!["echo".to_string(), "/usr/local/bin".to_string()]]
);
}

#[test]
fn rejects_double_quoted_strings_with_expansions() {
assert!(parse_seq("echo \"hi ${USER}\"").is_none());
assert!(parse_seq("echo \"$HOME\"").is_none());
}

#[test]
fn accepts_numbers_as_words() {
let cmds = parse_seq("echo 123 456").unwrap();
Expand Down
6 changes: 5 additions & 1 deletion codex-rs/protocol/src/num_format.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,10 @@ pub fn format_with_separators(n: i64) -> String {
formatter().format(&Decimal::from(n)).to_string()
}

fn format_with_separators_with_formatter(n: i64, formatter: &DecimalFormatter) -> String {
formatter.format(&Decimal::from(n)).to_string()
}

fn format_si_suffix_with_formatter(n: i64, formatter: &DecimalFormatter) -> String {
let n = n.max(0);
if n < 1000 {
Expand Down Expand Up @@ -58,7 +62,7 @@ fn format_si_suffix_with_formatter(n: i64, formatter: &DecimalFormatter) -> Stri
// Above 1000G, keep whole‑G precision.
format!(
"{}G",
format_with_separators(((n as f64) / 1e9).round() as i64)
format_with_separators_with_formatter(((n as f64) / 1e9).round() as i64, formatter)
)
}

Expand Down
Loading