Skip to content

Commit 59ab0f3

Browse files
CopilotPh0enixKM
Copilot
and
Ph0enixKM
authored
[Feature] Add then block for handling command exit codes (#800)
* Initial plan * Implement core then block functionality Co-authored-by: Ph0enixKM <29208124+Ph0enixKM@users.noreply.github.com> * Fix then block tests and complete implementation Co-authored-by: Ph0enixKM <29208124+Ph0enixKM@users.noreply.github.com> * Clean up imports and finalize then block implementation Co-authored-by: Ph0enixKM <29208124+Ph0enixKM@users.noreply.github.com> * Address PR feedback: simplify conditions, use VarStmtFragment, optimize empty blocks Co-authored-by: Ph0enixKM <29208124+Ph0enixKM@users.noreply.github.com> * Address all PR feedback: remove unused fields, use Type::Int, use variable function, wrap in context Co-authored-by: Ph0enixKM <29208124+Ph0enixKM@users.noreply.github.com> * Final cleanup: remove unused comments, use VarStmtFragment builder pattern, improve error handling Co-authored-by: Ph0enixKM <29208124+Ph0enixKM@users.noreply.github.com> * Remove unused set_position and set_function_name methods from Then struct Co-authored-by: Ph0enixKM <29208124+Ph0enixKM@users.noreply.github.com> * Debug: Investigating __status variable issue in trust contexts Co-authored-by: Ph0enixKM <29208124+Ph0enixKM@users.noreply.github.com> * Fix trust context regression and type consistency: use Type::Int for __status, return __status in empty blocks Co-authored-by: Ph0enixKM <29208124+Ph0enixKM@users.noreply.github.com> * WIP: Add lookahead for then( pattern to avoid ternary expression conflict - still debugging Co-authored-by: Ph0enixKM <29208124+Ph0enixKM@users.noreply.github.com> * Revert WIP lookahead commit - back to working state before ternary conflict investigation Co-authored-by: Ph0enixKM <29208124+Ph0enixKM@users.noreply.github.com> * fix: then command modifier --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: Ph0enixKM <29208124+Ph0enixKM@users.noreply.github.com> Co-authored-by: Phoenix Himself <pkaras.it@gmail.com>
1 parent d3aae6b commit 59ab0f3

File tree

15 files changed

+255
-88
lines changed

15 files changed

+255
-88
lines changed

‎src/modules/command/cmd.rs‎

Lines changed: 42 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ use crate::modules::types::{Type, Typed};
33
use crate::modules::expression::literal::bool;
44
use crate::modules::condition::failed::Failed;
55
use crate::modules::condition::succeeded::Succeeded;
6+
use crate::modules::condition::then::Then;
67
use crate::modules::expression::expr::Expr;
78
use crate::modules::expression::interpolated_region::{InterpolatedRegionType, parse_interpolated_region};
89
use super::modifier::CommandModifier;
@@ -15,7 +16,21 @@ pub struct Command {
1516
interps: Vec<Expr>,
1617
modifier: CommandModifier,
1718
failed: Failed,
18-
succeeded: Succeeded
19+
succeeded: Succeeded,
20+
then: Then
21+
}
22+
23+
impl Command {
24+
pub fn handle_multiple_failure_handlers(meta: &mut ParserMetadata, keyword: &str) -> SyntaxResult {
25+
let token = meta.get_current_token();
26+
if let Ok(word) = token_by(meta, |word| ["failed", "succeeded", "then"].contains(&word.as_str())) {
27+
return error!(meta, token => {
28+
message: format!("Cannot use both '{keyword}' and '{}' blocks for the same command", word),
29+
comment: "Use either '{keyword}' to handle both success and failure, 'failed' or 'succeeded' blocks, but not both"
30+
});
31+
}
32+
Ok(())
33+
}
1934
}
2035

2136
impl Typed for Command {
@@ -33,7 +48,8 @@ impl SyntaxModule<ParserMetadata> for Command {
3348
interps: vec![],
3449
modifier: CommandModifier::new().parse_expr(),
3550
failed: Failed::new(),
36-
succeeded: Succeeded::new()
51+
succeeded: Succeeded::new(),
52+
then: Then::new()
3753
}
3854
}
3955

@@ -42,35 +58,33 @@ impl SyntaxModule<ParserMetadata> for Command {
4258
self.modifier.use_modifiers(meta, |_this, meta| {
4359
let tok = meta.get_current_token();
4460
(self.strings, self.interps) = parse_interpolated_region(meta, &InterpolatedRegionType::Command)?;
45-
46-
// Set position for both failed and succeeded handlers
61+
62+
// Set position for failed and succeeded handlers
4763
let position = PositionInfo::from_between_tokens(meta, tok.clone(), meta.get_current_token());
4864
self.failed.set_position(position.clone());
49-
self.succeeded.set_position(position);
50-
51-
// Try to parse succeeded block first
52-
syntax(meta, &mut self.succeeded)?;
53-
54-
// If succeeded block was parsed successfully, check for conflicts with failed
55-
if self.succeeded.is_parsed {
56-
// Check if there's an attempt to use failed block as well
57-
if token(meta, "failed").is_ok() {
58-
return error!(meta, meta.get_current_token() => {
59-
message: "Cannot use both 'succeeded' and 'failed' blocks for the same command",
60-
comment: "Use either 'succeeded' or 'failed' block, but not both"
61-
});
62-
}
63-
return Ok(());
65+
66+
// Try to parse then block first
67+
match syntax(meta, &mut self.then) {
68+
Ok(_) => return Command::handle_multiple_failure_handlers(meta, "then"),
69+
err @ Err(Failure::Loud(_)) => return err,
70+
_ => {}
71+
}
72+
73+
// Try to parse succeeded block
74+
match syntax(meta, &mut self.succeeded) {
75+
Ok(_) => return Command::handle_multiple_failure_handlers(meta, "succeeded"),
76+
err @ Err(Failure::Loud(_)) => return err,
77+
_ => {}
6478
}
6579

6680
// If no succeeded block, try to parse failed block
6781
match syntax(meta, &mut self.failed) {
68-
Ok(_) => Ok(()),
82+
Ok(_) => Self::handle_multiple_failure_handlers(meta, "failed"),
6983
Err(Failure::Quiet(_)) => {
70-
// Neither succeeded nor failed block found
84+
// Neither succeeded, failed, nor then block found
7185
error!(meta, tok => {
72-
message: "Every command statement must handle execution result",
73-
comment: "You can use '?' to propagate failure, 'failed' block to handle failure, 'succeeded' block to handle success, or 'trust' modifier to ignore results"
86+
message: "Every command statement must handle execution result",
87+
comment: "You can use '?' to propagate failure, 'failed' block to handle failure, 'succeeded' block to handle success, 'then' block to handle both, or 'trust' modifier to ignore results"
7488
})
7589
},
7690
Err(err) => Err(err)
@@ -87,6 +101,7 @@ impl Command {
87101
.collect::<Vec<FragmentKind>>();
88102
let failed = self.failed.translate(meta);
89103
let succeeded = self.succeeded.translate(meta);
104+
let then = self.then.translate(meta);
90105

91106
let mut is_silent = self.modifier.is_silent || meta.silenced;
92107
let mut is_sudo = self.modifier.is_sudo || meta.sudoed;
@@ -107,8 +122,10 @@ impl Command {
107122
swap(&mut is_silent, &mut meta.silenced);
108123
swap(&mut is_sudo, &mut meta.sudoed);
109124

110-
// Choose between failed, succeeded, or no handler
111-
let handler = if self.failed.is_parsed {
125+
// Choose between failed, succeeded, then, or no handler
126+
let handler = if self.then.is_parsed {
127+
then
128+
} else if self.failed.is_parsed {
112129
failed
113130
} else if self.succeeded.is_parsed {
114131
succeeded

‎src/modules/condition/failed.rs‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -64,13 +64,13 @@ impl SyntaxModule<ParserMetadata> for Failed {
6464
} else {
6565
match (self.function_name.clone(), self.error_position.clone()) {
6666
(Some(fun_name), Some(pos)) => {
67-
return error_pos!(meta, pos, format!("Failed function call '{fun_name}' must be followed by a 'failed' block or statement"))
67+
return error_pos!(meta, pos, format!("Failed function call '{fun_name}' must be followed by a 'then', 'succeeded' or 'failed' block or statement"))
6868
}
6969
(None, Some(pos)) => {
70-
return error_pos!(meta, pos, format!("Failed command must be followed by a 'failed' block or statement"))
70+
return error_pos!(meta, pos, format!("Failed command must be followed by a 'failed', 'succeeded' or 'failed' block or statement"))
7171
}
7272
_ => {
73-
return error!(meta, tok, format!("Failed expression must be followed by a 'failed' block or statement"))
73+
return error!(meta, tok, format!("Failed expression must be followed by a 'failed', 'succeeded' or 'failed' block or statement"))
7474
}
7575
}
7676
}
@@ -87,7 +87,7 @@ impl TranslateModule for Failed {
8787
if self.is_parsed {
8888
let block = self.block.translate(meta);
8989
// the condition of '$?' clears the status code thus we need to store it in a variable
90-
let status_variable_stmt = VarStmtFragment::new("__status", Type::Num, fragments!("$?"));
90+
let status_variable_stmt = VarStmtFragment::new("__status", Type::Int, fragments!("$?"));
9191
let status_variable_expr = VarExprFragment::from_stmt(&status_variable_stmt);
9292
if self.is_question_mark {
9393
// Set default return value if failure happened in a function

‎src/modules/condition/mod.rs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,3 +2,4 @@ pub mod ifcond;
22
pub mod ifchain;
33
pub mod failed;
44
pub mod succeeded;
5+
pub mod then;

‎src/modules/condition/succeeded.rs‎

Lines changed: 7 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -7,29 +7,15 @@ use crate::modules::types::Type;
77
#[derive(Debug, Clone)]
88
pub struct Succeeded {
99
pub is_parsed: bool,
10-
error_position: Option<PositionInfo>,
11-
function_name: Option<String>,
1210
block: Box<Block>
1311
}
1412

15-
impl Succeeded {
16-
pub fn set_position(&mut self, position: PositionInfo) {
17-
self.error_position = Some(position);
18-
}
19-
20-
pub fn set_function_name(&mut self, name: String) {
21-
self.function_name = Some(name);
22-
}
23-
}
24-
2513
impl SyntaxModule<ParserMetadata> for Succeeded {
2614
syntax_name!("Succeeded Expression");
2715

2816
fn new() -> Self {
2917
Succeeded {
3018
is_parsed: false,
31-
function_name: None,
32-
error_position: None,
3319
block: Box::new(Block::new().with_needs_noop().with_condition())
3420
}
3521
}
@@ -48,13 +34,14 @@ impl SyntaxModule<ParserMetadata> for Succeeded {
4834
self.is_parsed = true;
4935
Ok(())
5036
},
51-
Err(_) => {
37+
Err(err) => {
5238
// If we're in a trust context, mark as parsed
5339
if meta.context.is_trust_ctx {
5440
self.is_parsed = true;
41+
Ok(())
42+
} else {
43+
Err(err)
5544
}
56-
// Otherwise, return quietly (no succeeded block found)
57-
Ok(())
5845
}
5946
}
6047
}
@@ -65,9 +52,9 @@ impl TranslateModule for Succeeded {
6552
if self.is_parsed {
6653
let block = self.block.translate(meta);
6754
// the condition of '$?' clears the status code thus we need to store it in a variable
68-
let status_variable_stmt = VarStmtFragment::new("__status", Type::Num, fragments!("$?"));
55+
let status_variable_stmt = VarStmtFragment::new("__status", Type::Int, fragments!("$?"));
6956
let status_variable_expr = VarExprFragment::from_stmt(&status_variable_stmt);
70-
57+
7158
match &block {
7259
FragmentKind::Empty => {
7360
status_variable_stmt.to_frag()
@@ -88,4 +75,4 @@ impl TranslateModule for Succeeded {
8875
FragmentKind::Empty
8976
}
9077
}
91-
}
78+
}

‎src/modules/condition/then.rs‎

Lines changed: 107 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,107 @@
1+
use heraclitus_compiler::prelude::*;
2+
use crate::fragments;
3+
use crate::modules::prelude::*;
4+
use crate::modules::block::Block;
5+
use crate::modules::types::Type;
6+
use crate::modules::variable::variable_name_extensions;
7+
8+
#[derive(Debug, Clone)]
9+
pub struct Then {
10+
pub is_parsed: bool,
11+
block: Box<Block>,
12+
param_name: String,
13+
param_global_id: Option<usize>
14+
}
15+
16+
impl SyntaxModule<ParserMetadata> for Then {
17+
syntax_name!("Then Expression");
18+
19+
fn new() -> Self {
20+
Then {
21+
is_parsed: false,
22+
block: Box::new(Block::new().with_needs_noop().with_condition()),
23+
param_name: String::new(),
24+
param_global_id: None
25+
}
26+
}
27+
28+
fn parse(&mut self, meta: &mut ParserMetadata) -> SyntaxResult {
29+
match token(meta, "then") {
30+
Ok(_) => {
31+
context!({
32+
// Parse the parameter in parentheses
33+
token(meta, "(")?;
34+
let param_tok = meta.get_current_token();
35+
36+
// Check if we immediately hit a closing paren (empty parameter)
37+
if token(meta, ")").is_ok() {
38+
let pos = PositionInfo::from_between_tokens(meta, param_tok, meta.get_current_token());
39+
return error_pos!(meta, pos, "Parameter name cannot be empty");
40+
}
41+
42+
self.param_name = variable(meta, variable_name_extensions())?;
43+
token(meta, ")")?;
44+
45+
// Add the parameter variable to the scope and parse the block
46+
meta.with_push_scope(|meta| {
47+
self.param_global_id = meta.add_var(&self.param_name, Type::Int, false);
48+
syntax(meta, &mut *self.block)?;
49+
Ok(())
50+
})?;
51+
52+
if self.block.is_empty() {
53+
let message = Message::new_warn_at_token(meta, meta.get_current_token())
54+
.message("Empty then block")
55+
.comment("You should use 'trust' modifier to run commands without handling errors");
56+
meta.add_message(message);
57+
}
58+
self.is_parsed = true;
59+
Ok(())
60+
}, |pos| {
61+
error_pos!(meta, pos, "Failed to parse then block")
62+
})
63+
},
64+
Err(err) => {
65+
// If we're in a trust context, mark as parsed
66+
if meta.context.is_trust_ctx {
67+
self.is_parsed = true;
68+
Ok(())
69+
} else {
70+
Err(err)
71+
}
72+
}
73+
}
74+
}
75+
}
76+
77+
impl TranslateModule for Then {
78+
fn translate(&self, meta: &mut TranslateMetadata) -> FragmentKind {
79+
if self.is_parsed {
80+
let block = self.block.translate(meta);
81+
// the condition of '$?' clears the status code thus we need to store it in a variable
82+
let status_variable_stmt = VarStmtFragment::new("__status", Type::Int, fragments!("$?"));
83+
let status_variable_expr = VarExprFragment::from_stmt(&status_variable_stmt);
84+
85+
match &block {
86+
FragmentKind::Empty => {
87+
status_variable_stmt.to_frag()
88+
},
89+
FragmentKind::Block(block) if block.statements.is_empty() => {
90+
status_variable_stmt.to_frag()
91+
},
92+
_ => {
93+
let param_assignment = VarStmtFragment::new(&self.param_name, Type::Int, status_variable_expr.to_frag())
94+
.with_global_id(self.param_global_id);
95+
96+
BlockFragment::new(vec![
97+
status_variable_stmt.to_frag(),
98+
param_assignment.to_frag(),
99+
block,
100+
], false).to_frag()
101+
}
102+
}
103+
} else {
104+
FragmentKind::Empty
105+
}
106+
}
107+
}

0 commit comments

Comments
 (0)