fix(engine): use grammatical article for adjustment layer error messages (#1549)

This commit is contained in:
buivanvu94
2026-10-09 13:24:03 +07:00
committed by BFlat
parent f144ec788d
commit 1ddda05394
24 changed files with 59 additions and 24 deletions
+16
View File
@@ -543,6 +543,14 @@ impl LayerContent {
LayerContent::Smart(_) => "Smart Object",
}
}
/// English indefinite article for this layer kind ("an" for Adjustment, "a" for all others).
pub fn article(&self) -> &'static str {
match self {
LayerContent::Adjustment(_) => "an",
_ => "a",
}
}
}
#[derive(Clone, Debug, PartialEq)]
@@ -1224,6 +1232,14 @@ mod tests {
d.layer_mut(bg).unwrap().surface_mut().unwrap().write_pixel(0, 0, &[0.0, 0.0, 0.0, 1.0]);
assert_eq!(snap.layers[0].surface().unwrap().pixel(0, 0), vec![1.0; 4]);
}
#[test]
fn layer_content_article_and_kind_name() {
assert_eq!(LayerContent::Adjustment(Adjustment::Invert).article(), "an");
assert_eq!(LayerContent::Adjustment(Adjustment::Invert).kind_name(), "Adjustment");
assert_eq!(LayerContent::Raster(Surface::new(PixelFormat::RGBA8)).article(), "a");
assert_eq!(LayerContent::Group(Group { children: Vec::new(), expanded: true, artboard: None }).article(), "a");
}
}
#[cfg(test)]
+1 -1
View File
@@ -204,7 +204,7 @@ fn has_pixels(s: &Session) -> std::result::Result<(), String> {
}
let d = s.active().ok_or("no document open")?;
let l = d.active_layer.and_then(|id| d.doc.layer(id)).ok_or("no active layer")?;
if matches!(l.content, LayerContent::Raster(_)) { Ok(()) } else { Err(format!("active layer is a {} layer, not a pixel layer", l.content.kind_name())) }
if matches!(l.content, LayerContent::Raster(_)) { Ok(()) } else { Err(format!("active layer is {} {} layer, not a pixel layer", l.content.article(), l.content.kind_name())) }
}
fn has_doc(s: &Session) -> std::result::Result<(), String> {
+1 -1
View File
@@ -1649,6 +1649,6 @@ fn has_apply_target(s: &Session) -> std::result::Result<(), String> {
if matches!(l.content, LayerContent::Raster(_)) {
Ok(())
} else {
Err(format!("Apply Image needs a pixel layer (active layer is a {} layer)", l.content.kind_name()))
Err(format!("Apply Image needs a pixel layer (active layer is {} {} layer)", l.content.article(), l.content.kind_name()))
}
}
+2 -2
View File
@@ -39,7 +39,7 @@ fn has_layer(s: &Session) -> std::result::Result<(), String> {
}
fn has_pixel_layer(s: &Session) -> std::result::Result<(), String> {
let l = crate::active_layer_of(s)?;
if matches!(l.content, LayerContent::Raster(_)) { Ok(()) } else { Err(format!("active layer is a {} layer, not a pixel layer", l.content.kind_name())) }
if matches!(l.content, LayerContent::Raster(_)) { Ok(()) } else { Err(format!("active layer is {} {} layer, not a pixel layer", l.content.article(), l.content.kind_name())) }
}
/// A pixel layer, or a targeted alpha channel / Quick Mask (adjustments and fills apply to it).
fn has_pixel_or_channel(s: &Session) -> std::result::Result<(), String> {
@@ -57,7 +57,7 @@ pub(crate) fn has_paintable(s: &Session) -> std::result::Result<(), String> {
if matches!(l.content, LayerContent::Raster(_)) || l.mask.is_some() {
Ok(())
} else {
Err(format!("active layer is a {} layer without a mask", l.content.kind_name()))
Err(format!("active layer is {} {} layer without a mask", l.content.article(), l.content.kind_name()))
}
}
+1 -1
View File
@@ -21,7 +21,7 @@ const REFINE: RefineParams = RefineParams { radius: 2.0, smart_radius: true, smo
/// Remove Background needs an unlocked pixel layer.
fn check(doc: &Document, l: &Layer) -> std::result::Result<(), String> {
if !matches!(l.content, LayerContent::Raster(_)) {
return Err(format!("the layer is a {} layer, not a pixel layer", l.content.kind_name()));
return Err(format!("the layer is {} {} layer, not a pixel layer", l.content.article(), l.content.kind_name()));
}
let locks = doc.effective_locks(l.id);
if locks.pixels || locks.all {
+1 -1
View File
@@ -201,7 +201,7 @@ fn pixel_layer(s: &Session) -> std::result::Result<(), String> {
match &d.doc.layer(id).ok_or("no active layer")?.content {
LayerContent::Raster(_) => Ok(()),
LayerContent::Smart(sm) if sm.cache.is_some() => Ok(()),
other => Err(format!("needs a pixel layer or smart object (active layer is a {} layer)", other.kind_name())),
other => Err(format!("needs a pixel layer or smart object (active layer is {} {} layer)", other.article(), other.kind_name())),
}
}
+1 -1
View File
@@ -112,7 +112,7 @@ fn pixel_layer(s: &Session) -> std::result::Result<LayerId, String> {
let id = d.active_layer.ok_or("no active layer")?;
let l = d.doc.layer(id).ok_or("no active layer")?;
if !matches!(l.content, LayerContent::Raster(_)) {
return Err(format!("active layer is a {} layer, not a pixel layer", l.content.kind_name()));
return Err(format!("active layer is {} {} layer, not a pixel layer", l.content.article(), l.content.kind_name()));
}
let locks = d.doc.effective_locks(id);
if locks.all || locks.pixels {
+1 -1
View File
@@ -47,7 +47,7 @@ fn enum_param<T: serde::de::DeserializeOwned>(p: &Value, cmd: &str, k: &str, def
fn has_pixel_layer(s: &Session) -> std::result::Result<(), String> {
let d = s.active().ok_or("no document open")?;
let l = d.active_layer.and_then(|id| d.doc.layer(id)).ok_or("no active layer")?;
if matches!(l.content, LayerContent::Raster(_)) { Ok(()) } else { Err(format!("active layer is a {} layer, not a pixel layer", l.content.kind_name())) }
if matches!(l.content, LayerContent::Raster(_)) { Ok(()) } else { Err(format!("active layer is {} {} layer, not a pixel layer", l.content.article(), l.content.kind_name())) }
}
fn active_pixel_layer(s: &Session, cmd: &str) -> Result<LayerId> {
+1 -1
View File
@@ -29,7 +29,7 @@ fn has_layer(s: &Session) -> std::result::Result<(), String> {
fn has_pixels(s: &Session) -> std::result::Result<(), String> {
match active_layer(s)?.content {
LayerContent::Raster(_) => Ok(()),
ref c => Err(format!("active layer is a {} layer, not a pixel layer", c.kind_name())),
ref c => Err(format!("active layer is {} {} layer, not a pixel layer", c.article(), c.kind_name())),
}
}
+1 -1
View File
@@ -195,7 +195,7 @@ pub(crate) fn has_filterable_layer(s: &Session) -> std::result::Result<(), Strin
match &l.content {
LayerContent::Raster(_) => Ok(()),
LayerContent::Smart(sm) if sm.cache.is_some() => Ok(()),
other => Err(format!("filters need a pixel layer (active layer is a {} layer)", other.kind_name())),
other => Err(format!("filters need a pixel layer (active layer is {} {} layer)", other.article(), other.kind_name())),
}
}
+1 -1
View File
@@ -38,7 +38,7 @@ fn has_layer(s: &Session) -> Enabled {
fn has_raster(s: &Session) -> Enabled {
let l = active_layer(s)?;
if matches!(l.content, LayerContent::Raster(_)) { Ok(()) } else { Err(format!("active layer is a {} layer, not a pixel layer", l.content.kind_name())) }
if matches!(l.content, LayerContent::Raster(_)) { Ok(()) } else { Err(format!("active layer is {} {} layer, not a pixel layer", l.content.article(), l.content.kind_name())) }
}
fn has_raster_with_mask(s: &Session) -> Enabled {
+1 -1
View File
@@ -262,7 +262,7 @@ fn filterable(s: &Session) -> std::result::Result<(), String> {
match &d.doc.layer(id).ok_or("no active layer")?.content {
LayerContent::Raster(_) => Ok(()),
LayerContent::Smart(sm) if sm.cache.is_some() => Ok(()),
other => Err(format!("filters need a pixel layer (active layer is a {} layer)", other.kind_name())),
other => Err(format!("filters need a pixel layer (active layer is {} {} layer)", other.article(), other.kind_name())),
}
}
+1 -1
View File
@@ -93,7 +93,7 @@ fn run(s: &mut Session, p: &Value) -> Result<Value> {
};
crate::smart_cmds::add_smart_filter(doc, layer, sf, selection.as_ref())
}
other => Err(EngineError::Other(format!("plug-in filters need a pixel layer (active layer is a {} layer)", other.kind_name()))),
other => Err(EngineError::Other(format!("plug-in filters need a pixel layer (active layer is {} {} layer)", other.article(), other.kind_name()))),
}
})?;
Ok(json!({"layer": layer.0, "plugin": id}))
+1 -1
View File
@@ -42,7 +42,7 @@ fn has_pixel_layer(s: &Session) -> std::result::Result<(), String> {
if matches!(l.content, LayerContent::Raster(_)) || l.mask.is_some() {
Ok(())
} else {
Err(format!("active layer is a {} layer, not a pixel layer", l.content.kind_name()))
Err(format!("active layer is {} {} layer, not a pixel layer", l.content.article(), l.content.kind_name()))
}
}
+1 -1
View File
@@ -890,7 +890,7 @@ fn active_smart(s: &Session) -> std::result::Result<&SmartObject, String> {
let id = d.active_layer.ok_or("no active layer")?;
match &d.doc.layer(id).ok_or("no active layer")?.content {
LayerContent::Smart(sm) => Ok(sm),
other => Err(format!("the active layer is a {} layer, not a smart object", other.kind_name())),
other => Err(format!("the active layer is {} {} layer, not a smart object", other.article(), other.kind_name())),
}
}
fn has_smart(s: &Session) -> std::result::Result<(), String> {
+1 -1
View File
@@ -190,7 +190,7 @@ fn refine_edge(s: &mut Session, p: &Value) -> Result<Value> {
let src_layer = d.doc.layer(id).ok_or(EngineError::NoLayer(id))?;
let src = src_layer
.surface()
.ok_or_else(|| EngineError::Other(format!("the active layer is a {} layer without pixels", src_layer.content.kind_name())))?;
.ok_or_else(|| EngineError::Other(format!("the active layer is {} {} layer without pixels", src_layer.content.article(), src_layer.content.kind_name())))?;
let name = d.doc.copy_name(&src_layer.name);
let empty = Region { bbox: Rect::EMPTY, mask: Vec::new() };
let reg = region.as_ref().unwrap_or(&empty);
+19
View File
@@ -1086,3 +1086,22 @@ fn move_document_reorders_tabs_and_keeps_the_active_one() {
}
assert_eq!(names(&s), first);
}
#[test]
fn adjustment_layer_disabled_diagnostics_use_grammatical_article() {
let mut s = session_with_doc();
s.execute("layer.newAdjustmentLayer.curves", json!({})).unwrap();
let err_blur = s.execute("filter.blur.gaussianBlur", json!({"radius": 2})).unwrap_err().to_string();
assert!(err_blur.contains("an Adjustment layer"), "expected 'an Adjustment layer', got: {err_blur}");
assert!(!err_blur.contains("a Adjustment layer"), "found 'a Adjustment layer': {err_blur}");
let err_equalize = s.execute("image.adjustments.equalize", json!({})).unwrap_err().to_string();
assert!(err_equalize.contains("an Adjustment layer"), "expected 'an Adjustment layer', got: {err_equalize}");
assert!(!err_equalize.contains("a Adjustment layer"), "found 'a Adjustment layer': {err_equalize}");
let err_clear = s.execute("edit.clear", json!({})).unwrap_err().to_string();
assert!(err_clear.contains("an Adjustment layer"), "expected 'an Adjustment layer', got: {err_clear}");
assert!(!err_clear.contains("a Adjustment layer"), "found 'a Adjustment layer': {err_clear}");
}
+1 -1
View File
@@ -27,7 +27,7 @@ fn trap(s: &mut Session, p: &Value) -> Result<Value> {
let bounds = doc.bounds();
let l = doc.layer_mut(id).ok_or(EngineError::NoLayer(id))?;
let LayerContent::Raster(surf) = &mut l.content else {
return Err(EngineError::BadParams { cmd: cmd.into(), msg: format!("Trap needs a pixel layer (active layer is a {})", l.content.kind_name()) });
return Err(EngineError::BadParams { cmd: cmd.into(), msg: format!("Trap needs a pixel layer (active layer is {} {})", l.content.article(), l.content.kind_name()) });
};
if bounds.is_empty() {
return Ok(());
+1 -1
View File
@@ -69,7 +69,7 @@ struct Frame {
fn frame(doc: &Document, id: LayerId, cmd: &str) -> Result<Frame> {
let layer = doc.layer(id).ok_or(EngineError::NoLayer(id))?;
let LayerContent::Text(t) = &layer.content else {
return Err(bad(cmd, format!("layer {} is a {} layer, not a type layer", id.0, layer.content.kind_name())));
return Err(bad(cmd, format!("layer {} is {} {} layer, not a type layer", id.0, layer.content.article(), layer.content.kind_name())));
};
let text = t.text.clone();
let transform = t.transform;
+1 -1
View File
@@ -418,7 +418,7 @@ fn with_text_layer<R>(s: &mut Session, p: &Value, label: &str, f: impl FnOnce(&m
let l = doc.layer_mut(id).ok_or(EngineError::NoLayer(id))?;
let auto_named = is_auto_named(l);
let LayerContent::Text(t) = &mut l.content else {
return Err(EngineError::Other(format!("layer {} is a {} layer, not a type layer", id.0, l.content.kind_name())));
return Err(EngineError::Other(format!("layer {} is {} {} layer, not a type layer", id.0, l.content.article(), l.content.kind_name())));
};
let before = t.cache.as_ref().map(|c| c.tile_bounds());
let mut rename = None;
+1 -1
View File
@@ -72,7 +72,7 @@ fn with_text<R>(s: &mut Session, p: &Value, label: &str, f: impl FnOnce(&mut Tex
let l = doc.layer_mut(id).ok_or(EngineError::NoLayer(id))?;
let auto_named = is_auto_named(l);
let LayerContent::Text(t) = &mut l.content else {
return Err(EngineError::Other(format!("layer {} is a {} layer, not a type layer", id.0, l.content.kind_name())));
return Err(EngineError::Other(format!("layer {} is {} {} layer, not a type layer", id.0, l.content.article(), l.content.kind_name())));
};
let r = f(t, &snapshot)?;
refresh(&snapshot, t);
+2 -2
View File
@@ -503,7 +503,7 @@ fn shape_info(s: &Session, id: LayerId) -> Result<Value> {
let d = s.active().ok_or(EngineError::NoDocument)?;
let l = d.doc.layer(id).ok_or(EngineError::NoLayer(id))?;
let LayerContent::Shape(sh) = &l.content else {
return Err(EngineError::Other(format!("layer {} is a {} layer, not a shape layer", id.0, l.content.kind_name())));
return Err(EngineError::Other(format!("layer {} is {} {} layer, not a shape layer", id.0, l.content.article(), l.content.kind_name())));
};
let bounds = sh.cache.as_ref().map(|c| c.content_bounds()).filter(|r| !r.is_empty()).map(|r| json!([r.x0, r.y0, r.width(), r.height()]));
Ok(json!({
@@ -523,7 +523,7 @@ pub(crate) fn with_shape<R>(s: &mut Session, id: LayerId, label: &str, f: impl F
let snapshot = doc.clone();
let l = doc.layer_mut(id).ok_or(EngineError::NoLayer(id))?;
if !matches!(l.content, LayerContent::Shape(_)) {
return Err(EngineError::Other(format!("layer {} is a {} layer, not a shape layer", id.0, l.content.kind_name())));
return Err(EngineError::Other(format!("layer {} is {} {} layer, not a shape layer", id.0, l.content.article(), l.content.kind_name())));
}
let LayerContent::Shape(mut sh) = std::mem::replace(&mut l.content, LayerContent::Fill(Fill::Solid(Color::BLACK))) else {
return Err(EngineError::Other(format!("layer {} is not a shape layer", id.0)));
+1 -1
View File
@@ -69,7 +69,7 @@ fn has_layer(s: &Session) -> std::result::Result<(), String> {
let d = s.active().ok_or("no document open")?;
match d.active_layer.and_then(|id| d.doc.layer(id)).map(|l| &l.content) {
Some(LayerContent::Raster(_)) => Ok(()),
Some(other) => Err(format!("Vanishing Point needs a pixel layer (active layer is a {} layer)", other.kind_name())),
Some(other) => Err(format!("Vanishing Point needs a pixel layer (active layer is {} {} layer)", other.article(), other.kind_name())),
None => Err("no active layer".into()),
}
}
+1 -1
View File
@@ -51,7 +51,7 @@ pub(crate) fn active_pixels(app: &PhotocraftApp) -> Result<(LayerId, Surface, bo
match &l.content {
LayerContent::Raster(s) => Ok((id, s.clone(), false)),
LayerContent::Smart(sm) => sm.cache.clone().map(|c| (id, c, true)).ok_or_else(|| "the smart object has no pixels".to_string()),
other => Err(format!("needs a pixel layer or smart object (active layer is a {} layer)", other.kind_name())),
other => Err(format!("needs a pixel layer or smart object (active layer is {} {} layer)", other.article(), other.kind_name())),
}
}