diff --git a/crates/konnect-core/src/tools/sch_batch.rs b/crates/konnect-core/src/tools/sch_batch.rs index 6cdacdb0..52058141 100644 --- a/crates/konnect-core/src/tools/sch_batch.rs +++ b/crates/konnect-core/src/tools/sch_batch.rs @@ -241,7 +241,12 @@ pub fn tools() -> Vec { "x": { "type": "number", "description": "X position in mm" }, "y": { "type": "number", "description": "Y position in mm" }, "size": { "type": "number", "description": "Font size in mm", "default": 1.27 }, - "rotation": { "type": "number", "description": "Rotation in degrees", "default": 0 } + "rotation": { "type": "number", "description": "Rotation in degrees", "default": 0 }, + "justify": { + "type": "string", + "description": "Alignment of the text against x/y: at most one horizontal token (left, right) and one vertical token (top, bottom), space separated. An axis you leave out is centred - KiCad has no 'center' keyword and encodes centring by omission, so 'bottom' means horizontally centred and bottom-aligned. 'center' is shorthand for centring both axes. Defaults to 'left bottom', what KiCad itself writes for a placed annotation; a centred horizontal axis can carry a long line off the page.", + "default": "left bottom" + } }, "required": ["schematic", "text", "x", "y"] }), @@ -1026,6 +1031,62 @@ async fn handle_connect_passthrough( }))) } +/// KiCad's default alignment for a placed text annotation. +/// +/// Measured against KiCad 10's own demo projects: every text block in the +/// current file format carries `(justify left bottom)`. +const DEFAULT_TEXT_JUSTIFY: &str = "left bottom"; + +/// Translate a caller's alignment into the `(justify ...)` clause KiCad writes. +/// +/// Alignment is per axis, and centring an axis means leaving its token out: +/// `left` is left-aligned and vertically centred, `bottom` is horizontally +/// centred and bottom-aligned. `"center"` centres both and so returns an empty +/// string. There is no token for it — `(justify center)` makes KiCad refuse the +/// whole file, the same way a misplaced item does. +fn schematic_text_justify(value: &str) -> Result { + fn claim( + slot: &mut Option<&'static str>, + value: &'static str, + token: &str, + ) -> Result<(), String> { + if let Some(existing) = slot { + return Err(format!( + "justify names '{existing}' and '{token}' on the same axis - use at most one horizontal (left, right) and one vertical (top, bottom) token" + )); + } + *slot = Some(value); + Ok(()) + } + + let trimmed = value.trim(); + if trimmed.is_empty() || trimmed.eq_ignore_ascii_case("center") { + return Ok(String::new()); + } + + let mut horizontal: Option<&'static str> = None; + let mut vertical: Option<&'static str> = None; + for token in trimmed.split_whitespace() { + match token.to_ascii_lowercase().as_str() { + "left" => claim(&mut horizontal, "left", token)?, + "right" => claim(&mut horizontal, "right", token)?, + "top" => claim(&mut vertical, "top", token)?, + "bottom" => claim(&mut vertical, "bottom", token)?, + _ => { + return Err(format!( + "unknown justify token '{token}' - use left, right, top, bottom, or center" + )) + } + } + } + + // KiCad writes the horizontal token first. + let mut parts = Vec::with_capacity(2); + parts.extend(horizontal); + parts.extend(vertical); + Ok(format!(" (justify {})", parts.join(" "))) +} + async fn handle_add_schematic_text( args: &serde_json::Value, _ctx: &crate::tools::ToolContext, @@ -1045,6 +1106,14 @@ async fn handle_add_schematic_text( }; let size = args["size"].as_f64().unwrap_or(1.27); let rotation = args["rotation"].as_f64().unwrap_or(0.0); + let justify = args["justify"].as_str().unwrap_or(DEFAULT_TEXT_JUSTIFY); + // Without a justify token KiCad centres the text on `x`, so a long line + // crosses the page edge and is dropped from the PDF export while the + // .kicad_sch still looks complete. + let justify_sexp = match schematic_text_justify(justify) { + Ok(v) => v, + Err(e) => return Ok(CallToolResult::error(e)), + }; let uuid = new_uuid(); // Escape for a KiCad quoted string. Newlines and tabs must become their @@ -1060,7 +1129,7 @@ async fn handle_add_schematic_text( let text_sexp = format!( "\n (text \"{escaped}\"\n (at {x} {y} {rotation})\n \ - (effects (font (size {size} {size})))\n (uuid \"{uuid}\")\n )" + (effects (font (size {size} {size})){justify_sexp})\n (uuid \"{uuid}\")\n )" ); let content = read_consistent(&sch_path)?; @@ -1076,6 +1145,7 @@ async fn handle_add_schematic_text( "x": x, "y": y, "size": size, "rotation": rotation, + "justify": justify, "uuid": uuid }))) } @@ -2095,7 +2165,8 @@ mod multi_unit_field_tests { #[cfg(test)] mod add_text_placement_tests { - use super::tools; + use super::{schematic_text_justify, tools}; + use crate::mcp::protocol::CallToolResult; use crate::tools::ToolContext; use serde_json::json; use std::io::Write; @@ -2103,7 +2174,7 @@ mod add_text_placement_tests { const SCH: &str = "(kicad_sch\n\t(version 20260306)\n\t(generator \"eeschema\")\n\t(uuid \"root\")\n\t(lib_symbols\n\t\t(symbol \"Device:R\"\n\t\t\t(property \"Reference\" \"R\")\n\t\t)\n\t)\n\t(symbol\n\t\t(lib_id \"Device:R\")\n\t\t(at 100 80 0)\n\t\t(unit 1)\n\t\t(uuid \"u1\")\n\t\t(property \"Reference\" \"R1\"\n\t\t\t(at 100 75 0)\n\t\t)\n\t)\n\t(sheet_instances\n\t\t(path \"/\" (page \"1\"))\n\t)\n)\n"; - async fn add_text(text: &str) -> String { + async fn add_text_inner(text: &str, justify: Option<&str>) -> (String, CallToolResult) { let mut f = tempfile::NamedTempFile::with_suffix(".kicad_sch").unwrap(); f.write_all(SCH.as_bytes()).unwrap(); f.flush().unwrap(); @@ -2123,12 +2194,27 @@ mod add_text_placement_tests { }; let router = Arc::new(crate::router::ToolRouter::new()); let ctx = Arc::new(ToolContext::new(cfg, router)); - let args = json!({ + let mut args = json!({ "schematic": f.path().to_str().unwrap(), "text": text, "x": 30.0, "y": 114.3 }); - (def.handler)(&args, ctx).await.unwrap(); - std::fs::read_to_string(f.path()).unwrap() + if let Some(j) = justify { + args["justify"] = json!(j); + } + let result = (def.handler)(&args, ctx).await.unwrap(); + (std::fs::read_to_string(f.path()).unwrap(), result) + } + + async fn add_text(text: &str) -> String { + add_text_inner(text, None).await.0 + } + + async fn add_text_justified(text: &str, justify: &str) -> String { + add_text_inner(text, Some(justify)).await.0 + } + + async fn add_text_result(text: &str, justify: &str) -> CallToolResult { + add_text_inner(text, Some(justify)).await.1 } /// The regression: the text was spliced in at the file's last `)`, which @@ -2168,6 +2254,77 @@ mod add_text_placement_tests { let out = add_text("a \"b\" c\\d\te").await; assert!(out.contains(r#"(text "a \"b\" c\\d\te""#), "got:\n{out}"); } + + /// The reported defect: with no `(justify ...)` KiCad centres the text on + /// `x`, so a long line crosses the left page edge and vanishes from the PDF + /// export while the .kicad_sch still reads as complete. + #[tokio::test] + async fn text_is_left_aligned_by_default() { + let out = add_text("hello").await; + assert!( + out.contains("(effects (font (size 1.27 1.27)) (justify left bottom))"), + "got:\n{out}" + ); + } + + #[tokio::test] + async fn justify_is_written_as_kicad_orders_it() { + let out = add_text_justified("hello", "top right").await; + assert!( + out.contains("(justify right top)"), + "horizontal token comes first, got:\n{out}" + ); + } + + /// KiCad has no `center` token: centred text is text with no justify at + /// all. This is the pre-change behaviour, still reachable on request. + #[tokio::test] + async fn center_writes_no_justify_token() { + let out = add_text_justified("hello", "center").await; + assert!(!out.contains("(justify"), "got:\n{out}"); + assert!( + out.contains("(effects (font (size 1.27 1.27)))"), + "got:\n{out}" + ); + } + + #[tokio::test] + async fn an_unknown_token_is_refused() { + let result = add_text_result("hello", "middle").await; + assert!( + result.is_error, + "an unknown alignment must not be guessed at" + ); + } + + #[tokio::test] + async fn two_tokens_on_one_axis_are_refused() { + let result = add_text_result("hello", "left right").await; + assert!(result.is_error); + } + + #[test] + fn justify_parsing_covers_the_shapes_kicad_writes() { + // The five forms present in KiCad 10's own demo projects. + for (input, expected) in [ + ("left bottom", " (justify left bottom)"), + ("right bottom", " (justify right bottom)"), + ("left", " (justify left)"), + // One axis alone: the other is centred by omission. + ("bottom", " (justify bottom)"), + ("left top", " (justify left top)"), + ("right", " (justify right)"), + ] { + assert_eq!(schematic_text_justify(input).unwrap(), expected, "{input}"); + } + assert_eq!(schematic_text_justify("center").unwrap(), ""); + assert_eq!( + schematic_text_justify(" BOTTOM Left ").unwrap(), + " (justify left bottom)" + ); + assert!(schematic_text_justify("sideways").is_err()); + assert!(schematic_text_justify("top bottom").is_err()); + } } /// `add_schematic_text` was not the only handler splicing at the file's last diff --git a/tool-directory.md b/tool-directory.md index 9ab46bc8..1895eb5c 100644 --- a/tool-directory.md +++ b/tool-directory.md @@ -155,7 +155,7 @@ Six tools, grouped into *discovery/routing* and *observability*. | `batch_edit_schematic_components` | Apply field updates (Value, Footprint, custom properties) to multiple components in a single atomic write. | | `batch_delete_schematic_components` | Delete multiple components by reference designator in a single atomic write. | | `connect_passthrough` | Add a wire stub and matching net label at a point to route a signal through a region without drawing a full path. Direction defaults to `auto`. | -| `add_schematic_text` | Add a text annotation (non-net label) to the schematic at a given position. | +| `add_schematic_text` | Add a text annotation (non-net label) to the schematic at a given position. Aligns the text against that position with `justify`, per axis and defaulting to `left bottom` as KiCad does; an omitted axis is centred, and `center` centres both. | | `get_schematic_layout` | Return a compact spatial summary of the schematic: component positions, bounding box, optionally wires and labels. | | `validate_wire_connections` | Check all wire endpoints for floating ends not connected to a pin, label, or another wire. | | `validate_component_connections` | Check that every non-passive pin has at least one wire or label connected. Reports unconnected pins. |