Skip to content

Commit 1e52190

Browse files
lalvareztlalvarezt
committed
refactor(parser): shuffle some things around to make the code more expressive and hopefully maintainable
1 parent 521a006 commit 1e52190

File tree

2 files changed

+159
-152
lines changed

2 files changed

+159
-152
lines changed

‎src/pipeline/mod.rs‎

Lines changed: 41 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -281,6 +281,34 @@ struct DebugContext {
281281
total_items: usize,
282282
}
283283

284+
fn apply_string_operation<F>(val: Value, transform: F, op_name: &str) -> Result<Value, String>
285+
where
286+
F: FnOnce(String) -> String,
287+
{
288+
if let Value::Str(s) = val {
289+
Ok(Value::Str(transform(s)))
290+
} else {
291+
Err(format!(
292+
"{} operation can only be applied to strings. Use map to apply to lists.",
293+
op_name
294+
))
295+
}
296+
}
297+
298+
fn apply_list_operation<F>(val: Value, transform: F, op_name: &str) -> Result<Value, String>
299+
where
300+
F: FnOnce(Vec<String>) -> Vec<String>,
301+
{
302+
if let Value::List(list) = val {
303+
Ok(Value::List(transform(list)))
304+
} else {
305+
Err(format!(
306+
"{} operation can only be applied to lists",
307+
op_name
308+
))
309+
}
310+
}
311+
284312
fn apply_single_operation(
285313
op: &StringOp,
286314
val: Value,
@@ -308,11 +336,7 @@ fn apply_single_operation(
308336
Ok(result)
309337
}
310338
StringOp::Slice { range } => {
311-
if let Value::List(list) = val {
312-
Ok(Value::List(apply_range(&list, range)))
313-
} else {
314-
Err("Slice operation can only be applied to lists".to_string())
315-
}
339+
apply_list_operation(val, |list| apply_range(&list, range), "Slice")
316340
}
317341
StringOp::Filter { pattern } => {
318342
let re = Regex::new(pattern).map_err(|e| format!("Invalid regex: {}", e))?;
@@ -353,18 +377,16 @@ fn apply_single_operation(
353377
Ok(Value::List(list))
354378
}
355379
},
356-
StringOp::Unique => {
357-
if let Value::List(list) = val {
380+
StringOp::Unique => apply_list_operation(
381+
val,
382+
|list| {
358383
let mut seen = std::collections::HashSet::new();
359-
let unique = list
360-
.into_iter()
384+
list.into_iter()
361385
.filter(|item| seen.insert(item.clone()))
362-
.collect();
363-
Ok(Value::List(unique))
364-
} else {
365-
Err("Unique operation can only be applied to lists".to_string())
366-
}
367-
}
386+
.collect()
387+
},
388+
"Unique",
389+
),
368390
StringOp::Substring { range } => {
369391
if let Value::Str(s) = val {
370392
let chars: Vec<char> = s.chars().collect();
@@ -405,26 +427,8 @@ fn apply_single_operation(
405427
)
406428
}
407429
}
408-
StringOp::Upper => {
409-
if let Value::Str(s) = val {
410-
Ok(Value::Str(s.to_uppercase()))
411-
} else {
412-
Err(
413-
"Upper operation can only be applied to strings. Use map to apply to lists."
414-
.to_string(),
415-
)
416-
}
417-
}
418-
StringOp::Lower => {
419-
if let Value::Str(s) = val {
420-
Ok(Value::Str(s.to_lowercase()))
421-
} else {
422-
Err(
423-
"Lower operation can only be applied to strings. Use map to apply to lists."
424-
.to_string(),
425-
)
426-
}
427-
}
430+
StringOp::Upper => apply_string_operation(val, |s| s.to_uppercase(), "Upper"),
431+
StringOp::Lower => apply_string_operation(val, |s| s.to_lowercase(), "Lower"),
428432
StringOp::Trim { direction } => {
429433
if let Value::Str(s) = val {
430434
let result = match direction {
@@ -458,24 +462,10 @@ fn apply_single_operation(
458462
}
459463
}
460464
StringOp::Append { suffix } => {
461-
if let Value::Str(s) = val {
462-
Ok(Value::Str(format!("{}{}", s, suffix)))
463-
} else {
464-
Err(
465-
"Append operation can only be applied to strings. Use map to apply to lists."
466-
.to_string(),
467-
)
468-
}
465+
apply_string_operation(val, |s| format!("{}{}", s, suffix), "Append")
469466
}
470467
StringOp::Prepend { prefix } => {
471-
if let Value::Str(s) = val {
472-
Ok(Value::Str(format!("{}{}", prefix, s)))
473-
} else {
474-
Err(
475-
"Prepend operation can only be applied to strings. Use map to apply to lists."
476-
.to_string(),
477-
)
478-
}
468+
apply_string_operation(val, |s| format!("{}{}", prefix, s), "Prepend")
479469
}
480470
StringOp::StripAnsi => {
481471
if let Value::Str(s) = val {

‎src/pipeline/parser.rs‎

Lines changed: 118 additions & 101 deletions
Original file line numberDiff line numberDiff line change
@@ -58,14 +58,12 @@ fn parse_operation(pair: pest::iterators::Pair<Rule>) -> Result<StringOp, String
5858
.map_or_else(|| Ok(RangeSpec::Range(None, None, false)), parse_range_spec)?;
5959
Ok(StringOp::Split { sep, range })
6060
}
61-
Rule::join => {
62-
let sep = process_arg(pair.into_inner().next().unwrap().as_str());
63-
Ok(StringOp::Join { sep })
64-
}
65-
Rule::substring => {
66-
let range = parse_range_spec(pair.into_inner().next().unwrap())?;
67-
Ok(StringOp::Substring { range })
68-
}
61+
Rule::join => Ok(StringOp::Join {
62+
sep: extract_single_arg(pair)?,
63+
}),
64+
Rule::substring => Ok(StringOp::Substring {
65+
range: extract_range_arg(pair)?,
66+
}),
6967
Rule::replace => {
7068
let sed_parts = parse_sed_string(pair.into_inner().next().unwrap())?;
7169
Ok(StringOp::Replace {
@@ -76,105 +74,124 @@ fn parse_operation(pair: pest::iterators::Pair<Rule>) -> Result<StringOp, String
7674
}
7775
Rule::upper => Ok(StringOp::Upper),
7876
Rule::lower => Ok(StringOp::Lower),
79-
Rule::trim => {
80-
let direction = pair
81-
.into_inner()
82-
.next()
83-
.map(|p| match p.as_str() {
84-
"left" => TrimDirection::Left,
85-
"right" => TrimDirection::Right,
86-
"both" => TrimDirection::Both,
87-
_ => TrimDirection::Both,
88-
})
89-
.unwrap_or(TrimDirection::Both);
90-
Ok(StringOp::Trim { direction })
91-
}
92-
Rule::strip => {
93-
let chars = pair.into_inner().next().unwrap().as_str().to_string();
94-
Ok(StringOp::Strip { chars })
95-
}
96-
Rule::append => {
97-
let suffix = process_arg(pair.into_inner().next().unwrap().as_str());
98-
Ok(StringOp::Append { suffix })
99-
}
100-
Rule::prepend => {
101-
let prefix = process_arg(pair.into_inner().next().unwrap().as_str());
102-
Ok(StringOp::Prepend { prefix })
103-
}
77+
Rule::trim => Ok(StringOp::Trim {
78+
direction: parse_trim_direction(pair),
79+
}),
80+
Rule::strip => Ok(StringOp::Strip {
81+
chars: extract_single_arg_raw(pair)?,
82+
}),
83+
Rule::append => Ok(StringOp::Append {
84+
suffix: extract_single_arg(pair)?,
85+
}),
86+
Rule::prepend => Ok(StringOp::Prepend {
87+
prefix: extract_single_arg(pair)?,
88+
}),
10489
Rule::strip_ansi => Ok(StringOp::StripAnsi),
105-
Rule::filter => {
106-
let pattern = pair.into_inner().next().unwrap().as_str().to_string();
107-
Ok(StringOp::Filter { pattern })
108-
}
109-
Rule::filter_not => {
110-
let pattern = pair.into_inner().next().unwrap().as_str().to_string();
111-
Ok(StringOp::FilterNot { pattern })
112-
}
113-
Rule::slice => {
114-
let range = parse_range_spec(pair.into_inner().next().unwrap())?;
115-
Ok(StringOp::Slice { range })
116-
}
117-
Rule::sort => {
118-
let direction = pair
119-
.into_inner()
120-
.next()
121-
.map(|p| match p.as_str() {
122-
"desc" => SortDirection::Desc,
123-
"asc" => SortDirection::Asc,
124-
_ => SortDirection::Asc,
125-
})
126-
.unwrap_or(SortDirection::Asc);
127-
Ok(StringOp::Sort { direction })
128-
}
90+
Rule::filter => Ok(StringOp::Filter {
91+
pattern: extract_single_arg_raw(pair)?,
92+
}),
93+
Rule::filter_not => Ok(StringOp::FilterNot {
94+
pattern: extract_single_arg_raw(pair)?,
95+
}),
96+
Rule::slice => Ok(StringOp::Slice {
97+
range: extract_range_arg(pair)?,
98+
}),
99+
Rule::sort => Ok(StringOp::Sort {
100+
direction: parse_sort_direction(pair),
101+
}),
129102
Rule::reverse => Ok(StringOp::Reverse),
130103
Rule::unique => Ok(StringOp::Unique),
131-
Rule::pad => {
132-
let mut parts = pair.into_inner();
133-
let width = parts
134-
.next()
135-
.unwrap()
136-
.as_str()
137-
.parse()
138-
.map_err(|_| "Invalid padding width")?;
139-
let char = parts
140-
.next()
141-
.map(|p| process_arg(p.as_str()).chars().next().unwrap_or(' '))
142-
.unwrap_or(' ');
143-
let direction = parts
144-
.next()
145-
.map(|p| match p.as_str() {
146-
"left" => PadDirection::Left,
147-
"right" => PadDirection::Right,
148-
"both" => PadDirection::Both,
149-
_ => PadDirection::Right,
150-
})
151-
.unwrap_or(PadDirection::Right);
152-
Ok(StringOp::Pad {
153-
width,
154-
char,
155-
direction,
156-
})
157-
}
158-
Rule::regex_extract | Rule::map_regex_extract => {
159-
let mut parts = pair.into_inner();
160-
let pattern = parts.next().unwrap().as_str().to_string();
161-
let group = parts.next().and_then(|p| p.as_str().parse().ok());
162-
Ok(StringOp::RegexExtract { pattern, group })
163-
}
164-
Rule::map => {
165-
let map_op_pair = pair.into_inner().next().unwrap();
166-
let operation_list_pair = map_op_pair.into_inner().next().unwrap();
104+
Rule::pad => parse_pad_operation(pair),
105+
Rule::regex_extract | Rule::map_regex_extract => parse_regex_extract_operation(pair),
106+
Rule::map => parse_map_operation(pair),
107+
_ => Err(format!("Unsupported operation: {:?}", pair.as_rule())),
108+
}
109+
}
167110

168-
let mut operations = Vec::new();
169-
for op_pair in operation_list_pair.into_inner() {
170-
let inner_op_pair = op_pair.into_inner().next().unwrap();
171-
operations.push(parse_operation(inner_op_pair)?);
172-
}
111+
// Helper functions to reduce repetition
173112

174-
Ok(StringOp::Map { operations })
175-
}
176-
_ => Err(format!("Unsupported operation: {:?}", pair.as_rule())),
113+
fn extract_single_arg(pair: pest::iterators::Pair<Rule>) -> Result<String, String> {
114+
Ok(process_arg(pair.into_inner().next().unwrap().as_str()))
115+
}
116+
117+
fn extract_single_arg_raw(pair: pest::iterators::Pair<Rule>) -> Result<String, String> {
118+
Ok(pair.into_inner().next().unwrap().as_str().to_string())
119+
}
120+
121+
fn extract_range_arg(pair: pest::iterators::Pair<Rule>) -> Result<RangeSpec, String> {
122+
parse_range_spec(pair.into_inner().next().unwrap())
123+
}
124+
125+
fn parse_trim_direction(pair: pest::iterators::Pair<Rule>) -> TrimDirection {
126+
pair.into_inner()
127+
.next()
128+
.map(|p| match p.as_str() {
129+
"left" => TrimDirection::Left,
130+
"right" => TrimDirection::Right,
131+
"both" => TrimDirection::Both,
132+
_ => TrimDirection::Both,
133+
})
134+
.unwrap_or(TrimDirection::Both)
135+
}
136+
137+
fn parse_sort_direction(pair: pest::iterators::Pair<Rule>) -> SortDirection {
138+
pair.into_inner()
139+
.next()
140+
.map(|p| match p.as_str() {
141+
"desc" => SortDirection::Desc,
142+
"asc" => SortDirection::Asc,
143+
_ => SortDirection::Asc,
144+
})
145+
.unwrap_or(SortDirection::Asc)
146+
}
147+
148+
fn parse_pad_operation(pair: pest::iterators::Pair<Rule>) -> Result<StringOp, String> {
149+
let mut parts = pair.into_inner();
150+
let width = parts
151+
.next()
152+
.unwrap()
153+
.as_str()
154+
.parse()
155+
.map_err(|_| "Invalid padding width")?;
156+
let char = parts
157+
.next()
158+
.map(|p| process_arg(p.as_str()).chars().next().unwrap_or(' '))
159+
.unwrap_or(' ');
160+
let direction = parts
161+
.next()
162+
.map(|p| match p.as_str() {
163+
"left" => PadDirection::Left,
164+
"right" => PadDirection::Right,
165+
"both" => PadDirection::Both,
166+
_ => PadDirection::Right,
167+
})
168+
.unwrap_or(PadDirection::Right);
169+
170+
Ok(StringOp::Pad {
171+
width,
172+
char,
173+
direction,
174+
})
175+
}
176+
177+
fn parse_regex_extract_operation(pair: pest::iterators::Pair<Rule>) -> Result<StringOp, String> {
178+
let mut parts = pair.into_inner();
179+
let pattern = parts.next().unwrap().as_str().to_string();
180+
let group = parts.next().and_then(|p| p.as_str().parse().ok());
181+
Ok(StringOp::RegexExtract { pattern, group })
182+
}
183+
184+
fn parse_map_operation(pair: pest::iterators::Pair<Rule>) -> Result<StringOp, String> {
185+
let map_op_pair = pair.into_inner().next().unwrap();
186+
let operation_list_pair = map_op_pair.into_inner().next().unwrap();
187+
188+
let mut operations = Vec::new();
189+
for op_pair in operation_list_pair.into_inner() {
190+
let inner_op_pair = op_pair.into_inner().next().unwrap();
191+
operations.push(parse_operation(inner_op_pair)?);
177192
}
193+
194+
Ok(StringOp::Map { operations })
178195
}
179196

180197
fn process_arg(s: &str) -> String {

0 commit comments

Comments
 (0)