mirror of
https://github.com/storytold/photocraft.git
synced 2026-10-14 19:26:51 -04:00
fix(cli): report missing font fallbacks in run, info and convert (#1299)
* fix(cli): report missing font substitutions in run, info, and convert (#1251) * test(cli): verify missing fonts in run/info/convert and no duplicate or false warnings * docs(cli): document missing-font fallback warnings in convert, info and run * style(cli): apply exact rustfmt format for multi-command warning test
This commit is contained in:
1 parent
a593b3651c
commit
05b405aab0
2 files changed
+136
-2
No files matched your search
@@ -186,6 +186,12 @@ fn warn_all(err: &mut dyn Write, ws: &[String]) {
|
||||
}
|
||||
}
|
||||
|
||||
/// The engine knows which type families aren't installed. Warn before an export
|
||||
/// uses a fallback face rather than silently changing the appearance of the text.
|
||||
fn missing_font_warnings(fonts: Vec<String>) -> Vec<String> {
|
||||
fonts.into_iter().map(|font| format!("font '{font}' is not installed; text may render using a fallback face")).collect()
|
||||
}
|
||||
|
||||
fn automation_workspace(args: &Args) -> Result<AuthorizedWorkspace, String> {
|
||||
AuthorizedWorkspace::new(args.get("--automation-read-root").map(Path::new), args.get("--automation-write-root").map(Path::new))
|
||||
.map_err(|error| error.to_string())
|
||||
@@ -198,6 +204,7 @@ fn convert(a: &Args, _out: &mut dyn Write, err: &mut dyn Write) -> R {
|
||||
let opts = export_opts(a)?;
|
||||
let o = files::open(Path::new(input)).map_err(|e| e.to_string())?;
|
||||
warn_all(err, &o.warnings);
|
||||
warn_all(err, &missing_font_warnings(photocraft_engine::type_extra_cmds::missing_fonts(&o.document)));
|
||||
let ws = files::save(&o.document, Path::new(output), a.get("--format"), &opts, None).map_err(|e| e.to_string())?;
|
||||
warn_all(err, &ws);
|
||||
Ok(())
|
||||
@@ -215,7 +222,11 @@ fn info(a: &Args, out: &mut dyn Write) -> R {
|
||||
let mut h = Headless::trusted_local();
|
||||
let opened = h.open(Path::new(file)).map_err(|e| e.to_string())?;
|
||||
let mut doc = h.inspect(None).map_err(|e| e.to_string())?;
|
||||
doc["warnings"] = opened["warnings"].clone();
|
||||
let mut warnings = opened["warnings"].as_array().cloned().unwrap_or_default();
|
||||
if let Some(st) = h.session.active() {
|
||||
warnings.extend(missing_font_warnings(photocraft_engine::type_extra_cmds::missing_fonts(&st.doc)).into_iter().map(Value::String));
|
||||
}
|
||||
doc["warnings"] = Value::Array(warnings);
|
||||
doc["file"] = json!(file);
|
||||
// Drop per-session noise.
|
||||
if let Value::Object(m) = &mut doc {
|
||||
@@ -261,10 +272,34 @@ fn run_cmds(a: &Args, out: &mut dyn Write, err: &mut dyn Write) -> R {
|
||||
if cmds.is_empty() && a.get("--out").is_none() {
|
||||
return Err("run needs at least one --cmd (or --out)".into());
|
||||
}
|
||||
let mut reported_fonts = std::collections::HashSet::<String>::new();
|
||||
for (id, params) in cmds {
|
||||
let r = h.command_run(&id, params).map_err(|e| format!("`{id}`: {e}"))?;
|
||||
print_json(out, &json!({"command": id, "result": r}), true)?;
|
||||
let new_warnings: Vec<String> = h
|
||||
.session
|
||||
.active()
|
||||
.map(|st| missing_font_warnings(photocraft_engine::type_extra_cmds::missing_fonts(&st.doc)))
|
||||
.unwrap_or_default()
|
||||
.into_iter()
|
||||
.filter(|w| reported_fonts.insert(w.clone()))
|
||||
.collect();
|
||||
warn_all(err, &new_warnings);
|
||||
let mut result = json!({"command": id, "result": r});
|
||||
if !new_warnings.is_empty() {
|
||||
result["warnings"] = json!(new_warnings);
|
||||
}
|
||||
print_json(out, &result, true)?;
|
||||
}
|
||||
// A `run` with only `--out` still reports missing fonts before exporting.
|
||||
let remaining: Vec<String> = h
|
||||
.session
|
||||
.active()
|
||||
.map(|st| missing_font_warnings(photocraft_engine::type_extra_cmds::missing_fonts(&st.doc)))
|
||||
.unwrap_or_default()
|
||||
.into_iter()
|
||||
.filter(|w| reported_fonts.insert(w.clone()))
|
||||
.collect();
|
||||
warn_all(err, &remaining);
|
||||
if let Some(o) = a.get("--out") {
|
||||
let r = h.save(None, Some(Path::new(o)), a.get("--format"), &opts).map_err(|e| e.to_string())?;
|
||||
let ws: Vec<String> = serde_json::from_value(r["warnings"].clone()).unwrap_or_default();
|
||||
@@ -429,3 +464,88 @@ fn mcp(a: &Args) -> R {
|
||||
let rt = tokio::runtime::Builder::new_multi_thread().enable_all().build().map_err(|e| e.to_string())?;
|
||||
rt.block_on(server.serve_stdio()).map_err(|e| e.to_string())
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod missing_font_warning_tests {
|
||||
use super::*;
|
||||
|
||||
fn invoke(args: &[&str]) -> (i32, String, String) {
|
||||
let mut output = Vec::new();
|
||||
let mut errors = Vec::new();
|
||||
let args: Vec<String> = args.iter().map(|s| s.to_string()).collect();
|
||||
let code = run(&args, &mut output, &mut errors);
|
||||
(code, String::from_utf8(output).unwrap(), String::from_utf8(errors).unwrap())
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn missing_typeface_is_reported_by_run_info_and_convert() {
|
||||
let family = "DefinitelyMissingPhotoCraftTypeface1251";
|
||||
let folder = std::env::temp_dir().join(format!("photocraft-cli-font-warnings-{}", std::process::id()));
|
||||
std::fs::create_dir_all(&folder).unwrap();
|
||||
let native = folder.join("missing.pcraft");
|
||||
let png = folder.join("missing.png");
|
||||
let native_str = native.to_string_lossy().to_string();
|
||||
let png_str = png.to_string_lossy().to_string();
|
||||
let params = json!({"x": 6, "y": 38, "text": "Missing font warning", "font": family, "size": 20}).to_string();
|
||||
|
||||
let (code, stdout, stderr) = invoke(&[
|
||||
"run",
|
||||
"--new",
|
||||
r#"{"width":256,"height":96,"background":"white"}"#,
|
||||
"--cmd",
|
||||
"type.create",
|
||||
"--params",
|
||||
¶ms,
|
||||
"--cmd",
|
||||
"type.resolveMissingFonts",
|
||||
"--out",
|
||||
&native_str,
|
||||
]);
|
||||
assert_eq!(code, 0, "{stderr}");
|
||||
assert_eq!(stderr.matches(family).count(), 1, "multi-command runs should not repeat the same missing font warning");
|
||||
let commands: Vec<Value> = stdout.lines().map(|s| serde_json::from_str(s).unwrap()).collect();
|
||||
assert_eq!(commands.len(), 2, "{stdout}");
|
||||
assert!(commands[0]["warnings"].as_array().unwrap().iter().any(|w| w.as_str().is_some_and(|s| s.contains(family))));
|
||||
assert!(commands[1].get("warnings").is_none(), "only newly detected missing fonts are reported");
|
||||
|
||||
let (code, stdout, stderr) = invoke(&["info", &native_str, "--compact"]);
|
||||
assert_eq!(code, 0, "{stderr}");
|
||||
let info: Value = serde_json::from_str(&stdout).unwrap();
|
||||
assert!(info["warnings"].as_array().unwrap().iter().any(|w| w.as_str().is_some_and(|s| s.contains(family))));
|
||||
assert!(stderr.is_empty(), "info puts warnings in JSON, not stderr");
|
||||
|
||||
let (code, _, stderr) = invoke(&["convert", &native_str, &png_str]);
|
||||
assert_eq!(code, 0, "{stderr}");
|
||||
assert!(stderr.contains(&format!("warning: font '{family}'")), "{stderr}");
|
||||
assert!(png.is_file());
|
||||
|
||||
// Even with no commands, run --out checks the opened document before export.
|
||||
let (code, stdout, stderr) = invoke(&["run", &native_str, "--out", &png_str]);
|
||||
assert_eq!(code, 0, "{stderr}");
|
||||
assert!(stdout.is_empty());
|
||||
assert_eq!(stderr.matches(family).count(), 1);
|
||||
|
||||
let _ = std::fs::remove_dir_all(&folder);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn no_type_layers_do_not_produce_missing_font_warnings() {
|
||||
let folder = std::env::temp_dir().join(format!("photocraft-cli-no-font-warning-{}", std::process::id()));
|
||||
std::fs::create_dir_all(&folder).unwrap();
|
||||
let native = folder.join("blank.pcraft");
|
||||
let png = folder.join("blank.png");
|
||||
let native_str = native.to_string_lossy().to_string();
|
||||
let png_str = png.to_string_lossy().to_string();
|
||||
let (code, _, stderr) = invoke(&["run", "--new", r#"{"width":16,"height":16}"#, "--out", &native_str]);
|
||||
assert_eq!(code, 0, "{stderr}");
|
||||
assert!(!stderr.contains("font '"), "{stderr}");
|
||||
let (code, stdout, stderr) = invoke(&["info", &native_str, "--compact"]);
|
||||
assert_eq!(code, 0, "{stderr}");
|
||||
let info: Value = serde_json::from_str(&stdout).unwrap();
|
||||
assert!(info["warnings"].as_array().unwrap().iter().all(|w| !w.as_str().unwrap_or_default().contains("font '")));
|
||||
let (code, _, stderr) = invoke(&["convert", &native_str, &png_str]);
|
||||
assert_eq!(code, 0, "{stderr}");
|
||||
assert!(!stderr.contains("font '"), "{stderr}");
|
||||
let _ = std::fs::remove_dir_all(&folder);
|
||||
}
|
||||
}
|
||||
@@ -21,6 +21,20 @@ photocraft-cli run input.psd \
|
||||
--out output.png
|
||||
```
|
||||
|
||||
### Missing font warnings
|
||||
|
||||
If a type layer uses a font family that is not installed, `convert` and `run`
|
||||
write a warning to standard error before exporting; they do not silently present
|
||||
the fallback rendering as an exact match. `info --compact` includes the same
|
||||
warning in its `warnings` JSON array, without writing it to stderr. `run`
|
||||
also adds `warnings` to the JSON result for the command that first exposes
|
||||
the missing family. A multi-command `run` reports each missing family once,
|
||||
even if later commands still use it.
|
||||
|
||||
Warnings do not change the saved document's font declarations and do not stop
|
||||
the operation. To list or replace the missing families intentionally, use
|
||||
`type.resolveMissingFonts` on the document.
|
||||
|
||||
Paths supplied to the current CLI are ordinary operating-system paths. The CLI does not restrict them to a workspace root. Batch output creates the requested output directory, and save operations may create parent directories. `batch` refuses an output directory that is its input directory (the results would replace the originals) unless `--in-place` is given. Run untrusted action files only in an appropriately isolated account or environment.
|
||||
|
||||
CLI parsing reports malformed flags and JSON as errors, and a flag the subcommand doesn't take (a typo such as `--fromat`) is a usage error (exit status 2) rather than being ignored. It is not a security policy layer. Engine commands remain responsible for validating their own parameters.
|
||||
Reference in new issue
Block a user