Skip to content

Commit 06601a5

Browse files
rizsottorizsotto
committed
small improvements on semantic formatting
1 parent 0023f8f commit 06601a5

File tree

2 files changed

+75
-55
lines changed

2 files changed

+75
-55
lines changed

‎bear/src/config.rs‎

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,6 @@
6565
//! paths:
6666
//! directory: canonical
6767
//! file: canonical
68-
//! output: canonical
6968
//! entry:
7069
//! command_as_array: true
7170
//! keep_output_field: true
@@ -348,8 +347,6 @@ mod types {
348347
pub directory: PathResolver,
349348
#[serde(default)]
350349
pub file: PathResolver,
351-
#[serde(default)]
352-
pub output: PathResolver,
353350
}
354351

355352
#[derive(Clone, Debug, Default, PartialEq, Deserialize, Serialize)]
@@ -593,7 +590,6 @@ pub mod loader {
593590
paths:
594591
directory: canonical
595592
file: canonical
596-
output: canonical
597593
"#;
598594

599595
let result = Loader::from_reader(content).unwrap();
@@ -670,7 +666,6 @@ pub mod loader {
670666
paths: PathFormat {
671667
directory: PathResolver::Canonical,
672668
file: PathResolver::Canonical,
673-
output: PathResolver::Canonical,
674669
},
675670
entry: EntryFormat {
676671
command_field_as_array: true,
@@ -728,7 +723,6 @@ pub mod loader {
728723
paths: PathFormat {
729724
directory: PathResolver::Canonical,
730725
file: PathResolver::Canonical,
731-
output: PathResolver::Canonical,
732726
},
733727
entry: EntryFormat {
734728
command_field_as_array: true,
@@ -792,7 +786,6 @@ pub mod loader {
792786
paths:
793787
directory: relative
794788
file: relative
795-
output: relative
796789
entry:
797790
command_field_as_array: false
798791
keep_output_field: false
@@ -838,7 +831,6 @@ pub mod loader {
838831
paths: PathFormat {
839832
directory: PathResolver::Relative,
840833
file: PathResolver::Relative,
841-
output: PathResolver::Relative,
842834
},
843835
entry: EntryFormat {
844836
command_field_as_array: false,

‎bear/src/semantic/interpreters/format.rs‎

Lines changed: 75 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,9 @@
1818
//! according to configuration (absolute, relative, canonical).
1919
2020
use crate::config::{PathFormat, PathResolver};
21-
use crate::semantic::{ArgumentKind, Command, CompilerCommand, Execution, Interpreter};
21+
use crate::semantic::{
22+
ArgumentGroup, ArgumentKind, Command, CompilerCommand, Execution, Interpreter,
23+
};
2224
use std::path::{Path, PathBuf};
2325
use std::{env, io};
2426
use thiserror::Error;
@@ -48,6 +50,10 @@ impl<T: Interpreter> FormattingInterpreter<T> {
4850
}
4951

5052
impl<T: Interpreter> Interpreter for FormattingInterpreter<T> {
53+
/// This function formats the recognized command if a formatter is configured.
54+
///
55+
/// Implemented as an Interpreter trait method, because it wraps another interpreter
56+
/// and applies formatting to the result semantic.
5157
fn recognize(&self, execution: &Execution) -> Option<Command> {
5258
// First, let the inner interpreter recognize the command
5359
let command = self.inner.recognize(execution)?;
@@ -73,61 +79,84 @@ impl<T: Interpreter> Interpreter for FormattingInterpreter<T> {
7379
}
7480
}
7581

82+
/// Represents the path formatting configuration.
7683
pub(super) struct PathFormatter {
7784
config: PathFormat,
7885
current_dir: PathBuf,
7986
}
8087

8188
impl PathFormatter {
82-
fn format_command(&self, mut cmd: CompilerCommand) -> Result<CompilerCommand, FormatError> {
89+
/// Formats the compiler command according to the configuration.
90+
fn format_command(&self, cmd: CompilerCommand) -> Result<CompilerCommand, FormatError> {
91+
// Make sure the working directory is absolute, so we can resolve paths correctly,
92+
// independently how the path format is requested.
93+
let canonic_working_dir = cmd.working_dir.canonicalize()?;
94+
8395
// Format the working directory
84-
let working_dir = cmd.working_dir.canonicalize()?;
85-
cmd.working_dir = self
86-
.config
87-
.directory
88-
.resolve(&self.current_dir, &working_dir)?;
96+
let working_dir = self.format_working_dir(&canonic_working_dir)?;
8997

9098
// Format paths in arguments
91-
for arg_group in &mut cmd.arguments {
92-
match arg_group.kind {
93-
ArgumentKind::Source => {
94-
// Format source file paths
95-
for arg in &mut arg_group.args {
96-
if let Ok(formatted) =
97-
Self::format_path(arg, &working_dir, &self.config.file)
98-
{
99-
*arg = formatted;
100-
}
101-
}
102-
}
103-
ArgumentKind::Output => {
104-
// Format output file paths
105-
// For output arguments, we need to handle "-o filename" pairs
106-
if arg_group.args.len() >= 2 && arg_group.args[0] == "-o" {
107-
if let Ok(formatted) =
108-
Self::format_path(&arg_group.args[1], &working_dir, &self.config.output)
109-
{
110-
arg_group.args[1] = formatted;
111-
}
112-
}
99+
let arguments = cmd
100+
.arguments
101+
.iter()
102+
.flat_map(|argument| self.format_argument(argument, &canonic_working_dir))
103+
.collect();
104+
105+
Ok(CompilerCommand {
106+
executable: cmd.executable,
107+
working_dir,
108+
arguments,
109+
})
110+
}
111+
112+
fn format_working_dir(&self, working_dir: &Path) -> Result<PathBuf, FormatError> {
113+
self.config
114+
.directory
115+
.resolve(&self.current_dir, working_dir)
116+
}
117+
118+
fn format_argument(
119+
&self,
120+
arg_group: &ArgumentGroup,
121+
working_dir: &Path,
122+
) -> Result<ArgumentGroup, FormatError> {
123+
match arg_group.kind {
124+
ArgumentKind::Source => {
125+
if arg_group.args.len() != 1 {
126+
panic!("source argument must have exactly one argument");
113127
}
114-
_ => {
115-
// Don't format other argument types for now
116-
// In the future, we might want to format include paths, etc.
128+
129+
let source = &arg_group.args[0];
130+
Ok(ArgumentGroup {
131+
args: vec![self.format_file(source.as_str(), working_dir)?],
132+
kind: ArgumentKind::Source,
133+
})
134+
}
135+
ArgumentKind::Output => {
136+
if arg_group.args.len() != 2 {
137+
panic!("output argument must have exactly two arguments");
117138
}
139+
140+
let output = &arg_group.args[1];
141+
Ok(ArgumentGroup {
142+
args: vec![
143+
arg_group.args[0].clone(), // Keep the first argument (e.g., "-o")
144+
self.format_file(output.as_str(), working_dir)?,
145+
],
146+
kind: ArgumentKind::Output,
147+
})
148+
}
149+
_ => {
150+
// Don't format other argument types for now
151+
// In the future, we might want to format include paths, etc.
152+
Ok(arg_group.clone())
118153
}
119154
}
120-
121-
Ok(cmd)
122155
}
123156

124-
fn format_path(
125-
path_str: &str,
126-
working_dir: &Path,
127-
resolver: &PathResolver,
128-
) -> Result<String, FormatError> {
157+
fn format_file(&self, path_str: &str, working_dir: &Path) -> Result<String, FormatError> {
129158
let path = PathBuf::from(path_str);
130-
let resolved = resolver.resolve(working_dir, &path)?;
159+
let resolved = self.config.file.resolve(working_dir, &path)?;
131160
Ok(resolved.to_string_lossy().to_string())
132161
}
133162
}
@@ -140,14 +169,19 @@ pub enum FormatError {
140169
PathsCannotBeRelative(PathBuf, PathBuf),
141170
}
142171

172+
/// Converts the `PathFormat` configuration into a `PathFormatter` instance.
173+
///
174+
/// This conversion checks the configuration and ensures that the paths are valid
175+
/// according to the rules defined in the `PathResolver` enum. And it also captures
176+
/// the current working directory to resolve relative paths correctly.
143177
impl TryFrom<&PathFormat> for PathFormatter {
144178
type Error = FormatConfigurationError;
145179

146180
fn try_from(config: &PathFormat) -> Result<Self, Self::Error> {
147181
use PathResolver::Relative;
148182

149183
// When the directory is relative, the file and output must be relative too.
150-
if config.directory == Relative && (config.file != Relative || config.output != Relative) {
184+
if config.directory == Relative && config.file != Relative {
151185
return Err(FormatConfigurationError::OnlyRelativePaths);
152186
}
153187

@@ -319,7 +353,6 @@ mod tests {
319353
let config = PathFormat {
320354
directory: PathResolver::Canonical,
321355
file: PathResolver::Canonical,
322-
output: PathResolver::Canonical,
323356
};
324357
let result = PathFormatter::try_from(&config);
325358
assert!(result.is_ok());
@@ -329,7 +362,6 @@ mod tests {
329362
let config = PathFormat {
330363
directory: PathResolver::Relative,
331364
file: PathResolver::Relative,
332-
output: PathResolver::Relative,
333365
};
334366
let result = PathFormatter::try_from(&config);
335367
assert!(result.is_ok());
@@ -342,7 +374,6 @@ mod tests {
342374
let config = PathFormat {
343375
directory: PathResolver::Relative,
344376
file: PathResolver::Canonical,
345-
output: PathResolver::Relative,
346377
};
347378
let result = PathFormatter::try_from(&config);
348379
assert!(result.is_err());
@@ -394,7 +425,6 @@ mod tests {
394425
config: PathFormat {
395426
directory: PathResolver::Canonical,
396427
file: PathResolver::Canonical,
397-
output: PathResolver::Canonical,
398428
},
399429
current_dir: execution_dir_path.to_path_buf(),
400430
};
@@ -427,7 +457,6 @@ mod tests {
427457
config: PathFormat {
428458
directory: PathResolver::Canonical,
429459
file: PathResolver::Relative,
430-
output: PathResolver::Relative,
431460
},
432461
current_dir: execution_dir_path.to_path_buf(),
433462
};
@@ -466,7 +495,6 @@ mod tests {
466495
config: PathFormat {
467496
directory: PathResolver::Relative,
468497
file: PathResolver::Relative,
469-
output: PathResolver::Relative,
470498
},
471499
current_dir: execution_dir_path.to_path_buf(),
472500
};

0 commit comments

Comments
 (0)