Skip to content

Commit 7bb9787

Browse files
committed
fix(cli): honor normalize output semantics and check safety
1 parent e806218 commit 7bb9787

3 files changed

Lines changed: 120 additions & 10 deletions

File tree

langcodec-cli/src/main.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -201,7 +201,7 @@ enum Commands {
201201
/// Normalize localization files.
202202
Normalize {
203203
/// The input files to normalize (supports glob patterns). Quote patterns to avoid shell expansion.
204-
#[arg(short, long, num_args = 1.., help = "Input files. Supports glob patterns. Quote patterns to avoid slow shell-side expansion (e.g., '/path/**/*/Localizable.strings').")]
204+
#[arg(short, long, required = true, num_args = 1.., help = "Input files. Supports glob patterns. Quote patterns to avoid slow shell-side expansion (e.g., '/path/**/*/Localizable.strings').")]
205205
inputs: Vec<String>,
206206

207207
/// Optional output file (single-file mode only). If omitted, writes in-place.

langcodec-cli/src/normalize.rs

Lines changed: 36 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ use crate::validation::{validate_file_path, validate_output_path};
33
use langcodec::{
44
Codec, FormatType, KeyStyle, NormalizeOptions as EngineNormalizeOptions, normalize_codec,
55
};
6+
use std::path::Path;
67

78
#[derive(Debug, Clone)]
89
pub struct NormalizeCliOptions {
@@ -61,6 +62,12 @@ fn write_back(codec: &Codec, input_path: &str, output_path: &Option<String>) ->
6162
}
6263
}
6364

65+
fn has_distinct_output_path(input_path: &str, output_path: &Option<String>) -> bool {
66+
output_path
67+
.as_ref()
68+
.is_some_and(|output| Path::new(output) != Path::new(input_path))
69+
}
70+
6471
pub fn run_normalize_command(opts: NormalizeCliOptions) -> Result<(), String> {
6572
let expanded = path_glob::expand_input_globs(&opts.inputs)
6673
.map_err(|e| format!("Failed to expand input patterns: {}", e))?;
@@ -73,9 +80,6 @@ pub fn run_normalize_command(opts: NormalizeCliOptions) -> Result<(), String> {
7380

7481
let input = &expanded[0];
7582
validate_file_path(input)?;
76-
if let Some(output) = &opts.output {
77-
validate_output_path(output)?;
78-
}
7983

8084
let mut codec = Codec::new();
8185
codec
@@ -92,21 +96,44 @@ pub fn run_normalize_command(opts: NormalizeCliOptions) -> Result<(), String> {
9296
)
9397
.map_err(|e| e.to_string())?;
9498

95-
if !report.changed {
99+
if opts.check {
100+
if report.changed {
101+
println!("would change: {}", input);
102+
return Err(format!("would change: {}", input));
103+
}
104+
96105
println!("No changes needed: {}", input);
97106
return Ok(());
98107
}
99108

100-
if opts.check {
101-
println!("would change: {}", input);
102-
return Err(format!("would change: {}", input));
109+
if opts.dry_run {
110+
if report.changed {
111+
println!("DRY-RUN: would change {}", input);
112+
} else {
113+
println!("No changes needed: {}", input);
114+
}
115+
return Ok(());
103116
}
104117

105-
if opts.dry_run {
106-
println!("DRY-RUN: would change {}", input);
118+
if !report.changed {
119+
if has_distinct_output_path(input, &opts.output) {
120+
if let Some(output) = &opts.output {
121+
validate_output_path(output)?;
122+
}
123+
write_back(&codec, input, &opts.output)?;
124+
println!("No changes needed: {}", input);
125+
println!("✅ Wrote output: {}", opts.output.as_deref().unwrap_or(input));
126+
return Ok(());
127+
}
128+
129+
println!("No changes needed: {}", input);
107130
return Ok(());
108131
}
109132

133+
if let Some(output) = &opts.output {
134+
validate_output_path(output)?;
135+
}
136+
110137
write_back(&codec, input, &opts.output)?;
111138
println!("✅ Normalized: {}", opts.output.as_deref().unwrap_or(input));
112139
Ok(())

langcodec-cli/tests/normalize_cli_tests.rs

Lines changed: 83 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,14 @@ fn test_main_help_lists_normalize() {
1414
assert!(stdout.contains("normalize"));
1515
}
1616

17+
#[test]
18+
fn test_normalize_requires_inputs_argument() {
19+
let output = langcodec_cmd().args(["normalize"]).output().unwrap();
20+
assert!(!output.status.success());
21+
let stderr = String::from_utf8_lossy(&output.stderr);
22+
assert!(stderr.contains("--inputs"));
23+
}
24+
1725
#[test]
1826
fn test_normalize_command_executes_successfully() {
1927
let temp_dir = TempDir::new().unwrap();
@@ -79,3 +87,78 @@ fn test_normalize_dry_run_does_not_write() {
7987
let after = fs::read_to_string(&input).unwrap();
8088
assert_eq!(after, before);
8189
}
90+
91+
#[test]
92+
fn test_normalize_output_written_even_when_unchanged() {
93+
let temp_dir = TempDir::new().unwrap();
94+
let input = temp_dir.path().join("en.strings");
95+
let output_path = temp_dir.path().join("out").join("normalized.strings");
96+
fs::write(&input, "\"a\" = \"A\";\n\"b\" = \"B\";\n").unwrap();
97+
98+
let output = langcodec_cmd()
99+
.args([
100+
"normalize",
101+
"-i",
102+
input.to_str().unwrap(),
103+
"-o",
104+
output_path.to_str().unwrap(),
105+
])
106+
.output()
107+
.unwrap();
108+
109+
assert!(
110+
output.status.success(),
111+
"stderr: {}",
112+
String::from_utf8_lossy(&output.stderr)
113+
);
114+
assert!(output_path.exists(), "expected output file to be created");
115+
116+
let written = fs::read_to_string(&output_path).unwrap();
117+
assert!(written.contains("\"a\" = \"A\";"));
118+
assert!(written.contains("\"b\" = \"B\";"));
119+
}
120+
121+
#[test]
122+
fn test_normalize_check_and_dry_run_with_output_do_not_create_directories() {
123+
for mode in ["--check", "--dry-run"] {
124+
let temp_dir = TempDir::new().unwrap();
125+
let input = temp_dir.path().join("en.strings");
126+
let missing_parent = temp_dir.path().join("missing").join("nested");
127+
let output_path = missing_parent.join("out.strings");
128+
fs::write(&input, "\"z\" = \"%@\";\n\"a\" = \"A\";\n").unwrap();
129+
130+
let output = langcodec_cmd()
131+
.args([
132+
"normalize",
133+
"-i",
134+
input.to_str().unwrap(),
135+
"-o",
136+
output_path.to_str().unwrap(),
137+
mode,
138+
])
139+
.output()
140+
.unwrap();
141+
142+
if mode == "--check" {
143+
assert!(
144+
!output.status.success(),
145+
"check mode should fail when drift is detected"
146+
);
147+
} else {
148+
assert!(
149+
output.status.success(),
150+
"dry-run failed with stderr: {}",
151+
String::from_utf8_lossy(&output.stderr)
152+
);
153+
}
154+
155+
assert!(
156+
!missing_parent.exists(),
157+
"mode {mode} unexpectedly created output directory"
158+
);
159+
assert!(
160+
!output_path.exists(),
161+
"mode {mode} unexpectedly created output file"
162+
);
163+
}
164+
}

0 commit comments

Comments
 (0)