From 0cad921f663d52236d4efe9f127bbf567af27212 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 10 Nov 2026 16:38:56 +0000 Subject: [PATCH 1/6] GUI Builder: MCP tools for editing forms The builder's MCP server could inspect, select, drag and save, but a client could not build anything. Add editing tools, each on the builder's own code paths and validated as the inspector validates the same change: guibuilder_component_types, guibuilder_get_form_xml, guibuilder_add_component, guibuilder_insert_xml, guibuilder_set_properties, guibuilder_rename_component, guibuilder_delete_components, guibuilder_set_layout, guibuilder_set_constraint, guibuilder_set_event_handler, guibuilder_toolbar_commands, guibuilder_set_binding and guibuilder_create_form. Each mutating call is one GuiDocument transaction run unattended: a single undo step for the user, abandoned whole when any part is refused. ComponentCatalog records the properties the inspector offers per type; a test opens the inspector for every type and fails when the two disagree. Tool arguments keep explicit nulls, which reset a property or remove a handler. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01SZMcbjRgY2gZgT5nLW9RVG --- .../guibuilder/CodenameOneGUIBuilder.java | 906 ++++++++++++++++++ .../guibuilder/GuiBuilderMcpController.java | 179 ++++ .../guibuilder/model/ComponentCatalog.java | 348 +++++++ .../guibuilder/McpEditingToolsTest.java | 494 ++++++++++ 4 files changed, 1927 insertions(+) create mode 100644 scripts/guibuilder/common/src/main/java/com/codename1/guibuilder/model/ComponentCatalog.java create mode 100644 scripts/guibuilder/javase/src/test/java/com/codename1/guibuilder/McpEditingToolsTest.java diff --git a/scripts/guibuilder/common/src/main/java/com/codename1/guibuilder/CodenameOneGUIBuilder.java b/scripts/guibuilder/common/src/main/java/com/codename1/guibuilder/CodenameOneGUIBuilder.java index 52c04cea64d..b43d9193a75 100644 --- a/scripts/guibuilder/common/src/main/java/com/codename1/guibuilder/CodenameOneGUIBuilder.java +++ b/scripts/guibuilder/common/src/main/java/com/codename1/guibuilder/CodenameOneGUIBuilder.java @@ -8459,6 +8459,912 @@ else if ("phonePortrait".equals(command) || "phoneLandscape".equals(command) return null; } + // ------------------------------------------------------------------------------------------ + // MCP editing operations. + // + // Everything a client can change goes through mcpEdit(): one operation is one document + // transaction, so it is exactly one undo step for the person at the keyboard, and a failure + // part way through is abandoned outright rather than leaving half an edit behind. Validation is + // the inspector's own -- the same ranges, the same BorderLayout and table-cell rules, the same + // rename that repoints relationships -- because a client that can store what the inspector + // refuses produces forms the canvas and the generator disagree about. + // ------------------------------------------------------------------------------------------ + + /** The largest .gui fragment guibuilder_insert_xml accepts. */ + private static final int MCP_MAX_XML = 256 * 1024; + + /** One editing operation. Returns an error, or null after filling the details it reports. */ + private interface McpEditOperation { + String apply(Map details); + } + + /** + * Runs a named editing operation for an MCP client. + * + * @param operation the operation name (the tool name without its {@code guibuilder_} prefix) + * @param args the parsed tool arguments + * @return the operation's details, with {@code error} set when it was refused + */ + Map mcpEdit(String operation, Map args) { + final Map a = args == null ? new LinkedHashMap() : args; + if ("component_types".equals(operation)) return com.codename1.guibuilder.model.ComponentCatalog.describe(); + if ("get_form_xml".equals(operation)) return mcpFormXml(a); + if ("create_form".equals(operation)) { + Map details = new LinkedHashMap(); + String error = unattended(() -> mcpCreateFormImpl(a, details)); + if (error != null) details.put("error", error); + return details; + } + McpEditOperation edit; + if ("add_component".equals(operation)) edit = details -> mcpAddComponent(a, details); + else if ("insert_xml".equals(operation)) edit = details -> mcpInsertXml(a, details); + else if ("set_properties".equals(operation)) edit = details -> mcpSetProperties(a, details); + else if ("rename_component".equals(operation)) edit = details -> mcpRename(a, details); + else if ("delete_components".equals(operation)) edit = details -> mcpDelete(a, details); + else if ("set_layout".equals(operation)) edit = details -> mcpSetLayout(a, details); + else if ("set_constraint".equals(operation)) edit = details -> mcpSetConstraint(a, details); + else if ("set_event_handler".equals(operation)) edit = details -> mcpSetEventHandler(a, details); + else if ("toolbar_commands".equals(operation)) edit = details -> mcpToolbarCommands(a, details); + else if ("set_binding".equals(operation)) edit = details -> mcpSetBinding(a, details); + else { + Map unknown = new LinkedHashMap(); + unknown.put("error", "Unknown GUI Builder operation " + operation); + return unknown; + } + return mcpTransaction(operation, edit); + } + + private Map mcpTransaction(String operation, McpEditOperation edit) { + final Map details = new LinkedHashMap(); + final Element[] focus = new Element[1]; + String error = unattended(() -> { + if (document == null) return "No active GUI document; open a form first"; + if (canvasHost == null) return "The design canvas is not ready"; + finishInlineEditor(); + cancelDesignerDrag(); + document.beginTransaction(); + boolean committed = false; + try { + String refused = edit.apply(details); + if (refused != null) return refused; + committed = true; + focus[0] = document.selected(); + return null; + } finally { + if (committed) document.endTransaction(); + else document.abortTransaction(); + } + }); + // Rebuilt either way: an abandoned edit restores the document, and the canvas may already + // show part of it from a step that needed fresh bounds. + unattended(() -> { + if (document == null) return null; + if (focus[0] != null && document.containsElement(focus[0])) { + selectedElements.clear(); + if (focus[0] != document.root()) selectedElements.add(focus[0]); + document.select(focus[0]); + } + refreshEditor(); + return null; + }); + if (error != null) { + details.clear(); + details.put("error", error); + recordAction("mcp_edit_refused", "operation", operation, "error", error); + setStatus(error); + } else { + recordAction("mcp_edit", "operation", operation); + if (document != null) setStatus("Modified through MCP • " + relativeFormName(document.path())); + } + return details; + } + + private Element mcpElement(Object name) { + if (!(name instanceof String) || ((String) name).length() == 0) return null; + return findElementNamed(document, (String) name); + } + + private static String mcpString(Map args, String key) { + Object value = args.get(key); + if (value == null) return null; + if (value instanceof String) return (String) value; + if (value instanceof Number) { + double number = ((Number) value).doubleValue(); + if (number == Math.floor(number) && Math.abs(number) < 1e15) return String.valueOf((long) number); + return String.valueOf(number); + } + return String.valueOf(value); + } + + /** An integer argument, or null when absent. Throws for a value that is not a whole number. */ + private static Integer mcpInteger(Map args, String key) { + Object value = args.get(key); + if (value == null) return null; + if (value instanceof Number) { + double number = ((Number) value).doubleValue(); + if (number != Math.floor(number)) throw new IllegalArgumentException(key + " must be a whole number"); + return Integer.valueOf((int) number); + } + Integer parsed = parseInteger(String.valueOf(value).trim()); + if (parsed == null) throw new IllegalArgumentException(key + " must be a whole number"); + return parsed; + } + + @SuppressWarnings("unchecked") + private static Map mcpObject(Map args, String key) { + Object value = args.get(key); + if (value instanceof Map) return (Map) value; + if (value == null) return null; + throw new IllegalArgumentException(key + " must be an object"); + } + + private String mcpName(Element element) { + return value(element, "name", value(element, "type", "component")); + } + + private String mcpType(Element element) { + return value(element, "type", "Container"); + } + + // --- add, insert, delete, rename --------------------------------------------------------- + + private String mcpAddComponent(Map args, Map details) { + String type = mcpString(args, "type"); + if (!com.codename1.guibuilder.model.ComponentCatalog.isPaletteType(type)) { + return "type must be one of " + Arrays.asList(com.codename1.guibuilder.model.ComponentCatalog.PALETTE_TYPES); + } + Element parent = args.get("parent") == null ? document.root() : mcpElement(args.get("parent")); + if (parent == null) return "No component named " + args.get("parent"); + if (!GuiDocument.acceptsChildren(parent)) { + return mcpName(parent) + " is a " + mcpType(parent) + " and cannot hold components"; + } + if (GuiDocument.borderLayoutIsFull(parent)) { + return mcpName(parent) + " is a BorderLayout and all five regions are taken"; + } + document.select(parent); + Element added = document.addComponent(type); + if (added == null) return "Nothing was added to " + mcpName(parent); + Integer index = mcpInteger(args, "index"); + if (index != null) { + String moved = mcpMoveToIndex(added, parent, index.intValue()); + if (moved != null) return moved; + } + String requestedName = mcpString(args, "name"); + if (requestedName != null) { + document.select(added); + String applied = document.renameSelected(requestedName); + if (applied == null || applied.length() == 0) return "Invalid component name " + requestedName; + } + Map layout = mcpObject(args, "layout"); + if (layout != null) { + String refused = mcpApplyLayout(added, layout); + if (refused != null) return refused; + } + Map properties = mcpObject(args, "properties"); + if (properties != null) { + String refused = mcpApplyProperties(added, properties); + if (refused != null) return refused; + } + Map constraint = mcpObject(args, "constraint"); + if (constraint != null) { + String refused = mcpApplyConstraint(added, constraint); + if (refused != null) return refused; + } + document.select(added); + details.put("component", mcpName(added)); + details.put("parent", mcpName(document.parentOf(added))); + return null; + } + + private String mcpInsertXml(Map args, Map details) { + String xml = mcpString(args, "xml"); + if (xml == null || xml.trim().length() == 0) return "xml is required"; + if (xml.length() > MCP_MAX_XML) return "xml is larger than " + MCP_MAX_XML + " characters"; + Element parsed; + try { + parsed = new com.codename1.xml.XMLParser().parse(new com.codename1.util.regex.StringReader(xml)); + } catch (RuntimeException ex) { + return "xml could not be parsed: " + ex.getMessage(); + } + if (parsed == null || !"component".equals(parsed.getTagName())) { + return "xml must be a single element"; + } + String invalid = mcpValidateFragment(parsed); + if (invalid != null) return invalid; + Element parent = args.get("parent") == null ? document.root() : mcpElement(args.get("parent")); + if (parent == null) return "No component named " + args.get("parent"); + if (!GuiDocument.acceptsChildren(parent)) { + return mcpName(parent) + " is a " + mcpType(parent) + " and cannot hold components"; + } + if (GuiDocument.borderLayoutIsFull(parent)) { + return mcpName(parent) + " is a BorderLayout and all five regions are taken"; + } + document.select(parent); + Element inserted = document.pasteXml(xml); + if (inserted == null) return "The fragment could not be inserted into " + mcpName(parent); + Integer index = mcpInteger(args, "index"); + if (index != null) { + String moved = mcpMoveToIndex(inserted, parent, index.intValue()); + if (moved != null) return moved; + } + if ("TableLayout".equals(value(parent, "layout", "BoxLayout"))) normalizeTableCells(parent); + if ("BorderLayout".equals(value(parent, "layout", "BoxLayout"))) { + String region = GuiDocument.effectiveBorderConstraint(parent, inserted); + for (Element sibling : componentChildren(parent)) { + if (sibling != inserted && region != null //NOPMD CompareObjectsWithEquals + && region.equals(GuiDocument.effectiveBorderConstraint(parent, sibling))) { + return "The fragment's region " + region + " is already taken in " + mcpName(parent) + + "; give it a free layoutConstraint"; + } + } + } + document.select(inserted); + List names = new ArrayList(); + for (Element element : allComponentsIn(inserted)) names.add(mcpName(element)); + details.put("component", mcpName(inserted)); + details.put("components", names); + return null; + } + + private List allComponentsIn(Element element) { + List out = new ArrayList(); + out.add(element); + for (Element child : componentChildren(element)) out.addAll(allComponentsIn(child)); + return out; + } + + /** Rejects what the canvas or the generator cannot represent before it reaches the document. */ + private String mcpValidateFragment(Element element) { + String type = element.getAttribute("type"); + if (!com.codename1.guibuilder.model.ComponentCatalog.isKnownChildType(type)) { + return "Unsupported component type " + type + "; use one of " + + Arrays.asList(com.codename1.guibuilder.model.ComponentCatalog.PALETTE_TYPES); + } + String layout = element.getAttribute("layout"); + if (layout != null && !Arrays.asList(com.codename1.guibuilder.model.ComponentCatalog.LAYOUTS).contains(layout)) { + return "Unsupported layout " + layout + " on " + value(element, "name", type); + } + if (layout != null && !GuiDocument.acceptsChildren(element)) { + return type + " cannot have a layout"; + } + if ("BorderLayout".equals(layout) && GuiDocument.componentsIn(element).size() > BORDER_LAYOUT_REGION_COUNT) { + return "A BorderLayout holds five components; " + value(element, "name", type) + " has more"; + } + for (int i = 0; i < element.getNumChildren(); i++) { + Object child = element.getChildAt(i); + if (!(child instanceof Element)) continue; + Element nested = (Element) child; + if (!"component".equals(nested.getTagName())) { + return "Only elements may appear in a fragment, found <" + nested.getTagName() + ">"; + } + if (!GuiDocument.acceptsChildren(element)) return type + " cannot hold components"; + String refused = mcpValidateFragment(nested); + if (refused != null) return refused; + } + return null; + } + + private String mcpMoveToIndex(Element element, Element parent, int index) { + if (index < 0) return "index must be zero or more"; + if ("TableLayout".equals(value(parent, "layout", "BoxLayout"))) { + return "Order in a TableLayout is its cells; set row and column with guibuilder_set_constraint instead of index"; + } + int count = componentChildren(parent).size(); + document.select(element); + if (!document.moveSelectedToParent(parent, Math.min(index, count - 1))) { + return "Could not move " + mcpName(element) + " to index " + index; + } + return null; + } + + private String mcpDelete(Map args, Map details) { + Object raw = args.get("components"); + if (!(raw instanceof List) || ((List) raw).isEmpty()) return "components must be a non-empty list of names"; + List targets = new ArrayList(); + for (Object name : (List) raw) { + Element element = mcpElement(name); + if (element == null) return "No component named " + name; + if (element == document.root()) return "The form itself cannot be deleted"; + targets.add(element); + } + List deleted = new ArrayList(); + for (Element element : targets) { + // A component inside one deleted earlier in the list went with it. + if (!document.containsElement(element)) continue; + deleted.add(mcpName(element)); + document.select(element); + if (!document.deleteSelected()) return "Could not delete " + mcpName(element); + } + details.put("deleted", deleted); + return null; + } + + private String mcpRename(Map args, Map details) { + Element element = mcpElement(args.get("component")); + if (element == null) return "No component named " + args.get("component"); + String requested = mcpString(args, "name"); + if (requested == null || requested.trim().length() == 0) return "name is required"; + String previous = mcpName(element); + document.select(element); + String applied = document.renameSelected(requested); + if (applied == null || applied.length() == 0) return "Invalid component name " + requested; + details.put("previous", previous); + details.put("component", applied); + if (!applied.equals(requested.trim())) { + details.put("note", "Renamed to " + applied + " because " + requested + " is already used or not allowed"); + } + return null; + } + + // --- properties ---------------------------------------------------------------------------- + + private String mcpSetProperties(Map args, Map details) { + Element element = mcpElement(args.get("component")); + if (element == null) return "No component named " + args.get("component"); + Map properties = mcpObject(args, "properties"); + if (properties == null || properties.isEmpty()) return "properties must be a non-empty object"; + String refused = mcpApplyProperties(element, properties); + if (refused != null) return refused; + document.select(element); + details.put("component", mcpName(element)); + return null; + } + + private String mcpApplyProperties(Element element, Map properties) { + String type = mcpType(element); + for (Map.Entry entry : properties.entrySet()) { + String key = entry.getKey(); + com.codename1.guibuilder.model.ComponentCatalog.Property property = + com.codename1.guibuilder.model.ComponentCatalog.property(type, key); + if (property == null) return mcpUnknownProperty(type, key); + Object raw = entry.getValue(); + String normalized; + if (raw == null) { + normalized = null; + } else if (com.codename1.guibuilder.model.ComponentCatalog.KIND_BOOLEAN.equals(property.kind())) { + String text = String.valueOf(raw).trim(); + if (!"true".equalsIgnoreCase(text) && !"false".equalsIgnoreCase(text)) { + return property.name() + " must be true or false"; + } + normalized = String.valueOf("true".equalsIgnoreCase(text)); + } else if (com.codename1.guibuilder.model.ComponentCatalog.KIND_INTEGER.equals(property.kind())) { + Map holder = new LinkedHashMap(); + holder.put(property.name(), raw); + int parsed = mcpInteger(holder, property.name()).intValue(); + int maximum = property.maximum(); + if ("selectedIndex".equals(property.name())) { + maximum = Math.max(0, GuiDocument.componentsIn(element).size() - 1); + } + if (parsed < property.minimum() || parsed > maximum) { + return property.name() + " must be from " + property.minimum() + " to " + maximum; + } + normalized = String.valueOf(parsed); + } else if (com.codename1.guibuilder.model.ComponentCatalog.KIND_CHOICE.equals(property.kind())) { + String text = String.valueOf(raw).trim(); + normalized = null; + for (String choice : property.choices()) if (choice.equalsIgnoreCase(text)) normalized = choice; + if (normalized == null) return property.name() + " must be one of " + Arrays.asList(property.choices()); + } else { + normalized = raw instanceof String ? (String) raw : mcpString(properties, key); + if ("uiid".equals(property.name())) { + normalized = normalized.trim(); + for (int i = 0; i < normalized.length(); i++) { + char c = normalized.charAt(i); + if (!(c >= 'a' && c <= 'z' || c >= 'A' && c <= 'Z' || c >= '0' && c <= '9' || c == '_' || c == '-')) { + return "uiid must be a CSS selector name (letters, digits, - and _)"; + } + } + if (normalized.length() == 0) normalized = null; + } + } + document.setAttribute(element, property.name(), normalized); + } + return null; + } + + private String mcpUnknownProperty(String type, String key) { + String lower = key == null ? "" : key.toLowerCase(); + if ("name".equals(lower)) return "Rename with guibuilder_rename_component"; + if ("layout".equals(lower) || lower.startsWith("boxlayout") || lower.startsWith("gridlayout") + || lower.startsWith("tablelayout")) { + return key + " is set with guibuilder_set_layout"; + } + if ("layoutconstraint".equals(lower) || lower.startsWith("table") || lower.startsWith("layered") + || lower.startsWith("guided")) { + return key + " is set with guibuilder_set_constraint"; + } + if ("actionevent".equals(lower)) return "Event handlers are set with guibuilder_set_event_handler"; + if ("bindingstrategy".equals(lower)) return "The binding strategy is set with guibuilder_set_binding"; + List names = new ArrayList(); + for (com.codename1.guibuilder.model.ComponentCatalog.Property property + : com.codename1.guibuilder.model.ComponentCatalog.propertiesFor(type)) names.add(property.name()); + return "A " + type + " has no property " + key + "; it accepts " + names; + } + + // --- layout and constraints ---------------------------------------------------------------- + + private String mcpSetLayout(Map args, Map details) { + Element element = mcpElement(args.get("container")); + if (element == null) return "No component named " + args.get("container"); + String refused = mcpApplyLayout(element, args); + if (refused != null) return refused; + document.select(element); + details.put("component", mcpName(element)); + details.put("layout", value(element, "layout", "BoxLayout")); + return null; + } + + private String mcpApplyLayout(Element element, Map args) { + String type = mcpType(element); + if (!com.codename1.guibuilder.model.ComponentCatalog.hasSelectableLayout(type)) { + return "A " + type + " lays out its own children; its layout cannot be changed"; + } + String layout = mcpString(args, "layout"); + if (layout == null) layout = value(element, "layout", "BoxLayout"); + if (!Arrays.asList(com.codename1.guibuilder.model.ComponentCatalog.LAYOUTS).contains(layout)) { + return "layout must be one of " + Arrays.asList(com.codename1.guibuilder.model.ComponentCatalog.LAYOUTS); + } + int children = GuiDocument.componentsIn(element).size(); + if ("BorderLayout".equals(layout) && children > BORDER_LAYOUT_REGION_COUNT) { + return "A BorderLayout holds five components; " + mcpName(element) + " has " + children; + } + String axis = mcpString(args, "axis"); + Integer rows = mcpInteger(args, "rows"); + Integer columns = mcpInteger(args, "columns"); + if (axis != null && !"BoxLayout".equals(layout)) return "axis applies to BoxLayout only"; + if ((rows != null || columns != null) && !"GridLayout".equals(layout) && !"TableLayout".equals(layout)) { + return "rows and columns apply to GridLayout and TableLayout only"; + } + document.setAttribute(element, "layout", layout); + if (axis != null) { + if (!"X".equalsIgnoreCase(axis) && !"Y".equalsIgnoreCase(axis)) return "axis must be X or Y"; + document.setAttribute(element, "boxLayoutAxis", axis.toUpperCase()); + } + if ("GridLayout".equals(layout)) { + if (rows != null && (rows.intValue() < 1 || rows.intValue() > 100)) return "rows must be from 1 to 100"; + if (columns != null && (columns.intValue() < 1 || columns.intValue() > 100)) return "columns must be from 1 to 100"; + if (rows != null) document.setAttribute(element, "gridLayoutRows", String.valueOf(rows)); + if (columns != null) document.setAttribute(element, "gridLayoutColumns", String.valueOf(columns)); + } + if ("TableLayout".equals(layout)) { + if (columns != null) { + int floor = Math.max(1, occupiedTableExtent(element, "tableColumn", "tableHorizontalSpan")); + if (columns.intValue() < floor || columns.intValue() > 100) { + return "columns must be from " + floor + " to 100 (the children already occupy " + floor + ")"; + } + document.setAttribute(element, "tableLayoutColumns", String.valueOf(columns)); + } + normalizeTableCells(element); + if (rows != null) { + int floor = Math.max(1, occupiedTableExtent(element, "tableRow", "tableVerticalSpan")); + if (rows.intValue() < floor || rows.intValue() > 100) { + return "rows must be from " + floor + " to 100 (the children already occupy " + floor + ")"; + } + document.setAttribute(element, "tableLayoutRows", String.valueOf(rows)); + } + } + return null; + } + + private String mcpSetConstraint(Map args, Map details) { + Element element = mcpElement(args.get("component")); + if (element == null) return "No component named " + args.get("component"); + String refused = mcpApplyConstraint(element, args); + if (refused != null) return refused; + document.select(element); + details.put("component", mcpName(element)); + return null; + } + + private String mcpApplyConstraint(Element element, Map args) { + if (element == document.root()) return "The form itself has no parent constraint"; + Element parent = document.parentOf(element); + String layout = value(parent, "layout", "BoxLayout"); + Integer index = mcpInteger(args, "index"); + if (index != null) { + String moved = mcpMoveToIndex(element, parent, index.intValue()); + if (moved != null) return moved; + } + boolean border = args.containsKey("region"); + boolean table = args.containsKey("row") || args.containsKey("column") || args.containsKey("rowSpan") + || args.containsKey("columnSpan") || args.containsKey("columnWidth") || args.containsKey("rowHeight"); + boolean guided = args.containsKey("reference") || args.containsKey("insets") + || args.containsKey("horizontalSize") || args.containsKey("verticalSize") || args.containsKey("align"); + if (border && !"BorderLayout".equals(layout)) return "region applies to children of a BorderLayout; " + + mcpName(parent) + " is a " + layout; + if (table && !"TableLayout".equals(layout)) return "row, column and spans apply to children of a TableLayout; " + + mcpName(parent) + " is a " + layout; + if (guided && !"LayeredLayout".equals(layout)) return "reference, insets, sizes and align apply to children of a" + + " LayeredLayout; " + mcpName(parent) + " is a " + layout; + if (border) { + String refused = mcpApplyBorderRegion(element, parent, mcpString(args, "region")); + if (refused != null) return refused; + } + if (table) { + String refused = mcpApplyTableCell(element, parent, args); + if (refused != null) return refused; + } + if (guided) { + String refused = mcpApplyGuided(element, parent, args); + if (refused != null) return refused; + } + return null; + } + + private String mcpApplyBorderRegion(Element element, Element parent, String requested) { + String region = null; + for (String candidate : com.codename1.guibuilder.model.ComponentCatalog.BORDER_REGIONS) { + if (candidate.equalsIgnoreCase(requested)) region = candidate; + } + if (region == null) return "region must be one of " + + Arrays.asList(com.codename1.guibuilder.model.ComponentCatalog.BORDER_REGIONS); + // The occupant trades places, as the inspector's picker and a drop do. + Element occupant = null; + for (Element sibling : componentChildren(parent)) { + if (sibling == element) continue; //NOPMD CompareObjectsWithEquals + if (region.equals(GuiDocument.effectiveBorderConstraint(parent, sibling))) { + occupant = sibling; + break; + } + } + String vacated = GuiDocument.effectiveBorderConstraint(parent, element); + document.setAttribute(element, "layoutConstraint", region); + if (occupant != null) document.setAttribute(occupant, "layoutConstraint", vacated); + return null; + } + + private String mcpApplyTableCell(Element element, Element parent, Map args) { + Integer row = mcpInteger(args, "row"); + Integer column = mcpInteger(args, "column"); + Integer rowSpan = mcpInteger(args, "rowSpan"); + Integer columnSpan = mcpInteger(args, "columnSpan"); + Integer columnWidth = mcpInteger(args, "columnWidth"); + Integer rowHeight = mcpInteger(args, "rowHeight"); + int r = row != null ? row.intValue() : integer(element.getAttribute("tableRow"), 0); + int c = column != null ? column.intValue() : integer(element.getAttribute("tableColumn"), 0); + int rs = rowSpan != null ? rowSpan.intValue() : Math.max(1, integer(element.getAttribute("tableVerticalSpan"), 1)); + int cs = columnSpan != null ? columnSpan.intValue() : Math.max(1, integer(element.getAttribute("tableHorizontalSpan"), 1)); + int rows = Math.max(1, integer(value(parent, "tableLayoutRows", "2"), 2)); + int columns = Math.max(1, integer(value(parent, "tableLayoutColumns", "2"), 2)); + if (r < 0 || r >= rows) return "row must be from 0 to " + (rows - 1) + "; grow the table with guibuilder_set_layout"; + if (c < 0 || c >= columns) return "column must be from 0 to " + (columns - 1) + + "; grow the table with guibuilder_set_layout"; + if (rs < 1 || r + rs > rows) return "rowSpan must be from 1 to " + (rows - r); + if (cs < 1 || c + cs > columns) return "columnSpan must be from 1 to " + (columns - c); + if (!rectangleFree(parent, element, null, r, c, rs, cs)) { + return "That cell range overlaps another component in " + mcpName(parent); + } + if (columnWidth != null && (columnWidth.intValue() < -1 || columnWidth.intValue() > 100)) { + return "columnWidth must be a percentage from 0 to 100, or -1 for automatic"; + } + if (rowHeight != null && (rowHeight.intValue() < -1 || rowHeight.intValue() > 100)) { + return "rowHeight must be a percentage from 0 to 100, or -1 for automatic"; + } + document.setAttribute(element, "tableRow", String.valueOf(r)); + document.setAttribute(element, "tableColumn", String.valueOf(c)); + document.setAttribute(element, "tableVerticalSpan", rs == 1 ? null : String.valueOf(rs)); + document.setAttribute(element, "tableHorizontalSpan", cs == 1 ? null : String.valueOf(cs)); + if (columnWidth != null) document.setAttribute(element, "tableWidth", String.valueOf(columnWidth)); + if (rowHeight != null) document.setAttribute(element, "tableHeight", String.valueOf(rowHeight)); + return null; + } + + private String mcpApplyGuided(Element element, Element parent, Map args) { + if (args.containsKey("reference")) { + Object reference = args.get("reference"); + if (reference == null || "".equals(reference)) { + document.setAttribute(element, "guidedReferenceTarget", null); + } else { + Element target = namedSibling(parent, String.valueOf(reference)); + if (target == null || target == element) { //NOPMD CompareObjectsWithEquals + return "reference must name another child of " + mcpName(parent); + } + document.setAttribute(element, "guidedReferenceTarget", mcpName(target)); + } + } + String insets = mcpString(args, "insets"); + if (insets != null) { + String refused = mcpValidInsets(insets); + if (refused != null) return refused; + document.setAttribute(element, "layeredInsets", insets.trim()); + } + String horizontal = mcpString(args, "horizontalSize"); + String vertical = mcpString(args, "verticalSize"); + Object align = args.get("align"); + List actions = new ArrayList(); + if (align instanceof String) actions.add((String) align); + else if (align instanceof List) for (Object action : (List) align) actions.add(String.valueOf(action)); + else if (align != null) return "align must be an action name or a list of them"; + List known = Arrays.asList(com.codename1.guibuilder.model.ComponentCatalog.GUIDED_ALIGN_ACTIONS); + for (String action : actions) if (!known.contains(action)) return "align actions are " + known; + String[] policies = {GuidedLayoutSupport.PREFERRED, GuidedLayoutSupport.FIXED, GuidedLayoutSupport.FILL, + GuidedLayoutSupport.MATCH}; + if (horizontal != null && !Arrays.asList(policies).contains(horizontal)) { + return "horizontalSize must be one of " + Arrays.asList(policies); + } + if (vertical != null && !Arrays.asList(policies).contains(vertical)) { + return "verticalSize must be one of " + Arrays.asList(policies); + } + if (horizontal == null && vertical == null && actions.isEmpty()) return null; + // Size policies and alignment are measured from what the canvas shows, so it has to show the + // document as it now is before they run. + boolean needsReference = GuidedLayoutSupport.MATCH.equals(horizontal) || GuidedLayoutSupport.MATCH.equals(vertical); + for (String action : actions) if (action.startsWith("align") || action.startsWith("match")) needsReference = true; + if (needsReference && componentChildren(parent).size() < 2) { + return "Aligning or matching needs another component in " + mcpName(parent) + " as the reference"; + } + document.select(element); + refreshEditor(); + if (componentForElement(canvasHost, element) == null || componentForElement(canvasHost, parent) == null) { + return mcpName(element) + " is not visible on the canvas, so it cannot be measured"; + } + if (horizontal != null) { + document.select(element); + applyGuidedSizePolicy(true, horizontal); + } + if (vertical != null) { + document.select(element); + applyGuidedSizePolicy(false, vertical); + } + for (String action : actions) { + document.select(element); + applyLayeredAction(action); + } + return null; + } + + private static String mcpValidInsets(String insets) { + String[] parts = insets.trim().split(" "); + List tokens = new ArrayList(); + for (String part : parts) if (part.length() > 0) tokens.add(part); + if (tokens.size() != 4) return "insets must be four values: top right bottom left (e.g. \"12px auto auto 0px\")"; + for (String token : tokens) { + if ("auto".equals(token) || "baseline".equals(token)) continue; + String number = token; + if (number.endsWith("px") || number.endsWith("mm")) number = number.substring(0, number.length() - 2); + else if (number.endsWith("%")) number = number.substring(0, number.length() - 1); + else return "inset " + token + " must be auto, baseline, or a number with px, mm or %"; + try { + Double.parseDouble(number); + } catch (NumberFormatException ex) { + return "inset " + token + " is not a number"; + } + } + return null; + } + + // --- events, toolbar, binding ------------------------------------------------------------ + + private String mcpSetEventHandler(Map args, Map details) { + Element element = mcpElement(args.get("component")); + if (element == null) return "No component named " + args.get("component"); + String type = mcpType(element); + if (!com.codename1.guibuilder.model.ComponentCatalog.firesActionEvents(type)) { + return "A " + type + " does not fire action events; attach a handler to a Button, CheckBox," + + " RadioButton, TextField, TextArea or Slider"; + } + String handler = mcpString(args, "handler"); + if (handler == null || handler.trim().length() == 0) { + document.setAttribute(element, "actionEvent", null); + details.put("component", mcpName(element)); + details.put("handler", null); + return null; + } + handler = handler.trim(); + if (!mcpJavaIdentifier(handler)) return "handler must be a Java method name, e.g. onSignIn"; + document.setAttribute(element, "actionEvent", handler); + document.select(element); + details.put("component", mcpName(element)); + details.put("handler", handler); + details.put("signature", "protected void " + handler + "(ActionEvent event)"); + if (binding != null && binding.sourceDir() != null) { + String source = companionSourcePath(); + details.put("source", source); + boolean exists = false; + try { + exists = ProjectIO.exists(source) && ProjectIO.read(source).indexOf(" " + handler + "(ActionEvent") >= 0; + } catch (IOException ignored) { + exists = false; + } + details.put("methodExists", Boolean.valueOf(exists)); + details.put("note", exists ? "The method already exists in the companion source" + : "Saving the form adds an empty " + handler + " method to the user code region of the" + + " companion source; write its body there"); + } + return null; + } + + private static boolean mcpJavaIdentifier(String name) { + if (name.length() == 0) return false; + String keywords = " abstract assert boolean break byte case catch char class const continue default do double" + + " else enum extends final finally float for goto if implements import instanceof int interface long" + + " native new package private protected public return short static strictfp super switch synchronized" + + " this throw throws transient try void volatile while true false null _ "; + if (keywords.indexOf(" " + name + " ") >= 0) return false; + for (int i = 0; i < name.length(); i++) { + char c = name.charAt(i); + boolean letter = c >= 'a' && c <= 'z' || c >= 'A' && c <= 'Z' || c == '_' || c == '$'; + if (!letter && (i == 0 || c < '0' || c > '9')) return false; + } + return true; + } + + private String mcpToolbarCommands(Map args, Map details) { + if (!isFormLike(mcpType(document.root()))) return "Only a Form or Dialog has toolbar commands"; + String action = mcpString(args, "action"); + if (action == null) action = "list"; + List commands = document.commands(); + Integer index = mcpInteger(args, "index"); + if ("add".equals(action)) { + Element command = document.addCommand(); + String refused = mcpApplyCommand(command, args); + if (refused != null) return refused; + details.put("index", Integer.valueOf(document.commands().size() - 1)); + } else if ("update".equals(action) || "remove".equals(action)) { + if (index == null || index.intValue() < 0 || index.intValue() >= commands.size()) { + return "index must be from 0 to " + (commands.size() - 1); + } + Element command = commands.get(index.intValue()); + if ("remove".equals(action)) { + if (!document.removeCommand(command)) return "Could not remove command " + index; + } else { + String refused = mcpApplyCommand(command, args); + if (refused != null) return refused; + } + } else if (!"list".equals(action)) { + return "action must be list, add, update or remove"; + } + List out = new ArrayList(); + List current = document.commands(); + for (int i = 0; i < current.size(); i++) { + Element command = current.get(i); + Map item = new LinkedHashMap(); + item.put("index", Integer.valueOf(i)); + item.put("label", value(command, "name", "Command")); + item.put("placement", value(command, "placement", "right")); + item.put("handler", value(command, "actionEvent", null)); + out.add(item); + } + details.put("commands", out); + return null; + } + + private String mcpApplyCommand(Element command, Map args) { + String label = mcpString(args, "label"); + if (label != null) document.setCommandAttribute(command, "name", label); + String placement = mcpString(args, "placement"); + if (placement != null) { + if (!Arrays.asList(com.codename1.guibuilder.model.ComponentCatalog.COMMAND_PLACEMENTS).contains(placement)) { + return "placement must be one of " + Arrays.asList(com.codename1.guibuilder.model.ComponentCatalog.COMMAND_PLACEMENTS); + } + document.setCommandAttribute(command, "placement", placement); + } + String handler = mcpString(args, "handler"); + if (handler != null) { + if (!mcpJavaIdentifier(handler.trim())) return "handler must be a Java method name, e.g. onShare"; + document.setCommandAttribute(command, "actionEvent", handler.trim()); + } + return null; + } + + private String mcpSetBinding(Map args, Map details) { + if (!isFormLike(mcpType(document.root()))) return "Only a Form or Dialog has a binding strategy"; + String strategy = mcpString(args, "strategy"); + if (!"none".equals(strategy) && !"properties".equals(strategy) && !"bindable".equals(strategy)) { + return "strategy must be none, properties or bindable"; + } + String previous = value(document.root(), "bindingStrategy", "properties"); + details.put("strategy", strategy); + if (strategy.equals(previous)) return null; + document.setAttribute(document.root(), "bindingStrategy", strategy); + syncBindingModel(strategy); + details.put("note", status == null ? null : status.getText()); + return null; + } + + // --- read-only and form creation ------------------------------------------------------------- + + private Map mcpFormXml(Map args) { + final Map out = new LinkedHashMap(); + final Object name = args.get("component"); + if (document == null) { + out.put("error", "No active GUI document; open a form first"); + return out; + } + if (name == null) { + out.put("form", relativeFormName(document.path())); + out.put("modified", Boolean.valueOf(document.isModified())); + out.put("xml", document.toXml()); + return out; + } + Element element = mcpElement(name); + if (element == null) { + out.put("error", "No component named " + name); + return out; + } + out.put("component", mcpName(element)); + out.put("xml", new com.codename1.xml.XMLWriter(true).toXML(element)); + return out; + } + + private String mcpCreateFormImpl(Map args, Map details) { + if (binding == null || binding.guiDir() == null || binding.sourceDir() == null) { + return "No project is open"; + } + if (document != null && document.isModified()) return "The active form has unsaved changes; save it first"; + if (editorBufferIsDirty()) return "The open editor has unsaved changes; save or close that pane first"; + String className = mcpString(args, "className"); + String classError = mcpFormClassError(className); + if (classError != null) return classError; + String type = mcpString(args, "type"); + if (type == null) type = "Form"; + if (!Arrays.asList(com.codename1.guibuilder.model.ComponentCatalog.ROOT_TYPES).contains(type)) { + return "type must be one of " + Arrays.asList(com.codename1.guibuilder.model.ComponentCatalog.ROOT_TYPES); + } + String layout = mcpString(args, "layout"); + if (layout == null) layout = "BoxLayout"; + if (!Arrays.asList(com.codename1.guibuilder.model.ComponentCatalog.LAYOUTS).contains(layout)) { + return "layout must be one of " + Arrays.asList(com.codename1.guibuilder.model.ComponentCatalog.LAYOUTS); + } + String relative = className.replace('.', '/'); + String guiPath = ProjectIO.fsUrl(binding.guiDir()) + "/" + relative + ".gui"; + String sourcePath = binding.sourceDir() + "/" + relative + ".java"; + if (ProjectIO.exists(guiPath)) return "A form already exists at " + guiPath; + if (ProjectIO.exists(sourcePath)) return "A Java source already exists at " + sourcePath + + "; choose another class name so it is not overwritten"; + String simple = className.substring(className.lastIndexOf('.') + 1); + StringBuilder xml = new StringBuilder("\n\n\n"); + try { + ProjectIO.write(guiPath, xml.toString()); + } catch (IOException ex) { + return "Could not write " + guiPath + ": " + ex.getMessage(); + } + guiFiles = ProjectIO.findGuiFiles(binding.guiDir()); + String opened = null; + for (String path : guiFiles) if (relativeFormName(path).equals(className)) opened = path; + if (opened == null || !openForm(opened)) return "Created " + guiPath + " but could not open it"; + refreshForms(); + // Writes the companion class, so the new form compiles and can be shown at once. + if (!save()) return "Created " + guiPath + " but could not write its companion source"; + recordAction("form_created", "form", className, "source", "mcp"); + details.put("form", className); + details.put("gui", guiPath); + details.put("source", companionSourcePath()); + details.put("usage", "new " + simple + "()" + (isFormLike(type) ? ".show()" : "")); + return null; + } + + private static String mcpFormClassError(String className) { + if (className == null || className.length() == 0) return "className is required, e.g. com.example.myapp.LoginForm"; + String[] parts = className.split("\\."); + if (parts.length < 2 || className.endsWith(".")) return "className needs a package, e.g. com.example.myapp.LoginForm"; + for (int i = 0; i < parts.length; i++) { + String part = parts[i]; + if (part.length() == 0) return "className has an empty package segment"; + boolean last = i == parts.length - 1; + char first = part.charAt(0); + if (last ? first < 'A' || first > 'Z' : first < 'a' || first > 'z') { + return last ? "The class name must start with an upper-case letter" + : "Package segment " + part + " must start with a lower-case letter"; + } + for (int j = 1; j < part.length(); j++) { + char c = part.charAt(j); + if (!(c >= 'a' && c <= 'z' || c >= 'A' && c <= 'Z' || c >= '0' && c <= '9')) { + return "className may use only letters and digits in each segment"; + } + } + if (!mcpJavaIdentifier(part)) return part + " is a Java keyword"; + } + return null; + } + private static List componentBounds(Component component) { return rect(component.getAbsoluteX(), component.getAbsoluteY(), component.getWidth(), component.getHeight()); } diff --git a/scripts/guibuilder/common/src/main/java/com/codename1/guibuilder/GuiBuilderMcpController.java b/scripts/guibuilder/common/src/main/java/com/codename1/guibuilder/GuiBuilderMcpController.java index 97e2288313b..5c8be904daf 100644 --- a/scripts/guibuilder/common/src/main/java/com/codename1/guibuilder/GuiBuilderMcpController.java +++ b/scripts/guibuilder/common/src/main/java/com/codename1/guibuilder/GuiBuilderMcpController.java @@ -150,6 +150,171 @@ void register() { return resultWithState(error[0] == null, error[0]); } })); + registerEditingTools(); + } + + /** + * The editing tools. Each is one undo step in the builder, validated exactly as the inspector + * validates the same change, and answers with the operation's details and fresh state. + */ + private void registerEditingTools() { + editTool("guibuilder_component_types", "component_types", false, + "Describes what the GUI Builder can build: every component type with the properties it accepts" + + " (kind, default, range, choices), the layouts and their constraints, guided-layout size" + + " policies and align actions, toolbar command placements and binding strategies. Read it" + + " before adding components or setting properties.", + "{\"type\":\"object\",\"properties\":{}}"); + editTool("guibuilder_get_form_xml", "get_form_xml", false, + "Returns the active form's .gui XML as it is in the editor, unsaved changes included, or one" + + " component's subtree when component is given. Attribute names are lower case.", + "{\"type\":\"object\",\"properties\":{\"component\":{\"type\":\"string\"}}}"); + editTool("guibuilder_add_component", "add_component", true, + "Adds a component, as the palette does, as one undo step. parent defaults to the form; index is" + + " the position among the parent's children (not for TableLayout). Optionally name it and" + + " set layout (for a Container: {layout, axis, rows, columns}), properties (see" + + " guibuilder_component_types) and constraint (as guibuilder_set_constraint takes) in the" + + " same step. A new Container defaults to LayeredLayout. Returns the component's name.", + "{\"type\":\"object\",\"properties\":{" + + "\"type\":{\"type\":\"string\",\"enum\":" + json(PALETTE) + "}," + + "\"parent\":{\"type\":\"string\"},\"index\":{\"type\":\"integer\",\"minimum\":0}," + + "\"name\":{\"type\":\"string\"},\"layout\":{\"type\":\"object\"}," + + "\"properties\":{\"type\":\"object\"},\"constraint\":{\"type\":\"object\"}}," + + "\"required\":[\"type\"]}"); + editTool("guibuilder_insert_xml", "insert_xml", true, + "Inserts a .gui fragment -- one element with any nested components -- into parent" + + " (default: the form) as one undo step. Builds a whole section in one call. Names that" + + " are taken are made unique and references inside the fragment follow; the result lists" + + " the names used. Types must be palette types; a table child gets a free cell.", + "{\"type\":\"object\",\"properties\":{\"xml\":{\"type\":\"string\"}," + + "\"parent\":{\"type\":\"string\"},\"index\":{\"type\":\"integer\",\"minimum\":0}}," + + "\"required\":[\"xml\"]}"); + editTool("guibuilder_set_properties", "set_properties", true, + "Sets inspector properties of one component as one undo step, e.g. {\"text\":\"Sign in\"," + + "\"uiid\":\"PrimaryButton\",\"enabled\":false}. null resets a property to its default." + + " Ranges and choices are checked; names, layouts, constraints, handlers and binding have" + + " their own tools.", + "{\"type\":\"object\",\"properties\":{\"component\":{\"type\":\"string\"}," + + "\"properties\":{\"type\":\"object\"}},\"required\":[\"component\",\"properties\"]}"); + editTool("guibuilder_rename_component", "rename_component", true, + "Renames a component and repoints every layout relationship that referred to it. The name" + + " becomes the generated Java field. Returns the name applied, which differs when the" + + " requested one is taken.", + "{\"type\":\"object\",\"properties\":{\"component\":{\"type\":\"string\"}," + + "\"name\":{\"type\":\"string\"}},\"required\":[\"component\",\"name\"]}"); + editTool("guibuilder_delete_components", "delete_components", true, + "Deletes components (and everything inside them) as one undo step. The form itself cannot be" + + " deleted.", + "{\"type\":\"object\",\"properties\":{\"components\":{\"type\":\"array\",\"items\":{\"type\":\"string\"}," + + "\"minItems\":1}},\"required\":[\"components\"]}"); + editTool("guibuilder_set_layout", "set_layout", true, + "Sets a container's layout: BoxLayout (axis X or Y), BorderLayout (at most five children)," + + " FlowLayout, GridLayout (rows, columns 1-100), TableLayout (rows, columns; never below the" + + " cells already used) or LayeredLayout (guided). Omit layout to change only axis, rows or" + + " columns.", + "{\"type\":\"object\",\"properties\":{\"container\":{\"type\":\"string\"}," + + "\"layout\":{\"type\":\"string\",\"enum\":" + json(LAYOUTS) + "}," + + "\"axis\":{\"type\":\"string\",\"enum\":[\"X\",\"Y\"]}," + + "\"rows\":{\"type\":\"integer\"},\"columns\":{\"type\":\"integer\"}}," + + "\"required\":[\"container\"]}"); + editTool("guibuilder_set_constraint", "set_constraint", true, + "Places a component within its parent, by the parent's layout. Any layout except TableLayout:" + + " index (order among siblings). BorderLayout: region (North, South, East, West, Center;" + + " an occupant swaps places). TableLayout: row, column, rowSpan, columnSpan, columnWidth and" + + " rowHeight (percent, -1 automatic); the cells must be free. LayeredLayout (guided):" + + " reference (sibling name, null for nearest), insets (\"top right bottom left\", each auto," + + " baseline or a number with px, mm or %), horizontalSize and verticalSize (preferred," + + " fixed, fill, match) and align (alignLeft, alignHCenter, alignRight, alignTop," + + " alignBaseline, alignBottom, matchWidth, matchHeight, fillWidth, fillHeight).", + "{\"type\":\"object\",\"properties\":{\"component\":{\"type\":\"string\"}," + + "\"index\":{\"type\":\"integer\",\"minimum\":0}," + + "\"region\":{\"type\":\"string\",\"enum\":[\"North\",\"South\",\"East\",\"West\",\"Center\"]}," + + "\"row\":{\"type\":\"integer\"},\"column\":{\"type\":\"integer\"}," + + "\"rowSpan\":{\"type\":\"integer\"},\"columnSpan\":{\"type\":\"integer\"}," + + "\"columnWidth\":{\"type\":\"integer\"},\"rowHeight\":{\"type\":\"integer\"}," + + "\"reference\":{\"type\":[\"string\",\"null\"]},\"insets\":{\"type\":\"string\"}," + + "\"horizontalSize\":{\"type\":\"string\",\"enum\":[\"preferred\",\"fixed\",\"fill\",\"match\"]}," + + "\"verticalSize\":{\"type\":\"string\",\"enum\":[\"preferred\",\"fixed\",\"fill\",\"match\"]}," + + "\"align\":{\"type\":[\"string\",\"array\"],\"items\":{\"type\":\"string\"}}}," + + "\"required\":[\"component\"]}"); + editTool("guibuilder_set_event_handler", "set_event_handler", true, + "Attaches an action handler method to a Button, CheckBox, RadioButton, TextField, TextArea or" + + " Slider (handler null removes it). Saving the form adds an empty" + + " protected void (ActionEvent event) to the companion source's user code region;" + + " the result names that file. Write the method body there.", + "{\"type\":\"object\",\"properties\":{\"component\":{\"type\":\"string\"}," + + "\"handler\":{\"type\":[\"string\",\"null\"]}},\"required\":[\"component\",\"handler\"]}"); + editTool("guibuilder_toolbar_commands", "toolbar_commands", true, + "Lists, adds, updates or removes the form's toolbar commands. A command has a label, a placement" + + " (left, right, overflow, side) and a handler method. update and remove take the index" + + " from list. Returns the commands.", + "{\"type\":\"object\",\"properties\":{" + + "\"action\":{\"type\":\"string\",\"enum\":[\"list\",\"add\",\"update\",\"remove\"]}," + + "\"index\":{\"type\":\"integer\",\"minimum\":0},\"label\":{\"type\":\"string\"}," + + "\"placement\":{\"type\":\"string\",\"enum\":[\"left\",\"right\",\"overflow\",\"side\"]}," + + "\"handler\":{\"type\":\"string\"}}}"); + editTool("guibuilder_set_binding", "set_binding", true, + "Sets the form's data binding strategy: none, properties (a PropertyBusinessObject model) or" + + " bindable (a @Bindable POJO). The
Model class is created on save when missing; an" + + " existing model is kept, never overwritten.", + "{\"type\":\"object\",\"properties\":{\"strategy\":{\"type\":\"string\"," + + "\"enum\":[\"none\",\"properties\",\"bindable\"]}},\"required\":[\"strategy\"]}"); + editTool("guibuilder_create_form", "create_form", true, + "Creates a new form (.gui plus its companion Java class), opens it and saves it so it compiles." + + " className is fully qualified, e.g. com.example.myapp.LoginForm. type: Form (default)," + + " Dialog or Container. layout defaults to BoxLayout (vertical). Refuses while the active" + + " form has unsaved changes and never overwrites an existing file.", + "{\"type\":\"object\",\"properties\":{\"className\":{\"type\":\"string\"}," + + "\"type\":{\"type\":\"string\",\"enum\":[\"Form\",\"Dialog\",\"Container\"]}," + + "\"layout\":{\"type\":\"string\",\"enum\":" + json(LAYOUTS) + "}}," + + "\"required\":[\"className\"]}"); + } + + private static final String[] PALETTE = com.codename1.guibuilder.model.ComponentCatalog.PALETTE_TYPES; + private static final String[] LAYOUTS = com.codename1.guibuilder.model.ComponentCatalog.LAYOUTS; + + private static String json(String[] values) { + StringBuilder out = new StringBuilder("["); + for (int i = 0; i < values.length; i++) { + if (i > 0) out.append(','); + out.append('"').append(values[i]).append('"'); + } + return out.append(']').toString(); + } + + private void editTool(String name, final String operation, final boolean includeState, String description, + String schema) { + MCP.addTool(new Tool(name, description, schema, new ToolHandler() { + @Override public String invoke(String argumentsJson) throws Exception { + return JSONParser.toJson(edit(operation, parseKeepingNulls(argumentsJson), includeState)); + } + })); + } + + /** + * Runs one editing operation on the EDT. + * + * @param operation the builder operation + * @param args the parsed arguments + * @param includeState whether to append fresh state, as every mutating tool does + * @return the result map: success, error, the operation's details and state + */ + Map edit(final String operation, final Map args, boolean includeState) { + final Map[] holder = new Map[1]; + runOnEdt(new Runnable() { + @Override public void run() { holder[0] = builder.mcpEdit(operation, args); } + }); + Map details = holder[0] == null ? new LinkedHashMap() : holder[0]; + Map result = new LinkedHashMap(); + Object error = details.remove("error"); + result.put("success", Boolean.valueOf(error == null)); + if (error != null) result.put("error", error); + result.putAll(details); + if (includeState) { + // The rebuild queues the overlay refresh; cross one EDT turn so state is settled. + runOnEdt(new Runnable() { @Override public void run() { } }); + result.put("state", onEdtState()); + } + return result; } void record(String kind, Map details) { @@ -223,6 +388,20 @@ private static Map parse(String json) throws Exception { return parsed == null ? new LinkedHashMap() : parsed; } + /** + * Parses arguments with explicit nulls kept. The editing tools give null a meaning -- reset a + * property, remove a handler, clear a reference -- and the default parser drops null members, + * which turned "reset text" into an empty request. + */ + private static Map parseKeepingNulls(String json) throws Exception { + if (json == null || json.length() == 0) return new LinkedHashMap(); + JSONParser parser = new JSONParser(); + parser.setIncludeNullsInstance(true); + Map parsed = parser.parseJSON(new java.io.InputStreamReader( + new java.io.ByteArrayInputStream(json.getBytes("UTF-8")), "UTF-8")); + return parsed == null ? new LinkedHashMap() : parsed; + } + private static long longValue(Object value, long fallback) { return value instanceof Number ? ((Number) value).longValue() : fallback; } diff --git a/scripts/guibuilder/common/src/main/java/com/codename1/guibuilder/model/ComponentCatalog.java b/scripts/guibuilder/common/src/main/java/com/codename1/guibuilder/model/ComponentCatalog.java new file mode 100644 index 00000000000..7ff8cbe3de3 --- /dev/null +++ b/scripts/guibuilder/common/src/main/java/com/codename1/guibuilder/model/ComponentCatalog.java @@ -0,0 +1,348 @@ +/* + * Copyright (c) 2026, Codename One and/or its affiliates. All rights reserved. + * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. + * This code is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License version 2 only, as + * published by the Free Software Foundation. Codename One designates this + * particular file as subject to the "Classpath" exception as provided + * by Oracle in the LICENSE file that accompanied this code. + * + * This code is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License + * version 2 for more details (a copy is included in the LICENSE file that + * accompanied this code). + * + * You should have received a copy of the GNU General Public License version + * 2 along with this work; if not, write to the Free Software Foundation, + * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. + * + * Please contact Codename One through http://www.codenameone.com/ if you + * need additional information or have any questions. + */ + +package com.codename1.guibuilder.model; + +import java.util.ArrayList; +import java.util.LinkedHashMap; +import java.util.List; +import java.util.Map; + +/** + * The component types the GUI Builder can place and the properties each one accepts. + * + *

This is what the inspector's Properties tab offers, written down once so that a caller who + * cannot see the inspector -- an MCP client -- is held to exactly the same set and the same + * ranges. The inspector builds its fields by hand; {@code ComponentCatalogTest} opens it for + * every type and fails when the two disagree, so a field added there without an entry here (or + * the reverse) cannot ship. + * + *

Layout, constraint, name, event and binding attributes are deliberately absent: each has a + * dedicated operation with its own validation (border regions swap, table cells must stay free, + * renames repoint relationships, the binding model may need regenerating). + */ +public final class ComponentCatalog { + /** The types the palette adds, in palette order. */ + public static final String[] PALETTE_TYPES = { + "Button", "Label", "SpanLabel", "TextField", "TextArea", "CheckBox", "RadioButton", + "Slider", "Container", "Tabs" + }; + + /** The types a .gui document may be rooted at. */ + public static final String[] ROOT_TYPES = {"Form", "Dialog", "Container"}; + + /** The layouts the inspector offers for a container. */ + public static final String[] LAYOUTS = { + "BoxLayout", "BorderLayout", "FlowLayout", "GridLayout", "TableLayout", "LayeredLayout" + }; + + /** BorderLayout regions, in the order the inspector lists them. */ + public static final String[] BORDER_REGIONS = {"North", "South", "East", "West", "Center"}; + + /** The guided (LayeredLayout) alignment and sizing actions, as the inspector's Layout tab offers them. */ + public static final String[] GUIDED_ALIGN_ACTIONS = { + "alignLeft", "alignHCenter", "alignRight", "alignTop", "alignBaseline", "alignBottom", + "matchWidth", "matchHeight", "fillWidth", "fillHeight" + }; + + /** Toolbar command placements. */ + public static final String[] COMMAND_PLACEMENTS = {"left", "right", "overflow", "side"}; + + /** What a property holds. */ + public static final String KIND_TEXT = "text"; + public static final String KIND_BOOLEAN = "boolean"; + public static final String KIND_INTEGER = "integer"; + public static final String KIND_CHOICE = "choice"; + + /** One inspector property. */ + public static final class Property { + private final String name; + private final String label; + private final String kind; + private final String defaultValue; + private final int minimum; + private final int maximum; + private final String[] choices; + + Property(String name, String label, String kind, String defaultValue, int minimum, int maximum, + String[] choices) { + this.name = name; + this.label = label; + this.kind = kind; + this.defaultValue = defaultValue; + this.minimum = minimum; + this.maximum = maximum; + this.choices = choices; + } + + /** @return the .gui attribute */ + public String name() { return name; } + + /** @return the inspector label */ + public String label() { return label; } + + /** @return one of the {@code KIND_*} constants */ + public String kind() { return kind; } + + /** @return the value in force when the attribute is absent, or null when it depends on the component */ + public String defaultValue() { return defaultValue; } + + /** @return the smallest accepted integer */ + public int minimum() { return minimum; } + + /** @return the largest accepted integer */ + public int maximum() { return maximum; } + + /** @return the accepted values of a choice */ + public String[] choices() { return choices == null ? new String[0] : choices.clone(); } + + Map describe() { + Map out = new LinkedHashMap(); + out.put("name", name); + out.put("label", label); + out.put("kind", kind); + if (defaultValue != null) out.put("default", defaultValue); + if (KIND_INTEGER.equals(kind)) { + out.put("minimum", Integer.valueOf(minimum)); + out.put("maximum", Integer.valueOf(maximum)); + } + if (choices != null) out.put("choices", list(choices)); + return out; + } + } + + private ComponentCatalog() { } + + /** + * @param type a component type + * @return true for a type that can be added to a form + */ + public static boolean isPaletteType(String type) { + return contains(PALETTE_TYPES, type); + } + + /** + * @param type a component type + * @return true for a type a .gui file may hold anywhere: the palette types, plus Accordion, + * which older forms contain and the canvas and generator still understand + */ + public static boolean isKnownChildType(String type) { + return isPaletteType(type) || "Accordion".equals(type); + } + + /** + * @param type a component type + * @return true when the type registers an action listener, so an event handler can be attached + */ + public static boolean firesActionEvents(String type) { + return "Button".equals(type) || "CheckBox".equals(type) || "RadioButton".equals(type) + || "TextField".equals(type) || "TextArea".equals(type) || "Slider".equals(type); + } + + /** + * @param type a component type + * @return true for a Form or Dialog, which carry a title, a toolbar and a binding strategy + */ + public static boolean isFormLike(String type) { + return "Form".equals(type) || "Dialog".equals(type); + } + + /** + * @param type a component type + * @return true when the layout of a container of this type can be chosen. Tabs and Accordion + * lay themselves out. + */ + public static boolean hasSelectableLayout(String type) { + return GuiDocument.acceptsChildren(typeElement(type)) && !"Tabs".equals(type) && !"Accordion".equals(type); + } + + /** + * Every property the inspector offers for a type, name excluded. + * + * @param type the component type + * @return the properties, in inspector order + */ + public static List propertiesFor(String type) { + List out = new ArrayList(); + if (isFormLike(type)) { + out.add(text("title", "Title")); + } else if (hasText(type)) { + out.add(text("text", "Text")); + } + if ("TextField".equals(type) || "TextArea".equals(type)) out.add(text("hint", "Hint")); + out.add(new Property("uiid", "UIID (CSS selector)", KIND_TEXT, null, 0, 0, null)); + out.add(bool("enabled", "Enabled", true)); + out.add(bool("visible", "Visible", true)); + out.add(bool("rtl", "Right-to-left", false)); + if (isLabelType(type) || "SpanLabel".equals(type)) out.add(integer("gap", "Icon/text gap", "2", 0, 999)); + if (isLabelType(type) || "TextField".equals(type) || "TextArea".equals(type)) { + out.add(choice("alignment", "Alignment", "left", new String[]{"left", "center", "right"})); + } + if (isLabelType(type)) out.add(bool("tickerEnabled", "Ticker when clipped", false)); + if ("Button".equals(type)) out.add(bool("toggle", "Toggle button", false)); + if ("CheckBox".equals(type) || "RadioButton".equals(type)) out.add(bool("selected", "Selected", false)); + if ("TextField".equals(type) || "TextArea".equals(type)) { + out.add(integer("columns", "Columns", null, 1, 999)); + out.add(integer("maxSize", "Maximum length", null, 0, 99999)); + out.add(bool("editable", "Editable", true)); + out.add(bool("growByContent", "Grow by content", true)); + out.add(choice("constraint", "Input constraint", "ANY", + new String[]{"ANY", "EMAILADDR", "PASSWORD", "NUMERIC", "URL"})); + } + if ("TextArea".equals(type)) out.add(integer("rows", "Rows", null, 1, 999)); + if ("Slider".equals(type)) { + out.add(integer("minValue", "Minimum", "0", -99999, 99999)); + out.add(integer("maxValue", "Maximum", "100", -99999, 99999)); + out.add(integer("progress", "Progress", null, -99999, 99999)); + out.add(bool("editable", "Editable", false)); + out.add(bool("infinite", "Infinite progress", false)); + } + if ("Tabs".equals(type)) { + // The upper bound is the last tab that exists; the caller checks it against the document. + out.add(integer("selectedIndex", "Selected tab index", "0", 0, 99999)); + out.add(choice("tabPlacement", "Tab placement", "top", new String[]{"top", "bottom", "left", "right"})); + } + if (GuiDocument.acceptsChildren(typeElement(type))) { + out.add(bool("scrollableX", "Scroll horizontally", false)); + out.add(bool("scrollableY", "Scroll vertically", GuiDocument.defaultScrollableY(type))); + } + return out; + } + + /** + * @param type the component type + * @param name the attribute, in any letter case + * @return the property, or null when the type does not offer it + */ + public static Property property(String type, String name) { + if (name == null) return null; + for (Property candidate : propertiesFor(type)) { + if (candidate.name().equalsIgnoreCase(name)) return candidate; + } + return null; + } + + /** + * The whole catalog, as data an MCP client can read. + * + * @return types, layouts, constraints and events + */ + public static Map describe() { + Map out = new LinkedHashMap(); + List types = new ArrayList(); + List all = new ArrayList(); + for (String type : PALETTE_TYPES) all.add(type); + all.add("Form"); + all.add("Dialog"); + for (String type : all) { + Map entry = new LinkedHashMap(); + entry.put("type", type); + entry.put("palette", Boolean.valueOf(isPaletteType(type))); + entry.put("root", Boolean.valueOf(contains(ROOT_TYPES, type))); + entry.put("container", Boolean.valueOf(GuiDocument.acceptsChildren(typeElement(type)))); + entry.put("selectableLayout", Boolean.valueOf(hasSelectableLayout(type))); + entry.put("actionEvents", Boolean.valueOf(firesActionEvents(type))); + List properties = new ArrayList(); + for (Property property : propertiesFor(type)) properties.add(property.describe()); + entry.put("properties", properties); + types.add(entry); + } + out.put("types", types); + Map layouts = new LinkedHashMap(); + layouts.put("BoxLayout", "axis: Y (vertical, default) or X. Children stack in document order."); + layouts.put("BorderLayout", "At most five children, one per region: " + join(BORDER_REGIONS) + + ". Set a child's region with the region constraint."); + layouts.put("FlowLayout", "Children flow left to right and wrap, in document order."); + layouts.put("GridLayout", "rows (default 1) and columns (default 2), 1 to 100 each; equal cells in document order."); + layouts.put("TableLayout", "rows and columns (default 2 each, at least what the children occupy). Each child has" + + " row, column, rowSpan, columnSpan and optional columnWidth/rowHeight percentages."); + layouts.put("LayeredLayout", "Guided (free-form) layout, the default for a new container. Children are" + + " positioned with insets and relationships: reference, horizontalSize/verticalSize" + + " (preferred, fixed, fill, match) and align actions."); + out.put("layouts", layouts); + out.put("borderRegions", list(BORDER_REGIONS)); + out.put("guidedSizePolicies", list(new String[]{"preferred", "fixed", "fill", "match"})); + out.put("guidedAlignActions", list(GUIDED_ALIGN_ACTIONS)); + out.put("commandPlacements", list(COMMAND_PLACEMENTS)); + out.put("bindingStrategies", list(new String[]{"none", "properties", "bindable"})); + out.put("notes", list(new String[]{ + "A new Container defaults to LayeredLayout; pass layout to change it.", + "Every change is one undo step and stays in memory until save.", + "Components are addressed by their unique name."})); + return out; + } + + private static boolean hasText(String type) { + return "Label".equals(type) || "SpanLabel".equals(type) || "Button".equals(type) + || "CheckBox".equals(type) || "RadioButton".equals(type) + || "TextField".equals(type) || "TextArea".equals(type); + } + + private static boolean isLabelType(String type) { + return "Label".equals(type) || "Button".equals(type) || "CheckBox".equals(type) + || "RadioButton".equals(type); + } + + private static com.codename1.xml.Element typeElement(String type) { + com.codename1.xml.Element element = new com.codename1.xml.Element("component"); + if (type != null) element.setAttribute("type", type); + return element; + } + + private static Property text(String name, String label) { + return new Property(name, label, KIND_TEXT, "", 0, 0, null); + } + + private static Property bool(String name, String label, boolean fallback) { + return new Property(name, label, KIND_BOOLEAN, String.valueOf(fallback), 0, 0, null); + } + + private static Property integer(String name, String label, String fallback, int minimum, int maximum) { + return new Property(name, label, KIND_INTEGER, fallback, minimum, maximum, null); + } + + private static Property choice(String name, String label, String fallback, String[] choices) { + return new Property(name, label, KIND_CHOICE, fallback, 0, 0, choices); + } + + private static boolean contains(String[] values, String value) { + if (value == null) return false; + for (String candidate : values) if (candidate.equals(value)) return true; + return false; + } + + private static List list(String[] values) { + List out = new ArrayList(); + for (String value : values) out.add(value); + return out; + } + + private static String join(String[] values) { + StringBuilder out = new StringBuilder(); + for (int i = 0; i < values.length; i++) { + if (i > 0) out.append(", "); + out.append(values[i]); + } + return out.toString(); + } +} diff --git a/scripts/guibuilder/javase/src/test/java/com/codename1/guibuilder/McpEditingToolsTest.java b/scripts/guibuilder/javase/src/test/java/com/codename1/guibuilder/McpEditingToolsTest.java new file mode 100644 index 00000000000..c2aec2b3720 --- /dev/null +++ b/scripts/guibuilder/javase/src/test/java/com/codename1/guibuilder/McpEditingToolsTest.java @@ -0,0 +1,494 @@ +/* + * Copyright (c) 2026, Codename One and/or its affiliates. All rights reserved. + * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. + * This code is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License version 2 only, as + * published by the Free Software Foundation. Codename One designates this + * particular file as subject to the "Classpath" exception as provided + * by Oracle in the LICENSE file that accompanied this code. + * + * This code is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License + * version 2 for more details (a copy is included in the LICENSE file that + * accompanied this code). + * + * You should have received a copy of the GNU General Public License version + * 2 along with this work; if not, write to the Free Software Foundation, + * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. + * + * Please contact Codename One through http://www.codenameone.com/ if you + * need additional information or have any questions. + */ + +package com.codename1.guibuilder; + +import com.codename1.guibuilder.model.ComponentCatalog; +import com.codename1.guibuilder.model.GuiDocument; +import com.codename1.io.JSONParser; +import com.codename1.mcp.MCP; +import com.codename1.ui.Component; +import com.codename1.ui.Container; +import com.codename1.ui.Display; +import com.codename1.ui.Tabs; +import com.codename1.xml.Element; +import java.io.StringReader; +import java.lang.reflect.Field; +import java.lang.reflect.Method; +import java.nio.charset.StandardCharsets; +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.ArrayList; +import java.util.List; +import java.util.Map; +import java.util.TreeSet; +import javax.swing.JPanel; +import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.Test; + +import static org.junit.jupiter.api.Assertions.*; + +/** + * The editing tools, driven the way a client drives them: JSON-RPC through the MCP server into a + * running builder, then checked against the document, the canvas and the files on disk. + */ +class McpEditingToolsTest { + private static final String[] EDITING_TOOLS = { + "guibuilder_component_types", "guibuilder_get_form_xml", "guibuilder_add_component", + "guibuilder_insert_xml", "guibuilder_set_properties", "guibuilder_rename_component", + "guibuilder_delete_components", "guibuilder_set_layout", "guibuilder_set_constraint", + "guibuilder_set_event_handler", "guibuilder_toolbar_commands", "guibuilder_set_binding", + "guibuilder_create_form"}; + + private static int requestId; + private Path project; + + @BeforeAll + static void initializeCodenameOneRuntime() { + if (!Display.isInitialized()) Display.init(new JPanel()); + } + + @Test + void everyEditingToolIsPublished() throws Exception { + builder(); + String tools = MCP.getServer().handleMessage("{\"jsonrpc\":\"2.0\",\"id\":1,\"method\":\"tools/list\"}"); + for (String tool : EDITING_TOOLS) assertTrue(tools.contains("\"" + tool + "\""), tool + " is not listed"); + } + + @Test + void theCatalogOffersExactlyWhatTheInspectorOffers() throws Exception { + // The catalog is what an MCP client is validated against; the inspector is what a person + // sees. A property in one and not the other is either refused for no reason or accepted and + // ignored, so the two are compared field by field for every type. + CodenameOneGUIBuilder builder = builder(); + for (String type : ComponentCatalog.PALETTE_TYPES) { + Map added = call("guibuilder_add_component", "{\"type\":\"" + type + "\"}"); + assertEquals(Boolean.TRUE, added.get("success"), type + ": " + added.get("error")); + assertEquals(expected(type), inspectorProperties(builder), type); + } + assertEquals(expected("Form"), inspectorPropertiesFor(builder, "EditForm"), "Form"); + } + + @Test + void addingAComponentWithEverythingIsOneUndoStep() throws Exception { + CodenameOneGUIBuilder builder = builder(); + Map row = call("guibuilder_add_component", "{\"type\":\"Container\",\"name\":\"buttons\"," + + "\"layout\":{\"layout\":\"BoxLayout\",\"axis\":\"X\"}}"); + assertEquals(Boolean.TRUE, row.get("success"), String.valueOf(row.get("error"))); + assertEquals("buttons", row.get("component")); + Map added = call("guibuilder_add_component", "{\"type\":\"Button\",\"parent\":\"buttons\"," + + "\"name\":\"signIn\",\"properties\":{\"text\":\"Sign in\",\"uiid\":\"PrimaryButton\",\"enabled\":false}}"); + assertEquals(Boolean.TRUE, added.get("success"), String.valueOf(added.get("error"))); + assertEquals("signIn", added.get("component")); + assertNotNull(added.get("state"), "a mutating tool answers with fresh state"); + + Element button = element(builder, "signIn"); + assertEquals("buttons", builder.documentForTest().parentOf(button).getAttribute("name")); + assertEquals("Sign in", button.getAttribute("text")); + assertEquals("PrimaryButton", button.getAttribute("uiid")); + assertEquals("false", button.getAttribute("enabled")); + assertEquals("X", element(builder, "buttons").getAttribute("boxLayoutAxis")); + assertNotNull(preview(builder, button), "the new button is not on the canvas"); + + Map undone = call("guibuilder_command", "{\"command\":\"undo\"}"); + assertEquals(Boolean.TRUE, undone.get("success")); + assertNull(element(builder, "signIn"), "one undo must remove the whole add, properties included"); + assertNotNull(element(builder, "buttons"), "and nothing before it"); + } + + @Test + void aRefusedEditChangesNothingAndRecordsNoUndoStep() throws Exception { + CodenameOneGUIBuilder builder = builder(); + call("guibuilder_add_component", "{\"type\":\"TextField\",\"name\":\"email\"}"); + String before = builder.documentForTest().toXml(); + int undoDepth = undoDepth(builder); + + Map columns = call("guibuilder_set_properties", + "{\"component\":\"email\",\"properties\":{\"hint\":\"you@example.com\",\"columns\":0}}"); + assertEquals(Boolean.FALSE, columns.get("success")); + assertTrue(String.valueOf(columns.get("error")).contains("columns"), String.valueOf(columns.get("error"))); + assertEquals(before, builder.documentForTest().toXml(), "the hint set before the bad value must be rolled back"); + assertEquals(undoDepth, undoDepth(builder), "a refused edit must not leave an undo step"); + + Map unknown = call("guibuilder_set_properties", + "{\"component\":\"email\",\"properties\":{\"layout\":\"BoxLayout\"}}"); + assertTrue(String.valueOf(unknown.get("error")).contains("guibuilder_set_layout"), + "a layout attribute must point at the tool that validates it"); + Map choice = call("guibuilder_set_properties", + "{\"component\":\"email\",\"properties\":{\"constraint\":\"email\"}}"); + assertEquals(Boolean.FALSE, choice.get("success"), "constraint takes EMAILADDR, not a guess"); + } + + @Test + void nullResetsAPropertyToItsDefault() throws Exception { + CodenameOneGUIBuilder builder = builder(); + call("guibuilder_add_component", "{\"type\":\"Label\",\"name\":\"caption\",\"properties\":{\"uiid\":\"Big\"}}"); + assertEquals("Big", element(builder, "caption").getAttribute("uiid")); + Map reset = call("guibuilder_set_properties", "{\"component\":\"caption\",\"properties\":{\"uiid\":null}}"); + assertEquals(Boolean.TRUE, reset.get("success"), String.valueOf(reset.get("error"))); + assertNull(element(builder, "caption").getAttribute("uiid")); + } + + @Test + void insertedXmlGetsUniqueNamesAndKeepsItsInternalRelationships() throws Exception { + CodenameOneGUIBuilder builder = builder(); + String fragment = "" + + "" + + "" + + ""; + Map first = call("guibuilder_insert_xml", "{\"xml\":" + quote(fragment) + "}"); + assertEquals(Boolean.TRUE, first.get("success"), String.valueOf(first.get("error"))); + assertEquals(java.util.Arrays.asList("card", "title", "subtitle"), first.get("components")); + Map second = call("guibuilder_insert_xml", "{\"xml\":" + quote(fragment) + "}"); + assertEquals(Boolean.TRUE, second.get("success"), String.valueOf(second.get("error"))); + List renamed = (List) second.get("components"); + assertFalse(renamed.contains("card"), "a second copy must not reuse taken names: " + renamed); + Element copy = element(builder, String.valueOf(renamed.get(2))); + assertEquals(renamed.get(1), copy.getAttribute("guidedReferenceTarget"), + "a reference inside the fragment must follow its renamed target"); + + Map bad = call("guibuilder_insert_xml", + "{\"xml\":" + quote("") + "}"); + assertEquals(Boolean.FALSE, bad.get("success"), "a type the canvas cannot draw must be refused"); + Map screen = call("guibuilder_insert_xml", + "{\"xml\":" + quote("") + "}"); + assertEquals(Boolean.FALSE, screen.get("success"), "a form cannot be nested in a form"); + } + + @Test + void aBorderRegionTakenByAnotherChildSwapsPlaces() throws Exception { + CodenameOneGUIBuilder builder = builder(); + call("guibuilder_add_component", "{\"type\":\"Container\",\"name\":\"frame\",\"layout\":{\"layout\":\"BorderLayout\"}}"); + call("guibuilder_add_component", "{\"type\":\"Label\",\"name\":\"top\",\"parent\":\"frame\"," + + "\"constraint\":{\"region\":\"North\"}}"); + call("guibuilder_add_component", "{\"type\":\"Label\",\"name\":\"middle\",\"parent\":\"frame\"," + + "\"constraint\":{\"region\":\"Center\"}}"); + Map swap = call("guibuilder_set_constraint", "{\"component\":\"middle\",\"region\":\"North\"}"); + assertEquals(Boolean.TRUE, swap.get("success"), String.valueOf(swap.get("error"))); + Element frame = element(builder, "frame"); + assertEquals("North", GuiDocument.effectiveBorderConstraint(frame, element(builder, "middle"))); + assertEquals("Center", GuiDocument.effectiveBorderConstraint(frame, element(builder, "top"))); + + Map wrongLayout = call("guibuilder_set_constraint", "{\"component\":\"frame\",\"row\":1}"); + assertEquals(Boolean.FALSE, wrongLayout.get("success"), "a table cell means nothing in a BoxLayout parent"); + } + + @Test + void tableCellsCannotOverlap() throws Exception { + CodenameOneGUIBuilder builder = builder(); + call("guibuilder_add_component", "{\"type\":\"Container\",\"name\":\"grid\"," + + "\"layout\":{\"layout\":\"TableLayout\",\"rows\":2,\"columns\":2}}"); + call("guibuilder_add_component", "{\"type\":\"Label\",\"name\":\"a\",\"parent\":\"grid\"}"); + call("guibuilder_add_component", "{\"type\":\"Label\",\"name\":\"b\",\"parent\":\"grid\"}"); + Map clash = call("guibuilder_set_constraint", "{\"component\":\"b\",\"row\":0,\"column\":0}"); + assertEquals(Boolean.FALSE, clash.get("success"), "two children in one cell make TableLayout throw"); + Map moved = call("guibuilder_set_constraint", "{\"component\":\"b\",\"row\":1,\"column\":0," + + "\"columnSpan\":2}"); + assertEquals(Boolean.TRUE, moved.get("success"), String.valueOf(moved.get("error"))); + assertEquals("1", element(builder, "b").getAttribute("tableRow")); + assertEquals("2", element(builder, "b").getAttribute("tableHorizontalSpan")); + Map shrink = call("guibuilder_set_layout", "{\"container\":\"grid\",\"rows\":1}"); + assertEquals(Boolean.FALSE, shrink.get("success"), "the table cannot shrink below a placed child"); + } + + @Test + void guidedAlignmentUsesTheNamedReference() throws Exception { + CodenameOneGUIBuilder builder = builder(); + call("guibuilder_add_component", "{\"type\":\"Container\",\"name\":\"free\",\"layout\":{\"layout\":\"LayeredLayout\"}}"); + call("guibuilder_add_component", "{\"type\":\"Button\",\"name\":\"anchor\",\"parent\":\"free\"," + + "\"constraint\":{\"insets\":\"20px auto auto 30px\"}}"); + Map aligned = call("guibuilder_add_component", "{\"type\":\"Button\",\"name\":\"follower\"," + + "\"parent\":\"free\",\"constraint\":{\"insets\":\"80px auto auto 90px\",\"reference\":\"anchor\"," + + "\"align\":[\"alignLeft\"]}}"); + assertEquals(Boolean.TRUE, aligned.get("success"), String.valueOf(aligned.get("error"))); + Element follower = element(builder, "follower"); + assertEquals("anchor", follower.getAttribute("guidedReferenceTarget")); + assertTrue(String.valueOf(follower.getAttribute("guidedReferences")).contains("anchor"), + "alignLeft must anchor the left edge to the reference: " + follower.getAttribute("guidedReferences")); + Map badInsets = call("guibuilder_set_constraint", "{\"component\":\"follower\",\"insets\":\"10 auto\"}"); + assertEquals(Boolean.FALSE, badInsets.get("success")); + } + + @Test + void renameRepointsReferencesAndDeleteTakesChildrenWithIt() throws Exception { + CodenameOneGUIBuilder builder = builder(); + call("guibuilder_add_component", "{\"type\":\"Container\",\"name\":\"free\",\"layout\":{\"layout\":\"LayeredLayout\"}}"); + call("guibuilder_add_component", "{\"type\":\"Label\",\"name\":\"anchor\",\"parent\":\"free\"}"); + call("guibuilder_add_component", "{\"type\":\"Label\",\"name\":\"follower\",\"parent\":\"free\"," + + "\"constraint\":{\"reference\":\"anchor\"}}"); + Map renamed = call("guibuilder_rename_component", "{\"component\":\"anchor\",\"name\":\"heading\"}"); + assertEquals("heading", renamed.get("component")); + assertEquals("heading", element(builder, "follower").getAttribute("guidedReferenceTarget")); + Map taken = call("guibuilder_rename_component", "{\"component\":\"follower\",\"name\":\"heading\"}"); + assertNotEquals("heading", taken.get("component"), "a taken name is made unique, not duplicated"); + + Map deleted = call("guibuilder_delete_components", "{\"components\":[\"free\",\"heading\"]}"); + assertEquals(Boolean.TRUE, deleted.get("success"), String.valueOf(deleted.get("error"))); + assertEquals(java.util.Collections.singletonList("free"), deleted.get("deleted"), + "a child of a deleted container goes with it rather than failing the call"); + assertNull(element(builder, "heading")); + Map root = call("guibuilder_delete_components", "{\"components\":[\"EditForm\"]}"); + assertEquals(Boolean.FALSE, root.get("success")); + } + + @Test + void anEventHandlerBecomesAStubInTheCompanionOnSave() throws Exception { + builder(); + call("guibuilder_add_component", "{\"type\":\"Button\",\"name\":\"signIn\"}"); + Map handler = call("guibuilder_set_event_handler", "{\"component\":\"signIn\",\"handler\":\"onSignIn\"}"); + assertEquals(Boolean.TRUE, handler.get("success"), String.valueOf(handler.get("error"))); + assertEquals(Boolean.FALSE, handler.get("methodExists")); + Map label = call("guibuilder_add_component", "{\"type\":\"Label\",\"name\":\"caption\"}"); + assertEquals(Boolean.TRUE, label.get("success")); + assertEquals(Boolean.FALSE, call("guibuilder_set_event_handler", + "{\"component\":\"caption\",\"handler\":\"onTap\"}").get("success"), "a Label fires no action events"); + assertEquals(Boolean.FALSE, call("guibuilder_set_event_handler", + "{\"component\":\"signIn\",\"handler\":\"on-click\"}").get("success"), "not a Java method name"); + + assertEquals(Boolean.TRUE, call("guibuilder_command", "{\"command\":\"save\"}").get("success")); + String source = new String(Files.readAllBytes(project.resolve("src/main/java/com/example/EditForm.java")), + StandardCharsets.UTF_8); + assertTrue(source.contains("void onSignIn(ActionEvent"), "save must add the handler stub:\n" + source); + Map again = call("guibuilder_set_event_handler", "{\"component\":\"signIn\",\"handler\":\"onSignIn\"}"); + assertEquals(Boolean.TRUE, again.get("methodExists")); + + Map removed = call("guibuilder_set_event_handler", "{\"component\":\"signIn\",\"handler\":null}"); + assertEquals(Boolean.TRUE, removed.get("success"), String.valueOf(removed.get("error"))); + } + + @Test + void toolbarCommandsAndBinding() throws Exception { + CodenameOneGUIBuilder builder = builder(); + Map added = call("guibuilder_toolbar_commands", + "{\"action\":\"add\",\"label\":\"Share\",\"placement\":\"right\",\"handler\":\"onShare\"}"); + assertEquals(Boolean.TRUE, added.get("success"), String.valueOf(added.get("error"))); + List commands = (List) added.get("commands"); + assertEquals(1, commands.size()); + assertEquals("Share", ((Map) commands.get(0)).get("label")); + assertEquals(Boolean.FALSE, call("guibuilder_toolbar_commands", + "{\"action\":\"update\",\"index\":0,\"placement\":\"middle\"}").get("success")); + assertEquals(Boolean.TRUE, call("guibuilder_toolbar_commands", "{\"action\":\"remove\",\"index\":0}").get("success")); + assertTrue(builder.documentForTest().commands().isEmpty()); + + Map binding = call("guibuilder_set_binding", "{\"strategy\":\"bindable\"}"); + assertEquals(Boolean.TRUE, binding.get("success"), String.valueOf(binding.get("error"))); + assertEquals("bindable", builder.documentForTest().root().getAttribute("bindingStrategy")); + assertEquals(Boolean.FALSE, call("guibuilder_set_binding", "{\"strategy\":\"magic\"}").get("success")); + } + + @Test + void createFormWritesTheFormAndACompilableCompanionAndOpensIt() throws Exception { + CodenameOneGUIBuilder builder = builder(); + Map created = call("guibuilder_create_form", "{\"className\":\"com.example.LoginForm\"}"); + assertEquals(Boolean.TRUE, created.get("success"), String.valueOf(created.get("error"))); + assertTrue(Files.exists(project.resolve("src/main/guibuilder/com/example/LoginForm.gui"))); + String source = new String(Files.readAllBytes(project.resolve("src/main/java/com/example/LoginForm.java")), + StandardCharsets.UTF_8); + assertTrue(source.contains("class LoginForm extends Form"), source); + assertTrue(builder.documentForTest().path().endsWith("LoginForm.gui"), "the new form must be the active one"); + assertEquals("none", builder.documentForTest().root().getAttribute("bindingStrategy"), + "a new form must not need a model class to compile"); + + assertEquals(Boolean.FALSE, call("guibuilder_create_form", "{\"className\":\"com.example.LoginForm\"}").get("success"), + "an existing form is never overwritten"); + assertEquals(Boolean.FALSE, call("guibuilder_create_form", "{\"className\":\"LoginForm\"}").get("success"), + "a form needs a package"); + call("guibuilder_add_component", "{\"type\":\"Label\"}"); + Map dirty = call("guibuilder_create_form", "{\"className\":\"com.example.OtherForm\"}"); + assertEquals(Boolean.FALSE, dirty.get("success"), "unsaved edits must not be discarded by opening another form"); + } + + @Test + void theFirstFormOfAProjectCanBeCreatedAndEdited() throws Exception { + // A new project has no forms, so the builder starts on its empty-project screen; an agent + // must be able to start from there. + CodenameOneGUIBuilder builder = builder(false); + assertNull(builder.documentForTest()); + assertEquals(Boolean.FALSE, call("guibuilder_add_component", "{\"type\":\"Label\"}").get("success"), + "there is nothing to add to yet"); + Map created = call("guibuilder_create_form", "{\"className\":\"com.example.FirstForm\"," + + "\"type\":\"Dialog\"}"); + assertEquals(Boolean.TRUE, created.get("success"), String.valueOf(created.get("error"))); + assertEquals("new FirstForm().show()", created.get("usage")); + Map added = call("guibuilder_add_component", "{\"type\":\"Label\",\"name\":\"hello\"}"); + assertEquals(Boolean.TRUE, added.get("success"), String.valueOf(added.get("error"))); + assertNotNull(preview(builder, element(builder, "hello"))); + } + + @Test + void getFormXmlIncludesUnsavedEdits() throws Exception { + builder(); + call("guibuilder_add_component", "{\"type\":\"Label\",\"name\":\"fresh\"}"); + Map xml = call("guibuilder_get_form_xml", "{}"); + assertEquals(Boolean.TRUE, xml.get("modified")); + assertTrue(String.valueOf(xml.get("xml")).contains("fresh")); + Map one = call("guibuilder_get_form_xml", "{\"component\":\"fresh\"}"); + assertTrue(String.valueOf(one.get("xml")).startsWith(" expected(String type) { + TreeSet names = new TreeSet(); + names.add("name"); + for (ComponentCatalog.Property property : ComponentCatalog.propertiesFor(type)) names.add(property.name()); + return names; + } + + private static TreeSet inspectorPropertiesFor(CodenameOneGUIBuilder builder, String component) throws Exception { + Map selected = call("guibuilder_select", "{\"component\":\"" + component + "\"}"); + assertEquals(Boolean.TRUE, selected.get("success"), String.valueOf(selected.get("error"))); + return inspectorProperties(builder); + } + + /** The guibuilder.property.* fields on the inspector's Properties tab for the selection. */ + private static TreeSet inspectorProperties(CodenameOneGUIBuilder builder) throws Exception { + final TreeSet names = new TreeSet(); + Display.getInstance().callSeriallyAndWait(() -> { + try { + Method refresh = CodenameOneGUIBuilder.class.getDeclaredMethod("refreshInspector"); + refresh.setAccessible(true); + refresh.invoke(builder); + Field host = CodenameOneGUIBuilder.class.getDeclaredField("inspectorHost"); + host.setAccessible(true); + Tabs tabs = findTabs((Container) host.get(builder)); + collectProperties(tabs.getTabComponentAt(0), names); + } catch (Exception ex) { + throw new RuntimeException(ex); + } + }); + return names; + } + + private static Tabs findTabs(Container root) { + for (int i = 0; i < root.getComponentCount(); i++) { + Component child = root.getComponentAt(i); + if (child instanceof Tabs) return (Tabs) child; + if (child instanceof Container) { + Tabs nested = findTabs((Container) child); + if (nested != null) return nested; + } + } + return null; + } + + private static void collectProperties(Component component, TreeSet names) { + String identifier = component.getSemantics().getIdentifier(); + if (identifier != null && identifier.startsWith("guibuilder.property.")) { + names.add(identifier.substring("guibuilder.property.".length())); + } + if (component instanceof Container) { + Container container = (Container) component; + for (int i = 0; i < container.getComponentCount(); i++) collectProperties(container.getComponentAt(i), names); + } + } + + private CodenameOneGUIBuilder builder() throws Exception { + return builder(true); + } + + private CodenameOneGUIBuilder builder(boolean withForm) throws Exception { + project = Files.createTempDirectory("guibuilder-mcp-edit"); + Path gui = project.resolve("src/main/guibuilder/com/example"); + Files.createDirectories(gui); + if (withForm) Files.write(gui.resolve("EditForm.gui"), ("\n" + + "\n" + + "\n").getBytes(StandardCharsets.UTF_8)); + Path css = project.resolve("src/main/css/theme.css"); + Files.createDirectories(css.getParent()); + Files.write(css, "Label { color: #000000; }\n".getBytes(StandardCharsets.UTF_8)); + Files.createDirectories(project.resolve("src/main/java")); + Path input = Files.createTempFile("guibuilder", ".input"); + Files.write(input, ("projectDir=" + project + "\nguiDir=" + project.resolve("src/main/guibuilder") + "\n" + + "sourceDir=" + project.resolve("src/main/java") + "\ncssFile=" + css + "\n" + + "initialForm=com.example.EditForm\n").getBytes(StandardCharsets.UTF_8)); + input.toFile().deleteOnExit(); + System.setProperty("guibuilder.input", input.toString()); + System.setProperty("guibuilder.canvasMode", "desktop"); + final CodenameOneGUIBuilder builder = new CodenameOneGUIBuilder(); + // On the EDT, as Codename One runs an application's lifecycle; see LiveWorkspaceDragTest. + Display.getInstance().callSeriallyAndWait(() -> { + builder.init(null); + builder.runApp(); + }); + settle(); + return builder; + } + + /** Calls a tool through the MCP server exactly as a client would and returns its JSON result. */ + @SuppressWarnings("unchecked") + private static Map call(String tool, String argumentsJson) throws Exception { + String response = MCP.getServer().handleMessage("{\"jsonrpc\":\"2.0\",\"id\":" + (++requestId) + + ",\"method\":\"tools/call\",\"params\":{\"name\":\"" + tool + "\",\"arguments\":" + argumentsJson + "}}"); + Map envelope = new JSONParser().parseJSON(new StringReader(response)); + Map result = (Map) envelope.get("result"); + assertNotNull(result, tool + " failed at the protocol level: " + response); + List content = (List) result.get("content"); + String text = String.valueOf(((Map) content.get(0)).get("text")); + JSONParser parser = new JSONParser(); + parser.setUseBooleanInstance(true); + parser.setIncludeNullsInstance(true); + Map parsed = parser.parseJSON(new StringReader(text)); + settle(); + return parsed; + } + + private static String quote(String text) { + return JSONParser.toJson(java.util.Collections.singletonList(text)).replaceFirst("^\\[", "").replaceFirst("\\]$", ""); + } + + private static Element element(CodenameOneGUIBuilder builder, String name) { + for (Element candidate : builder.documentForTest().components()) { + if (name.equals(candidate.getAttribute("name"))) return candidate; + } + return null; + } + + private static Component preview(CodenameOneGUIBuilder builder, Element element) { + List found = new ArrayList(); + findPreview(builder.canvasHostForTest(), element, found); + return found.isEmpty() ? null : found.get(0); + } + + private static void findPreview(Container root, Element element, List found) { + for (int i = 0; i < root.getComponentCount(); i++) { + Component child = root.getComponentAt(i); + if (child.getClientProperty("gui.element") == element) found.add(child); + if (child instanceof Container) findPreview((Container) child, element, found); + } + } + + private static int undoDepth(CodenameOneGUIBuilder builder) throws Exception { + Field undo = GuiDocument.class.getDeclaredField("undo"); + undo.setAccessible(true); + return ((List) undo.get(builder.documentForTest())).size(); + } + + private static void settle() { + for (int i = 0; i < 2; i++) { + try { Thread.sleep(50); } catch (InterruptedException e) { Thread.currentThread().interrupt(); } + Display.getInstance().callSeriallyAndWait(() -> { }); + } + } +} From 6ec5a4de606d26e7aca7862e77483bc1bc8b1dcc Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 11 Nov 2026 02:12:04 +0000 Subject: [PATCH 2/6] GUI Builder MCP: review fixes for BorderLayout inserts, new Container forms, reads - insert_xml checked BorderLayout regions after the paste, when effectiveBorderConstraint() had already moved a duplicate to a free region, so a fragment asking for a taken region landed elsewhere (or, with index, pushed the existing child out). The fragment's own layoutConstraint is now checked before it lands, index is refused in a BorderLayout (order there only reassigns regions), and a new BorderLayout child's region is written down. - create_form wrote bindingStrategy="none" only for Form and Dialog roots, so a Container root defaulted to a binding model that set_binding cannot turn off. Every root type now starts with none. - Read-only and refused calls no longer disturb the person at the keyboard. toolbar_commands list bypasses the transaction; an edit refuses while the developer is typing in place or dragging, instead of committing their text or cancelling their drag; the canvas is rebuilt and "Modified" shown only when a committed edit changed the form. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01SZMcbjRgY2gZgT5nLW9RVG --- .../guibuilder/CodenameOneGUIBuilder.java | 139 ++++++++++++------ .../guibuilder/McpEditingToolsTest.java | 71 +++++++++ 2 files changed, 168 insertions(+), 42 deletions(-) diff --git a/scripts/guibuilder/common/src/main/java/com/codename1/guibuilder/CodenameOneGUIBuilder.java b/scripts/guibuilder/common/src/main/java/com/codename1/guibuilder/CodenameOneGUIBuilder.java index b43d9193a75..be5ce29daf5 100644 --- a/scripts/guibuilder/common/src/main/java/com/codename1/guibuilder/CodenameOneGUIBuilder.java +++ b/scripts/guibuilder/common/src/main/java/com/codename1/guibuilder/CodenameOneGUIBuilder.java @@ -610,6 +610,7 @@ private boolean openForm(String path) { private void refreshEditor() { if (document == null) return; + canvasRebuilds++; TooltipManager.hideTooltip(); normalizeSelection(); cancelDesignerDrag(); @@ -8489,6 +8490,15 @@ Map mcpEdit(String operation, Map args) { final Map a = args == null ? new LinkedHashMap() : args; if ("component_types".equals(operation)) return com.codename1.guibuilder.model.ComponentCatalog.describe(); if ("get_form_xml".equals(operation)) return mcpFormXml(a); + if ("toolbar_commands".equals(operation) && (a.get("action") == null || "list".equals(a.get("action")))) { + // Read only, so it never goes near the transaction: listing must not touch the person's + // edit, the canvas or the status line. + Map details = new LinkedHashMap(); + if (document == null) details.put("error", "No active GUI document; open a form first"); + else if (!isFormLike(mcpType(document.root()))) details.put("error", "Only a Form or Dialog has toolbar commands"); + else details.put("commands", mcpCommandList()); + return details; + } if ("create_form".equals(operation)) { Map details = new LinkedHashMap(); String error = unattended(() -> mcpCreateFormImpl(a, details)); @@ -8514,20 +8524,33 @@ Map mcpEdit(String operation, Map args) { return mcpTransaction(operation, edit); } + /** Counts canvas rebuilds, so an abandoned edit knows whether the canvas showed part of it. */ + private int canvasRebuilds; + private Map mcpTransaction(String operation, McpEditOperation edit) { final Map details = new LinkedHashMap(); final Element[] focus = new Element[1]; + final String[] before = new String[1]; + final boolean[] committedEdit = new boolean[1]; + final int rebuildsBefore = canvasRebuilds; String error = unattended(() -> { if (document == null) return "No active GUI document; open a form first"; if (canvasHost == null) return "The design canvas is not ready"; - finishInlineEditor(); - cancelDesignerDrag(); + // The person at the keyboard comes first. Committing their half-typed text or cancelling + // their drag to make room for an agent's edit takes the canvas out from under them, so + // the edit waits instead. + if (inlineEditor != null || designerDragArmed || designerDragActive + || guidedResizeArmed || guidedResizeActive) { + return "The developer is editing or dragging in the GUI Builder; try again when they finish"; + } + before[0] = document.toXml(); document.beginTransaction(); boolean committed = false; try { String refused = edit.apply(details); if (refused != null) return refused; committed = true; + committedEdit[0] = true; focus[0] = document.selected(); return null; } finally { @@ -8535,26 +8558,30 @@ private Map mcpTransaction(String operation, McpEditOperation ed else document.abortTransaction(); } }); - // Rebuilt either way: an abandoned edit restores the document, and the canvas may already - // show part of it from a step that needed fresh bounds. - unattended(() -> { - if (document == null) return null; - if (focus[0] != null && document.containsElement(focus[0])) { - selectedElements.clear(); - if (focus[0] != document.root()) selectedElements.add(focus[0]); - document.select(focus[0]); - } - refreshEditor(); - return null; - }); + // An abandoned edit restores the document by re-parsing it, which can serialize attributes in + // another order; only a committed edit can have changed anything. + boolean changed = committedEdit[0] && document != null && !before[0].equals(document.toXml()); + // Rebuilt only when the form changed, or when a step that measured the canvas rebuilt it for + // an edit that was then abandoned; otherwise the canvas, selection and status are left alone. + if (changed || canvasRebuilds != rebuildsBefore) { + unattended(() -> { + if (document == null) return null; + if (changed && focus[0] != null && document.containsElement(focus[0])) { + selectedElements.clear(); + if (focus[0] != document.root()) selectedElements.add(focus[0]); + document.select(focus[0]); + } + refreshEditor(); + return null; + }); + } if (error != null) { details.clear(); details.put("error", error); recordAction("mcp_edit_refused", "operation", operation, "error", error); - setStatus(error); - } else { + } else if (changed) { recordAction("mcp_edit", "operation", operation); - if (document != null) setStatus("Modified through MCP • " + relativeFormName(document.path())); + setStatus("Modified through MCP • " + relativeFormName(document.path())); } return details; } @@ -8624,6 +8651,7 @@ private String mcpAddComponent(Map args, Map det document.select(parent); Element added = document.addComponent(type); if (added == null) return "Nothing was added to " + mcpName(parent); + mcpPinBorderRegion(parent, added); Integer index = mcpInteger(args, "index"); if (index != null) { String moved = mcpMoveToIndex(added, parent, index.intValue()); @@ -8679,6 +8707,22 @@ private String mcpInsertXml(Map args, Map detail if (GuiDocument.borderLayoutIsFull(parent)) { return mcpName(parent) + " is a BorderLayout and all five regions are taken"; } + if ("BorderLayout".equals(value(parent, "layout", "BoxLayout"))) { + // Checked against the fragment's own constraint, before it lands: once it is in the + // document, effectiveBorderConstraint() moves a duplicate to the first free region, so + // asking the siblings afterwards can never find the clash. + String requested = parsed.getAttribute("layoutConstraint"); + String region = GuiDocument.normalizeBorderConstraint(requested); + if (requested != null && requested.trim().length() > 0 && region == null) { + return "layoutConstraint must be one of " + + Arrays.asList(com.codename1.guibuilder.model.ComponentCatalog.BORDER_REGIONS); + } + if (region != null && GuiDocument.childAtBorderConstraint(parent, region, null) != null) { + return "The " + region + " region of " + mcpName(parent) + " is already taken by " + + mcpName(GuiDocument.childAtBorderConstraint(parent, region, null)) + + "; give the fragment a free layoutConstraint"; + } + } document.select(parent); Element inserted = document.pasteXml(xml); if (inserted == null) return "The fragment could not be inserted into " + mcpName(parent); @@ -8688,16 +8732,7 @@ private String mcpInsertXml(Map args, Map detail if (moved != null) return moved; } if ("TableLayout".equals(value(parent, "layout", "BoxLayout"))) normalizeTableCells(parent); - if ("BorderLayout".equals(value(parent, "layout", "BoxLayout"))) { - String region = GuiDocument.effectiveBorderConstraint(parent, inserted); - for (Element sibling : componentChildren(parent)) { - if (sibling != inserted && region != null //NOPMD CompareObjectsWithEquals - && region.equals(GuiDocument.effectiveBorderConstraint(parent, sibling))) { - return "The fragment's region " + region + " is already taken in " + mcpName(parent) - + "; give it a free layoutConstraint"; - } - } - } + mcpPinBorderRegion(parent, inserted); document.select(inserted); List names = new ArrayList(); for (Element element : allComponentsIn(inserted)) names.add(mcpName(element)); @@ -8706,6 +8741,30 @@ private String mcpInsertXml(Map args, Map detail return null; } + /** + * Gives a new BorderLayout child the region it landed in as an explicit constraint, so a later + * reorder or insertion cannot reassign it. + */ + private void mcpPinBorderRegion(Element parent, Element child) { + if (!"BorderLayout".equals(value(parent, "layout", "BoxLayout"))) return; + document.setAttribute(child, "layoutConstraint", GuiDocument.effectiveBorderConstraint(parent, child)); + } + + private List mcpCommandList() { + List out = new ArrayList(); + List current = document.commands(); + for (int i = 0; i < current.size(); i++) { + Element command = current.get(i); + Map item = new LinkedHashMap(); + item.put("index", Integer.valueOf(i)); + item.put("label", value(command, "name", "Command")); + item.put("placement", value(command, "placement", "right")); + item.put("handler", value(command, "actionEvent", null)); + out.add(item); + } + return out; + } + private List allComponentsIn(Element element) { List out = new ArrayList(); out.add(element); @@ -8749,6 +8808,11 @@ private String mcpMoveToIndex(Element element, Element parent, int index) { if ("TableLayout".equals(value(parent, "layout", "BoxLayout"))) { return "Order in a TableLayout is its cells; set row and column with guibuilder_set_constraint instead of index"; } + if ("BorderLayout".equals(value(parent, "layout", "BoxLayout"))) { + // Reordering a BorderLayout's children reassigns the regions of any without one of + // their own, silently moving a component the caller never named. + return "Order in a BorderLayout is its regions; set region with guibuilder_set_constraint instead of index"; + } int count = componentChildren(parent).size(); document.select(element); if (!document.moveSelectedToParent(parent, Math.min(index, count - 1))) { @@ -9211,21 +9275,10 @@ private String mcpToolbarCommands(Map args, Map String refused = mcpApplyCommand(command, args); if (refused != null) return refused; } - } else if (!"list".equals(action)) { + } else { return "action must be list, add, update or remove"; } - List out = new ArrayList(); - List current = document.commands(); - for (int i = 0; i < current.size(); i++) { - Element command = current.get(i); - Map item = new LinkedHashMap(); - item.put("index", Integer.valueOf(i)); - item.put("label", value(command, "name", "Command")); - item.put("placement", value(command, "placement", "right")); - item.put("handler", value(command, "actionEvent", null)); - out.add(item); - } - details.put("commands", out); + details.put("commands", mcpCommandList()); return null; } @@ -9318,8 +9371,10 @@ private String mcpCreateFormImpl(Map args, Map d if (isFormLike(type)) xml.append(" title=\"").append(simple).append('"'); xml.append(" layout=\"").append(layout).append('"'); if ("BoxLayout".equals(layout)) xml.append(" boxLayoutAxis=\"Y\""); - // No binding model unless asked for: a new form should compile on its own. - if (isFormLike(type)) xml.append(" bindingStrategy=\"none\""); + // No binding model unless asked for: a new form should compile on its own. Every root type, + // a Container included: the generator reads the strategy from any root, and set_binding only + // offers a way back for a Form or Dialog. + xml.append(" bindingStrategy=\"none\""); xml.append(">\n\n"); try { ProjectIO.write(guiPath, xml.toString()); diff --git a/scripts/guibuilder/javase/src/test/java/com/codename1/guibuilder/McpEditingToolsTest.java b/scripts/guibuilder/javase/src/test/java/com/codename1/guibuilder/McpEditingToolsTest.java index c2aec2b3720..f13a0d39372 100644 --- a/scripts/guibuilder/javase/src/test/java/com/codename1/guibuilder/McpEditingToolsTest.java +++ b/scripts/guibuilder/javase/src/test/java/com/codename1/guibuilder/McpEditingToolsTest.java @@ -193,6 +193,72 @@ void aBorderRegionTakenByAnotherChildSwapsPlaces() throws Exception { assertEquals(Boolean.FALSE, wrongLayout.get("success"), "a table cell means nothing in a BoxLayout parent"); } + @Test + void insertedXmlCannotTakeABorderRegionThatIsAlreadyUsed() throws Exception { + // effectiveBorderConstraint() moves a duplicate region to the first free one, so a clash has + // to be caught on the fragment's own constraint, before it lands. + CodenameOneGUIBuilder builder = builder(); + call("guibuilder_add_component", "{\"type\":\"Container\",\"name\":\"frame\",\"layout\":{\"layout\":\"BorderLayout\"}}"); + call("guibuilder_add_component", "{\"type\":\"Label\",\"name\":\"body\",\"parent\":\"frame\"," + + "\"constraint\":{\"region\":\"Center\"}}"); + String before = builder.documentForTest().toXml(); + Map clash = call("guibuilder_insert_xml", "{\"parent\":\"frame\",\"xml\":" + + quote("") + "}"); + assertEquals(Boolean.FALSE, clash.get("success"), "two Center children must be refused, not shuffled"); + assertTrue(String.valueOf(clash.get("error")).contains("body"), String.valueOf(clash.get("error"))); + assertEquals(before, builder.documentForTest().toXml()); + Map reorder = call("guibuilder_insert_xml", "{\"parent\":\"frame\",\"index\":0,\"xml\":" + + quote("") + "}"); + assertEquals(Boolean.FALSE, reorder.get("success"), "an index would push body out of Center"); + assertEquals("Center", GuiDocument.effectiveBorderConstraint(element(builder, "frame"), element(builder, "body"))); + + Map free = call("guibuilder_insert_xml", "{\"parent\":\"frame\",\"xml\":" + + quote("") + "}"); + assertEquals(Boolean.TRUE, free.get("success"), String.valueOf(free.get("error"))); + assertEquals("North", element(builder, "top").getAttribute("layoutConstraint"), + "a fragment without a region takes the first free one, written down so it stays there"); + assertEquals("Center", GuiDocument.effectiveBorderConstraint(element(builder, "frame"), element(builder, "body"))); + } + + @Test + void readingDoesNotDisturbThePersonAtTheKeyboard() throws Exception { + CodenameOneGUIBuilder builder = builder(); + call("guibuilder_toolbar_commands", "{\"action\":\"add\",\"label\":\"Share\"}"); + int undoDepth = undoDepth(builder); + Field status = CodenameOneGUIBuilder.class.getDeclaredField("status"); + status.setAccessible(true); + String statusBefore = ((com.codename1.ui.Label) status.get(builder)).getText(); + Field rebuilds = CodenameOneGUIBuilder.class.getDeclaredField("canvasRebuilds"); + rebuilds.setAccessible(true); + int rebuildsBefore = rebuilds.getInt(builder); + + Map listed = call("guibuilder_toolbar_commands", "{\"action\":\"list\"}"); + assertEquals(1, ((List) listed.get("commands")).size()); + assertEquals(undoDepth, undoDepth(builder)); + assertEquals(statusBefore, ((com.codename1.ui.Label) status.get(builder)).getText(), "listing modifies nothing"); + assertEquals(rebuildsBefore, rebuilds.getInt(builder), "listing must not rebuild the canvas"); + + // While the developer is typing in place, an edit waits rather than committing their text. + Field inline = CodenameOneGUIBuilder.class.getDeclaredField("inlineEditor"); + inline.setAccessible(true); + com.codename1.ui.TextField typing = new com.codename1.ui.TextField("half typ"); + inline.set(builder, typing); + try { + Map refused = call("guibuilder_add_component", "{\"type\":\"Label\"}"); + assertEquals(Boolean.FALSE, refused.get("success")); + assertTrue(String.valueOf(refused.get("error")).contains("editing"), String.valueOf(refused.get("error"))); + assertSame(typing, inline.get(builder), "the developer's editor must be left open"); + assertEquals(1, ((List) call("guibuilder_toolbar_commands", "{}").get("commands")).size(), + "reads still work while the developer types"); + } finally { + inline.set(builder, null); + } + int rebuildsBeforeRefusal = rebuilds.getInt(builder); + Map refusedEdit = call("guibuilder_set_properties", "{\"component\":\"nobody\",\"properties\":{\"text\":\"x\"}}"); + assertEquals(Boolean.FALSE, refusedEdit.get("success")); + assertEquals(rebuildsBeforeRefusal, rebuilds.getInt(builder), "a refused edit changes nothing, so nothing is redrawn"); + } + @Test void tableCellsCannotOverlap() throws Exception { CodenameOneGUIBuilder builder = builder(); @@ -313,6 +379,11 @@ void createFormWritesTheFormAndACompilableCompanionAndOpensIt() throws Exception "an existing form is never overwritten"); assertEquals(Boolean.FALSE, call("guibuilder_create_form", "{\"className\":\"LoginForm\"}").get("success"), "a form needs a package"); + Map panel = call("guibuilder_create_form", "{\"className\":\"com.example.Panel\",\"type\":\"Container\"}"); + assertEquals(Boolean.TRUE, panel.get("success"), String.valueOf(panel.get("error"))); + assertEquals("none", builder.documentForTest().root().getAttribute("bindingStrategy"), + "a Container root must not get a binding model either; set_binding cannot turn it off there"); + assertFalse(Files.exists(project.resolve("src/main/java/com/example/PanelModel.java"))); call("guibuilder_add_component", "{\"type\":\"Label\"}"); Map dirty = call("guibuilder_create_form", "{\"className\":\"com.example.OtherForm\"}"); assertEquals(Boolean.FALSE, dirty.get("success"), "unsaved edits must not be discarded by opening another form"); From 8b354a05fd64b4fdac2a24c721c4a4dd2b6da940 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 11 Nov 2026 02:35:34 +0000 Subject: [PATCH 3/6] GUI Builder MCP: keep the canvas linked after a refused edit; pin BorderLayout siblings - A refused MCP edit aborted its transaction, which re-parses the XML and replaces every element object, but the canvas was no longer rebuilt when nothing changed, so it, the selection and the inspector kept the old elements: canvas clicks did nothing and inspector edits never reached the saved file. An edit refused before changing anything now closes the transaction without the re-parse (nothing is recorded for an unchanged document) and restores the selection; one rolled back after a partial change always rebuilds the canvas. Tests assert the canvas stays bound to the document after both. - Setting a BorderLayout region wrote only the named child's constraint, so siblings whose region came from their order (or a duplicate) moved silently. Every sibling's current region is written down first. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01SZMcbjRgY2gZgT5nLW9RVG --- .../guibuilder/CodenameOneGUIBuilder.java | 35 +++++++++++++-- .../guibuilder/McpEditingToolsTest.java | 43 +++++++++++++++++++ 2 files changed, 75 insertions(+), 3 deletions(-) diff --git a/scripts/guibuilder/common/src/main/java/com/codename1/guibuilder/CodenameOneGUIBuilder.java b/scripts/guibuilder/common/src/main/java/com/codename1/guibuilder/CodenameOneGUIBuilder.java index be5ce29daf5..c022dc67993 100644 --- a/scripts/guibuilder/common/src/main/java/com/codename1/guibuilder/CodenameOneGUIBuilder.java +++ b/scripts/guibuilder/common/src/main/java/com/codename1/guibuilder/CodenameOneGUIBuilder.java @@ -8532,6 +8532,7 @@ private Map mcpTransaction(String operation, McpEditOperation ed final Element[] focus = new Element[1]; final String[] before = new String[1]; final boolean[] committedEdit = new boolean[1]; + final boolean[] rolledBack = new boolean[1]; final int rebuildsBefore = canvasRebuilds; String error = unattended(() -> { if (document == null) return "No active GUI document; open a form first"; @@ -8544,6 +8545,7 @@ private Map mcpTransaction(String operation, McpEditOperation ed return "The developer is editing or dragging in the GUI Builder; try again when they finish"; } before[0] = document.toXml(); + final Element selectedBefore = document.selected(); document.beginTransaction(); boolean committed = false; try { @@ -8554,8 +8556,19 @@ private Map mcpTransaction(String operation, McpEditOperation ed focus[0] = document.selected(); return null; } finally { - if (committed) document.endTransaction(); - else document.abortTransaction(); + if (committed) { + document.endTransaction(); + } else if (before[0].equals(document.toXml())) { + // Refused before anything changed. Closing the transaction records nothing for an + // unchanged document, and unlike abortTransaction() it does not re-parse the XML, + // which would replace every element object while the canvas, the selection and + // the inspector still hold the old ones. + document.endTransaction(); + document.select(selectedBefore); + } else { + document.abortTransaction(); + rolledBack[0] = true; + } } }); // An abandoned edit restores the document by re-parsing it, which can serialize attributes in @@ -8563,7 +8576,10 @@ private Map mcpTransaction(String operation, McpEditOperation ed boolean changed = committedEdit[0] && document != null && !before[0].equals(document.toXml()); // Rebuilt only when the form changed, or when a step that measured the canvas rebuilt it for // an edit that was then abandoned; otherwise the canvas, selection and status are left alone. - if (changed || canvasRebuilds != rebuildsBefore) { + // A rollback re-parses the document into new element objects, so the canvas has to be rebuilt + // after one even though the form is back as it was: otherwise the canvas, the selection and + // the inspector keep editing elements that are no longer in the document. + if (changed || rolledBack[0] || canvasRebuilds != rebuildsBefore) { unattended(() -> { if (document == null) return null; if (changed && focus[0] != null && document.containsElement(focus[0])) { @@ -9062,6 +9078,19 @@ private String mcpApplyBorderRegion(Element element, Element parent, String requ } if (region == null) return "region must be one of " + Arrays.asList(com.codename1.guibuilder.model.ComponentCatalog.BORDER_REGIONS); + // Siblings without a constraint of their own take a region from their order, so changing this + // child's region would silently move them. Write down where each one is now first. + // A duplicate explicit region is moved the same way, so it is pinned too. Every region is read + // before any is written, because each write can change what the later siblings resolve to. + List siblings = componentChildren(parent); + List regions = new ArrayList(); + for (Element sibling : siblings) regions.add(GuiDocument.effectiveBorderConstraint(parent, sibling)); + for (int i = 0; i < siblings.size(); i++) { + Element sibling = siblings.get(i); + if (!regions.get(i).equals(GuiDocument.normalizeBorderConstraint(sibling.getAttribute("layoutConstraint")))) { + document.setAttribute(sibling, "layoutConstraint", regions.get(i)); + } + } // The occupant trades places, as the inspector's picker and a drop do. Element occupant = null; for (Element sibling : componentChildren(parent)) { diff --git a/scripts/guibuilder/javase/src/test/java/com/codename1/guibuilder/McpEditingToolsTest.java b/scripts/guibuilder/javase/src/test/java/com/codename1/guibuilder/McpEditingToolsTest.java index f13a0d39372..8cea014b3c5 100644 --- a/scripts/guibuilder/javase/src/test/java/com/codename1/guibuilder/McpEditingToolsTest.java +++ b/scripts/guibuilder/javase/src/test/java/com/codename1/guibuilder/McpEditingToolsTest.java @@ -129,6 +129,7 @@ void aRefusedEditChangesNothingAndRecordsNoUndoStep() throws Exception { assertTrue(String.valueOf(columns.get("error")).contains("columns"), String.valueOf(columns.get("error"))); assertEquals(before, builder.documentForTest().toXml(), "the hint set before the bad value must be rolled back"); assertEquals(undoDepth, undoDepth(builder), "a refused edit must not leave an undo step"); + assertCanvasLinked(builder, "after an edit rolled back part way"); Map unknown = call("guibuilder_set_properties", "{\"component\":\"email\",\"properties\":{\"layout\":\"BoxLayout\"}}"); @@ -257,6 +258,25 @@ void readingDoesNotDisturbThePersonAtTheKeyboard() throws Exception { Map refusedEdit = call("guibuilder_set_properties", "{\"component\":\"nobody\",\"properties\":{\"text\":\"x\"}}"); assertEquals(Boolean.FALSE, refusedEdit.get("success")); assertEquals(rebuildsBeforeRefusal, rebuilds.getInt(builder), "a refused edit changes nothing, so nothing is redrawn"); + assertCanvasLinked(builder, "after an edit refused before it changed anything"); + } + + @Test + void settingOneBorderRegionMovesNoOtherChild() throws Exception { + // Children without a constraint of their own get a region from their order (Center, North, + // South, ...); changing one child's region must not reshuffle the others. + CodenameOneGUIBuilder builder = builder(); + Map frame = call("guibuilder_insert_xml", "{\"xml\":" + quote("" + + "") + "}"); + assertEquals(Boolean.TRUE, frame.get("success"), String.valueOf(frame.get("error"))); + Element parent = element(builder, "frame"); + assertEquals("North", GuiDocument.effectiveBorderConstraint(parent, element(builder, "b"))); + Map moved = call("guibuilder_set_constraint", "{\"component\":\"a\",\"region\":\"West\"}"); + assertEquals(Boolean.TRUE, moved.get("success"), String.valueOf(moved.get("error"))); + assertEquals("West", GuiDocument.effectiveBorderConstraint(parent, element(builder, "a"))); + assertEquals("North", GuiDocument.effectiveBorderConstraint(parent, element(builder, "b")), "b was not named"); + assertEquals("South", GuiDocument.effectiveBorderConstraint(parent, element(builder, "c")), "c was not named"); } @Test @@ -419,6 +439,29 @@ void getFormXmlIncludesUnsavedEdits() throws Exception { // --- helpers ------------------------------------------------------------------------------- + /** + * Every preview on the canvas must be bound to an element the document still holds; one bound to + * a replaced element ignores clicks and takes inspector edits the save never sees. + */ + private static void assertCanvasLinked(CodenameOneGUIBuilder builder, String when) throws Exception { + final List stale = new ArrayList(); + Display.getInstance().callSeriallyAndWait(() -> collectStale(builder.canvasHostForTest(), builder.documentForTest(), stale)); + assertTrue(stale.isEmpty(), "canvas previews bound to elements no longer in the document " + when + ": " + stale); + Element selected = builder.documentForTest().selected(); + assertTrue(selected == null || builder.documentForTest().containsElement(selected), "selection is stale " + when); + } + + private static void collectStale(Container root, GuiDocument document, List stale) { + for (int i = 0; i < root.getComponentCount(); i++) { + Component child = root.getComponentAt(i); + Object bound = child.getClientProperty("gui.element"); + if (bound instanceof Element && !document.containsElement((Element) bound)) { + stale.add(String.valueOf(((Element) bound).getAttribute("name"))); + } + if (child instanceof Container) collectStale((Container) child, document, stale); + } + } + private static TreeSet expected(String type) { TreeSet names = new TreeSet(); names.add("name"); From 234363c4e7ac65d8803d0a5cb8d2d1ed439516fc Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 11 Nov 2026 05:15:50 +0000 Subject: [PATCH 4/6] GUI Builder MCP: no-op edits leave the form as they found it; reject huge numbers - An edit that changed nothing -- the value already there, or refused after rewriting an unchanged attribute -- still set the document's modified flag (all isModified() has before a form's first save), so an untouched form asked to be saved and create_form/open_form refused; and it left the document's selection on the agent's target while the canvas showed the person's. Both are put back whenever nothing changed (GuiDocument.restoreModified). - mcpInteger saturated values beyond the int range instead of refusing them, and the table span checks (r + rs > rows) then overflowed and passed. Such values are refused, and the checks compare against the room left. - ComponentCatalog's Javadoc named a test that does not exist; it now names McpEditingToolsTest.theCatalogOffersExactlyWhatTheInspectorOffers. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01SZMcbjRgY2gZgT5nLW9RVG --- .../guibuilder/CodenameOneGUIBuilder.java | 23 +++++++++-- .../guibuilder/model/ComponentCatalog.java | 7 ++-- .../guibuilder/model/GuiDocument.java | 13 ++++++ .../guibuilder/McpEditingToolsTest.java | 41 +++++++++++++++++++ 4 files changed, 77 insertions(+), 7 deletions(-) diff --git a/scripts/guibuilder/common/src/main/java/com/codename1/guibuilder/CodenameOneGUIBuilder.java b/scripts/guibuilder/common/src/main/java/com/codename1/guibuilder/CodenameOneGUIBuilder.java index c022dc67993..5678e3ee9fc 100644 --- a/scripts/guibuilder/common/src/main/java/com/codename1/guibuilder/CodenameOneGUIBuilder.java +++ b/scripts/guibuilder/common/src/main/java/com/codename1/guibuilder/CodenameOneGUIBuilder.java @@ -8533,6 +8533,8 @@ private Map mcpTransaction(String operation, McpEditOperation ed final String[] before = new String[1]; final boolean[] committedEdit = new boolean[1]; final boolean[] rolledBack = new boolean[1]; + final Element[] selectedBefore = new Element[1]; + final boolean[] modifiedBefore = new boolean[1]; final int rebuildsBefore = canvasRebuilds; String error = unattended(() -> { if (document == null) return "No active GUI document; open a form first"; @@ -8545,7 +8547,8 @@ private Map mcpTransaction(String operation, McpEditOperation ed return "The developer is editing or dragging in the GUI Builder; try again when they finish"; } before[0] = document.toXml(); - final Element selectedBefore = document.selected(); + selectedBefore[0] = document.selected(); + modifiedBefore[0] = document.isModified(); document.beginTransaction(); boolean committed = false; try { @@ -8564,7 +8567,6 @@ private Map mcpTransaction(String operation, McpEditOperation ed // which would replace every element object while the canvas, the selection and // the inspector still hold the old ones. document.endTransaction(); - document.select(selectedBefore); } else { document.abortTransaction(); rolledBack[0] = true; @@ -8574,6 +8576,14 @@ private Map mcpTransaction(String operation, McpEditOperation ed // An abandoned edit restores the document by re-parsing it, which can serialize attributes in // another order; only a committed edit can have changed anything. boolean changed = committedEdit[0] && document != null && !before[0].equals(document.toXml()); + if (document != null && before[0] != null && !changed && !rolledBack[0]) { + // Nothing changed -- refused up front, or committed with the values already there. The + // tools select their target as they go, and every attribute write marks the document + // modified; put both back, or the inspector acts on a component the canvas does not show + // as selected and an untouched form asks to be saved. + document.select(selectedBefore[0]); + document.restoreModified(modifiedBefore[0]); + } // Rebuilt only when the form changed, or when a step that measured the canvas rebuilt it for // an edit that was then abandoned; otherwise the canvas, selection and status are left alone. // A rollback re-parses the document into new element objects, so the canvas has to be rebuilt @@ -8626,6 +8636,10 @@ private static Integer mcpInteger(Map args, String key) { if (value instanceof Number) { double number = ((Number) value).doubleValue(); if (number != Math.floor(number)) throw new IllegalArgumentException(key + " must be a whole number"); + // A cast saturates rather than failing, and a saturated span made the range checks overflow. + if (number > Integer.MAX_VALUE || number < Integer.MIN_VALUE) { + throw new IllegalArgumentException(key + " is out of range"); + } return Integer.valueOf((int) number); } Integer parsed = parseInteger(String.valueOf(value).trim()); @@ -9122,8 +9136,9 @@ private String mcpApplyTableCell(Element element, Element parent, Map= rows) return "row must be from 0 to " + (rows - 1) + "; grow the table with guibuilder_set_layout"; if (c < 0 || c >= columns) return "column must be from 0 to " + (columns - 1) + "; grow the table with guibuilder_set_layout"; - if (rs < 1 || r + rs > rows) return "rowSpan must be from 1 to " + (rows - r); - if (cs < 1 || c + cs > columns) return "columnSpan must be from 1 to " + (columns - c); + // Written against the room left rather than as r + rs, which overflows for a huge span. + if (rs < 1 || rs > rows - r) return "rowSpan must be from 1 to " + (rows - r); + if (cs < 1 || cs > columns - c) return "columnSpan must be from 1 to " + (columns - c); if (!rectangleFree(parent, element, null, r, c, rs, cs)) { return "That cell range overlaps another component in " + mcpName(parent); } diff --git a/scripts/guibuilder/common/src/main/java/com/codename1/guibuilder/model/ComponentCatalog.java b/scripts/guibuilder/common/src/main/java/com/codename1/guibuilder/model/ComponentCatalog.java index 7ff8cbe3de3..75ce756079c 100644 --- a/scripts/guibuilder/common/src/main/java/com/codename1/guibuilder/model/ComponentCatalog.java +++ b/scripts/guibuilder/common/src/main/java/com/codename1/guibuilder/model/ComponentCatalog.java @@ -33,9 +33,10 @@ * *

This is what the inspector's Properties tab offers, written down once so that a caller who * cannot see the inspector -- an MCP client -- is held to exactly the same set and the same - * ranges. The inspector builds its fields by hand; {@code ComponentCatalogTest} opens it for - * every type and fails when the two disagree, so a field added there without an entry here (or - * the reverse) cannot ship. + * ranges. The inspector builds its fields by hand; + * {@code McpEditingToolsTest.theCatalogOffersExactlyWhatTheInspectorOffers} (in the javase + * module) opens it for every palette type and the Form root and fails when the two disagree, so a + * field added there without an entry here (or the reverse) cannot ship. * *

Layout, constraint, name, event and binding attributes are deliberately absent: each has a * dedicated operation with its own validation (border regions swap, table cells must stay free, diff --git a/scripts/guibuilder/common/src/main/java/com/codename1/guibuilder/model/GuiDocument.java b/scripts/guibuilder/common/src/main/java/com/codename1/guibuilder/model/GuiDocument.java index 10d01b49b12..918d05a6894 100644 --- a/scripts/guibuilder/common/src/main/java/com/codename1/guibuilder/model/GuiDocument.java +++ b/scripts/guibuilder/common/src/main/java/com/codename1/guibuilder/model/GuiDocument.java @@ -81,6 +81,19 @@ public boolean isModified() { return !savedXml.equals(xmlOnly()); } + /** + * Puts the modified state back after an edit that turned out to change nothing. + * + *

Writing an attribute marks the document modified even when the value is the one it already + * had, and before the first save that flag is all isModified() has to go on -- so a no-op left a + * form that nobody edited asking to be saved, and opening another form refused. + * + * @param wasModified what isModified() answered before the edit + */ + public void restoreModified(boolean wasModified) { + modified = wasModified; + } + public void markSaved() { modified = false; savedXml = xmlOnly(); diff --git a/scripts/guibuilder/javase/src/test/java/com/codename1/guibuilder/McpEditingToolsTest.java b/scripts/guibuilder/javase/src/test/java/com/codename1/guibuilder/McpEditingToolsTest.java index 8cea014b3c5..2ddaee47f04 100644 --- a/scripts/guibuilder/javase/src/test/java/com/codename1/guibuilder/McpEditingToolsTest.java +++ b/scripts/guibuilder/javase/src/test/java/com/codename1/guibuilder/McpEditingToolsTest.java @@ -279,6 +279,47 @@ void settingOneBorderRegionMovesNoOtherChild() throws Exception { assertEquals("South", GuiDocument.effectiveBorderConstraint(parent, element(builder, "c")), "c was not named"); } + @Test + void anEditThatChangesNothingLeavesTheFormAsItWas() throws Exception { + // Freshly opened and never saved, so the document's modified flag is all isModified() has. + CodenameOneGUIBuilder builder = builder(); + assertFalse(builder.documentForTest().isModified()); + Map same = call("guibuilder_set_properties", "{\"component\":\"EditForm\",\"properties\":{\"title\":\"Edit\"}}"); + assertEquals(Boolean.TRUE, same.get("success"), String.valueOf(same.get("error"))); + assertFalse(builder.documentForTest().isModified(), "writing the value already there is not an edit"); + Map badAxis = call("guibuilder_set_layout", "{\"container\":\"EditForm\",\"axis\":\"Z\"}"); + assertEquals(Boolean.FALSE, badAxis.get("success")); + assertFalse(builder.documentForTest().isModified(), "a refusal after rewriting the same layout is not an edit"); + assertEquals(Boolean.TRUE, call("guibuilder_create_form", "{\"className\":\"com.example.Untouched\"}").get("success"), + "an untouched form must not block opening another"); + + // The person has a component selected; an agent's no-op elsewhere must not move it. + call("guibuilder_add_component", "{\"type\":\"Label\",\"name\":\"mine\"}"); + call("guibuilder_select", "{\"component\":\"mine\"}"); + Map noop = call("guibuilder_set_properties", "{\"component\":\"Untouched\",\"properties\":{\"title\":\"Untouched\"}}"); + assertEquals(Boolean.TRUE, noop.get("success"), String.valueOf(noop.get("error"))); + assertEquals("mine", builder.documentForTest().selected().getAttribute("name"), + "the inspector acts on the document's selection, so a no-op must leave it where the person put it"); + } + + @Test + void hugeSpansAreRefusedRatherThanOverflowing() throws Exception { + CodenameOneGUIBuilder builder = builder(); + call("guibuilder_add_component", "{\"type\":\"Container\",\"name\":\"grid\"," + + "\"layout\":{\"layout\":\"TableLayout\",\"rows\":2,\"columns\":2}}"); + call("guibuilder_add_component", "{\"type\":\"Label\",\"name\":\"cell\",\"parent\":\"grid\"}"); + // 1.0e10 rather than 1e10: the MCP server parses arguments with longs on, and JSONParser then + // drops an integer written with an exponent, so 1e10 never reaches the tool at all. + for (String span : new String[]{"2147483647", "1.0e10", "99999999999", "\"99999999999\""}) { + Map result = call("guibuilder_set_constraint", "{\"component\":\"cell\",\"row\":1,\"rowSpan\":" + span + "}"); + assertEquals(Boolean.FALSE, result.get("success"), "rowSpan " + span + " must be refused"); + Map columns = call("guibuilder_set_constraint", "{\"component\":\"cell\",\"column\":1,\"columnSpan\":" + span + "}"); + assertEquals(Boolean.FALSE, columns.get("success"), "columnSpan " + span + " must be refused"); + } + assertNull(element(builder, "cell").getAttribute("tableVerticalSpan")); + assertNull(element(builder, "cell").getAttribute("tableHorizontalSpan")); + } + @Test void tableCellsCannotOverlap() throws Exception { CodenameOneGUIBuilder builder = builder(); From bb5ced85e50d31a9da9de817a5896a5b7d208b7c Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 11 Nov 2026 06:30:22 +0000 Subject: [PATCH 5/6] GUI Builder MCP: red-team and review fixes Review (shai-almog): - create_form now refuses while the developer is typing inline or dragging, like every other editing tool. - Tables inside an insert_xml fragment are checked: overlapping cells, cells outside the table and spans past its edge are refused, and children without a cell get a free one. - set_binding reports only what syncBindingModel said, and a meaningful status line is no longer overwritten by "Modified through MCP". Adversarial review: - An edit waits while a modal dialog is open (the developer's Save or form-switch question runs a nested event loop; an edit landing there was marked saved but never written), while an inspector text field has focus, and while a Code, Model or CSS pane is open. - insets are validated by LayeredLayout itself, as the inspector does (baseline only on top, whole px, no infinity). - insert_xml validates and normalizes every attribute the canvas or the generator reads: inspector properties (written back in canonical form, so enabled="TRUE" or constraint="password" no longer diverge between canvas and app), axis, cells and spans on every element, layeredInsets, guided anchors, positions, references, size policies and preferred sizes (several of which the generator emits into source as written). - Pasted names are sanitized like renames. - If the canvas cannot show a committed edit, the edit is undone and reported as an error instead of success. - A rollback restores the modified state; toolbar commands are written under canonical keys so a re-parse does not read as a change. - Handler names reject the identifiers the generator renames (record, var...). - The catalog reports the sample text a reset shows. - guibuilder_state reports editorDirty and developerBusy for clients deciding whether they may close the builder. - Existing bug the new safety net exposed: an empty container switched to BorderLayout threw on every canvas rebuild (its "Drop here" hint was added without a region), from the inspector too. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01SZMcbjRgY2gZgT5nLW9RVG --- .../guibuilder/CodenameOneGUIBuilder.java | 377 +++++++++++++++--- .../guibuilder/model/ComponentCatalog.java | 10 +- .../guibuilder/model/GuiDocument.java | 16 +- .../ui/ComponentPreviewFactory.java | 11 +- .../guibuilder/McpEditingToolsTest.java | 141 ++++++- 5 files changed, 494 insertions(+), 61 deletions(-) diff --git a/scripts/guibuilder/common/src/main/java/com/codename1/guibuilder/CodenameOneGUIBuilder.java b/scripts/guibuilder/common/src/main/java/com/codename1/guibuilder/CodenameOneGUIBuilder.java index 5678e3ee9fc..b430d431019 100644 --- a/scripts/guibuilder/common/src/main/java/com/codename1/guibuilder/CodenameOneGUIBuilder.java +++ b/scripts/guibuilder/common/src/main/java/com/codename1/guibuilder/CodenameOneGUIBuilder.java @@ -8172,6 +8172,11 @@ Map mcpState(long latestSequence) { out.put("canvasMode", canvasMode); out.put("darkMode", Boolean.valueOf(darkMode)); out.put("status", status == null ? null : status.getText()); + // Work the form's modified flag cannot see: text in an open Code, Model or CSS pane is in an + // editor buffer, and a half-typed inline edit or a drag is not in the document at all. A + // client deciding whether it may close or switch away has to know about both. + out.put("editorDirty", Boolean.valueOf(editorBufferIsDirty())); + out.put("developerBusy", Boolean.valueOf(mcpDeveloperBusy() != null)); List forms = new ArrayList(); for (String path : guiFiles) forms.add(relativeFormName(path)); out.put("forms", forms); @@ -8535,18 +8540,15 @@ private Map mcpTransaction(String operation, McpEditOperation ed final boolean[] rolledBack = new boolean[1]; final Element[] selectedBefore = new Element[1]; final boolean[] modifiedBefore = new boolean[1]; + final String[] statusBefore = new String[1]; final int rebuildsBefore = canvasRebuilds; String error = unattended(() -> { if (document == null) return "No active GUI document; open a form first"; if (canvasHost == null) return "The design canvas is not ready"; - // The person at the keyboard comes first. Committing their half-typed text or cancelling - // their drag to make room for an agent's edit takes the canvas out from under them, so - // the edit waits instead. - if (inlineEditor != null || designerDragArmed || designerDragActive - || guidedResizeArmed || guidedResizeActive) { - return "The developer is editing or dragging in the GUI Builder; try again when they finish"; - } + String busy = mcpDeveloperBusy(); + if (busy != null) return busy; before[0] = document.toXml(); + statusBefore[0] = status == null ? null : status.getText(); selectedBefore[0] = document.selected(); modifiedBefore[0] = document.isModified(); document.beginTransaction(); @@ -8589,8 +8591,10 @@ private Map mcpTransaction(String operation, McpEditOperation ed // A rollback re-parses the document into new element objects, so the canvas has to be rebuilt // after one even though the form is back as it was: otherwise the canvas, the selection and // the inspector keep editing elements that are no longer in the document. + if (rolledBack[0]) document.restoreModified(modifiedBefore[0]); + String rebuildFailure = null; if (changed || rolledBack[0] || canvasRebuilds != rebuildsBefore) { - unattended(() -> { + rebuildFailure = unattended(() -> { if (document == null) return null; if (changed && focus[0] != null && document.containsElement(focus[0])) { selectedElements.clear(); @@ -8601,17 +8605,70 @@ private Map mcpTransaction(String operation, McpEditOperation ed return null; }); } + if (rebuildFailure != null && changed && error == null) { + // The edit reached the document but the canvas cannot show it, and the generated source + // would fail the same way. Take it back out rather than report it as done. + final String failure = rebuildFailure; + unattended(() -> { + if (document.undo()) refreshEditor(); + return null; + }); + error = "The canvas could not show that change, so it was undone: " + failure; + } if (error != null) { details.clear(); details.put("error", error); recordAction("mcp_edit_refused", "operation", operation, "error", error); } else if (changed) { recordAction("mcp_edit", "operation", operation); - setStatus("Modified through MCP • " + relativeFormName(document.path())); + // An edit that reported something of its own on the status line (the binding model it will + // write, say) keeps it; the generic line would hide the one thing the developer needs. + String now = status == null ? null : status.getText(); + if (now == null || now.equals(statusBefore[0])) { + setStatus("Modified through MCP • " + relativeFormName(document.path())); + } } return details; } + /** + * Why an agent's change has to wait, or null when it need not. + * + *

The person at the keyboard comes first. Committing their half-typed text or cancelling their + * drag to make room for an agent's edit takes the canvas out from under them -- and half-typed + * inline text is not in the document yet, so no unsaved-changes check can see it. + */ + private String mcpDeveloperBusy() { + if (inlineEditor != null || designerDragArmed || designerDragActive + || guidedResizeArmed || guidedResizeActive) { + return "The developer is editing or dragging in the GUI Builder; try again when they finish"; + } + // A modal question (Save before switching? Regenerate the model?) runs a nested event loop, + // so an MCP call would land while the developer's save or switch is half done -- and save() + // marks whatever the document then holds as saved, though it wrote the earlier XML. + Form current = Display.getInstance().getCurrent(); + if (current instanceof com.codename1.ui.Dialog) { + return "The GUI Builder is waiting for the developer to answer a dialog; try again when they have"; + } + // Rebuilding the canvas rebuilds the inspector and any open Code, Model or CSS pane: a field + // the developer is typing in commits on the focus loss, and the pane loses its caret. + Component focused = current == null ? null : current.getFocused(); + if (focused instanceof TextArea && inspectorHost != null && isInside(focused, inspectorHost)) { + return "The developer is typing in the inspector; try again when they finish"; + } + if (activeCodeEditor != null) { + return "A Code, Model or CSS pane is open in the GUI Builder; ask the developer to close it first"; + } + return null; + } + + private static boolean isInside(Component component, Container ancestor) { + for (Container parent = component.getParent(); parent != null; parent = parent.getParent()) { + if (parent == ancestor) return true; //NOPMD CompareObjectsWithEquals + } + return false; + } + private Element mcpElement(Object name) { if (!(name instanceof String) || ((String) name).length() == 0) return null; return findElementNamed(document, (String) name); @@ -8754,7 +8811,8 @@ private String mcpInsertXml(Map args, Map detail } } document.select(parent); - Element inserted = document.pasteXml(xml); + // The fragment as validated and normalized, not the text as sent. + Element inserted = document.pasteXml(new com.codename1.xml.XMLWriter(true).toXML(parsed)); if (inserted == null) return "The fragment could not be inserted into " + mcpName(parent); Integer index = mcpInteger(args, "index"); if (index != null) { @@ -8762,6 +8820,9 @@ private String mcpInsertXml(Map args, Map detail if (moved != null) return moved; } if ("TableLayout".equals(value(parent, "layout", "BoxLayout"))) normalizeTableCells(parent); + for (Element nested : allComponentsIn(inserted)) { + if ("TableLayout".equals(nested.getAttribute("layout"))) normalizeTableCells(nested); + } mcpPinBorderRegion(parent, inserted); document.select(inserted); List names = new ArrayList(); @@ -8809,9 +8870,38 @@ private String mcpValidateFragment(Element element) { return "Unsupported component type " + type + "; use one of " + Arrays.asList(com.codename1.guibuilder.model.ComponentCatalog.PALETTE_TYPES); } + String label = value(element, "name", type); String layout = element.getAttribute("layout"); if (layout != null && !Arrays.asList(com.codename1.guibuilder.model.ComponentCatalog.LAYOUTS).contains(layout)) { - return "Unsupported layout " + layout + " on " + value(element, "name", type); + return "Unsupported layout " + layout + " on " + label; + } + // Every attribute the canvas or the generator reads is held to what the matching tool would + // accept, and stored in the form they expect: both compare booleans and choices exactly and + // emit several numbers into the generated source as they stand. + String attributeError = mcpNormalizeFragmentAttributes(element, type, label); + if (attributeError != null) return attributeError; + String handler = element.getAttribute("actionEvent"); + if (handler != null && handler.trim().length() > 0 && !"true".equals(handler.trim()) + && !mcpJavaIdentifier(handler.trim())) { + return label + ": actionEvent must be a Java method name"; + } + String axis = element.getAttribute("boxLayoutAxis"); + if (axis != null && !"X".equalsIgnoreCase(axis.trim()) && !"Y".equalsIgnoreCase(axis.trim())) { + return label + ": boxLayoutAxis must be X or Y"; + } + if (axis != null) mcpPutAttribute(element, "boxLayoutAxis", axis.trim().toUpperCase()); + String[] sizes = {"gridLayoutRows", "gridLayoutColumns", "tableLayoutRows", "tableLayoutColumns"}; + for (String size : sizes) { + String raw = element.getAttribute(size); + if (raw == null) continue; + Integer parsed = parseInteger(raw.trim()); + if (parsed == null || parsed.intValue() < 1 || parsed.intValue() > 100) { + return label + ": " + size + " must be from 1 to 100"; + } + } + if ("TableLayout".equals(layout)) { + String cells = mcpValidateFragmentTable(element); + if (cells != null) return cells; } if (layout != null && !GuiDocument.acceptsChildren(element)) { return type + " cannot have a layout"; @@ -8833,6 +8923,147 @@ private String mcpValidateFragment(Element element) { return null; } + /** + * Checks the cells a fragment's own TableLayout container gives its children. TableLayout throws + * on two children in one cell ("already occupied") or a span past the edge ("Illegal span"), on + * the canvas and in the generated app alike, so neither may reach the document. + */ + private String mcpValidateFragmentTable(Element table) { + int rows = Math.max(1, integer(table.getAttribute("tableLayoutRows"), 2)); + int columns = Math.max(1, integer(table.getAttribute("tableLayoutColumns"), 2)); + String label = value(table, "name", "TableLayout container"); + java.util.Set taken = new LinkedHashSet(); + for (Element child : GuiDocument.componentsIn(table)) { + String childLabel = value(child, "name", value(child, "type", "component")); + String rawRow = child.getAttribute("tableRow"); + String rawColumn = child.getAttribute("tableColumn"); + Integer rowSpan = parseOptionalCount(child.getAttribute("tableVerticalSpan")); + Integer columnSpan = parseOptionalCount(child.getAttribute("tableHorizontalSpan")); + if (rowSpan == null || columnSpan == null) return childLabel + ": table spans must be whole numbers from 1"; + if (rawRow == null && rawColumn == null) { + // No cell: one is assigned after the insert, but its span still has to fit the table. + if (rowSpan.intValue() > rows || columnSpan.intValue() > columns) { + return childLabel + ": its span does not fit " + label + " (" + rows + " x " + columns + ")"; + } + continue; + } + Integer row = rawRow == null ? null : parseInteger(rawRow.trim()); + Integer column = rawColumn == null ? null : parseInteger(rawColumn.trim()); + if (row == null || column == null) return childLabel + ": give both tableRow and tableColumn, as whole numbers"; + int r = row.intValue(); + int c = column.intValue(); + if (r < 0 || r >= rows || c < 0 || c >= columns) { + return childLabel + ": cell " + r + "," + c + " is outside " + label + " (" + rows + " x " + columns + ")"; + } + if (rowSpan.intValue() > rows - r || columnSpan.intValue() > columns - c) { + return childLabel + ": its span runs past the edge of " + label; + } + for (int i = r; i < r + rowSpan.intValue(); i++) { + for (int j = c; j < c + columnSpan.intValue(); j++) { + if (!taken.add(i + ":" + j)) return childLabel + ": cell " + i + "," + j + " of " + label + " is already used"; + } + } + } + return null; + } + + /** Replaces an attribute under its canonical (lower-case) key, dropping any camel-case copy. */ + private static void mcpPutAttribute(Element element, String name, String value) { + element.removeAttribute(name); + element.removeAttribute(name.toLowerCase()); + if (value != null) element.setAttribute(name.toLowerCase(), value); + } + + private String mcpNormalizeFragmentAttributes(Element element, String type, String label) { + java.util.Hashtable attributes = element.getAttributes(); + if (attributes != null) { + for (Object key : new ArrayList(attributes.keySet())) { + com.codename1.guibuilder.model.ComponentCatalog.Property property = + com.codename1.guibuilder.model.ComponentCatalog.property(type, String.valueOf(key)); + if (property == null) continue; + try { + mcpPutAttribute(element, property.name(), mcpNormalizeProperty(element, property, attributes.get(key))); + } catch (IllegalArgumentException refused) { + return label + ": " + refused.getMessage(); + } + } + } + // Cells and spans: whole numbers in range, wherever they appear -- a later set_layout can turn + // any container into a table, and the generator emits spans into the source as written. + String[][] ranges = {{"tableRow", "0", "99"}, {"tableColumn", "0", "99"}, {"tableVerticalSpan", "1", "100"}, + {"tableHorizontalSpan", "1", "100"}, {"tableWidth", "-1", "100"}, {"tableHeight", "-1", "100"}, + {"guidedPreferredWidth", "0", "100000"}, {"guidedPreferredHeight", "0", "100000"}, + {"selectedIndex", "0", "100000"}}; + for (String[] range : ranges) { + String raw = element.getAttribute(range[0]); + if (raw == null) continue; + Integer parsed = parseInteger(raw.trim()); + int minimum = Integer.parseInt(range[1]); + int maximum = Integer.parseInt(range[2]); + if (parsed == null || parsed.intValue() < minimum || parsed.intValue() > maximum) { + return label + ": " + range[0] + " must be a whole number from " + minimum + " to " + maximum; + } + mcpPutAttribute(element, range[0], String.valueOf(parsed)); + } + // The guided-layout attributes LayeredLayout and the generated setters parse. + String insets = element.getAttribute("layeredInsets"); + if (insets != null) { + String refused = mcpValidInsets(insets); + if (refused != null) return label + ": " + refused; + } + String[] anchors = {"guidedHorizontalAnchor", "guidedVerticalAnchor"}; + for (String anchor : anchors) { + String raw = element.getAttribute(anchor); + if (raw == null) continue; + Float parsed = mcpFiniteFloat(raw); + if (parsed == null || parsed.floatValue() < 0 || parsed.floatValue() > 1) { + return label + ": " + anchor + " must be a number from 0 to 1"; + } + mcpPutAttribute(element, anchor, String.valueOf(parsed.floatValue())); + } + String positions = element.getAttribute("guidedReferencePositions"); + if (positions != null) { + String[] parts = positions.trim().split(" "); + int count = 0; + for (String part : parts) { + if (part.length() == 0) continue; + if (mcpFiniteFloat(part) == null) return label + ": guidedReferencePositions must be four numbers"; + count++; + } + if (count != 4) return label + ": guidedReferencePositions must be four numbers"; + } + String references = element.getAttribute("guidedReferences"); + if (references != null && references.split("\\|", -1).length != 4) { + return label + ": guidedReferences must be four names or - separated by |"; + } + String[] sizes = {"guidedHorizontalSize", "guidedVerticalSize"}; + for (String size : sizes) { + String raw = element.getAttribute(size); + if (raw == null) continue; + if (!GuidedLayoutSupport.PREFERRED.equals(raw) && !GuidedLayoutSupport.FIXED.equals(raw) + && !GuidedLayoutSupport.FILL.equals(raw) && !GuidedLayoutSupport.MATCH.equals(raw)) { + return label + ": " + size + " must be preferred, fixed, fill or match"; + } + } + return null; + } + + private static Float mcpFiniteFloat(String raw) { + try { + float value = Float.parseFloat(raw.trim()); + return Float.isNaN(value) || Float.isInfinite(value) ? null : Float.valueOf(value); + } catch (NumberFormatException invalid) { + return null; + } + } + + /** A span attribute: 1 when absent, null when it is not a whole number of at least 1. */ + private static Integer parseOptionalCount(String raw) { + if (raw == null || raw.trim().length() == 0) return Integer.valueOf(1); + Integer parsed = parseInteger(raw.trim()); + return parsed == null || parsed.intValue() < 1 ? null : parsed; + } + private String mcpMoveToIndex(Element element, Element parent, int index) { if (index < 0) return "index must be zero or more"; if ("TableLayout".equals(value(parent, "layout", "BoxLayout"))) { @@ -8911,51 +9142,67 @@ private String mcpApplyProperties(Element element, Map propertie com.codename1.guibuilder.model.ComponentCatalog.Property property = com.codename1.guibuilder.model.ComponentCatalog.property(type, key); if (property == null) return mcpUnknownProperty(type, key); - Object raw = entry.getValue(); String normalized; - if (raw == null) { - normalized = null; - } else if (com.codename1.guibuilder.model.ComponentCatalog.KIND_BOOLEAN.equals(property.kind())) { - String text = String.valueOf(raw).trim(); - if (!"true".equalsIgnoreCase(text) && !"false".equalsIgnoreCase(text)) { - return property.name() + " must be true or false"; - } - normalized = String.valueOf("true".equalsIgnoreCase(text)); - } else if (com.codename1.guibuilder.model.ComponentCatalog.KIND_INTEGER.equals(property.kind())) { - Map holder = new LinkedHashMap(); - holder.put(property.name(), raw); - int parsed = mcpInteger(holder, property.name()).intValue(); - int maximum = property.maximum(); - if ("selectedIndex".equals(property.name())) { - maximum = Math.max(0, GuiDocument.componentsIn(element).size() - 1); - } - if (parsed < property.minimum() || parsed > maximum) { - return property.name() + " must be from " + property.minimum() + " to " + maximum; - } - normalized = String.valueOf(parsed); - } else if (com.codename1.guibuilder.model.ComponentCatalog.KIND_CHOICE.equals(property.kind())) { - String text = String.valueOf(raw).trim(); - normalized = null; - for (String choice : property.choices()) if (choice.equalsIgnoreCase(text)) normalized = choice; - if (normalized == null) return property.name() + " must be one of " + Arrays.asList(property.choices()); - } else { - normalized = raw instanceof String ? (String) raw : mcpString(properties, key); - if ("uiid".equals(property.name())) { - normalized = normalized.trim(); - for (int i = 0; i < normalized.length(); i++) { - char c = normalized.charAt(i); - if (!(c >= 'a' && c <= 'z' || c >= 'A' && c <= 'Z' || c >= '0' && c <= '9' || c == '_' || c == '-')) { - return "uiid must be a CSS selector name (letters, digits, - and _)"; - } - } - if (normalized.length() == 0) normalized = null; - } + try { + normalized = mcpNormalizeProperty(element, property, entry.getValue()); + } catch (IllegalArgumentException refused) { + return refused.getMessage(); } document.setAttribute(element, property.name(), normalized); } return null; } + /** + * The stored form of one property value, checked as the inspector checks it. + * + * @param element the component the value is for + * @param property the catalog entry + * @param raw the value as given, or null to reset it + * @return the value to store, or null to remove the attribute + * @throws IllegalArgumentException naming what is wrong with the value + */ + private String mcpNormalizeProperty(Element element, com.codename1.guibuilder.model.ComponentCatalog.Property property, + Object raw) { + if (raw == null) return null; + String kind = property.kind(); + if (com.codename1.guibuilder.model.ComponentCatalog.KIND_BOOLEAN.equals(kind)) { + String text = String.valueOf(raw).trim(); + if (!"true".equalsIgnoreCase(text) && !"false".equalsIgnoreCase(text)) { + throw new IllegalArgumentException(property.name() + " must be true or false"); + } + return String.valueOf("true".equalsIgnoreCase(text)); + } + if (com.codename1.guibuilder.model.ComponentCatalog.KIND_INTEGER.equals(kind)) { + Map holder = new LinkedHashMap(); + holder.put(property.name(), raw); + int parsed = mcpInteger(holder, property.name()).intValue(); + int maximum = property.maximum(); + if ("selectedIndex".equals(property.name())) { + maximum = Math.max(0, GuiDocument.componentsIn(element).size() - 1); + } + if (parsed < property.minimum() || parsed > maximum) { + throw new IllegalArgumentException(property.name() + " must be from " + property.minimum() + " to " + maximum); + } + return String.valueOf(parsed); + } + if (com.codename1.guibuilder.model.ComponentCatalog.KIND_CHOICE.equals(kind)) { + String text = String.valueOf(raw).trim(); + for (String choice : property.choices()) if (choice.equalsIgnoreCase(text)) return choice; + throw new IllegalArgumentException(property.name() + " must be one of " + Arrays.asList(property.choices())); + } + String text = raw instanceof String ? (String) raw : mcpString(java.util.Collections.singletonMap("v", raw), "v"); + if (!"uiid".equals(property.name())) return text; + text = text.trim(); + for (int i = 0; i < text.length(); i++) { + char c = text.charAt(i); + if (!(c >= 'a' && c <= 'z' || c >= 'A' && c <= 'Z' || c >= '0' && c <= '9' || c == '_' || c == '-')) { + throw new IllegalArgumentException("uiid must be a CSS selector name (letters, digits, - and _)"); + } + } + return text.length() == 0 ? null : text; + } + private String mcpUnknownProperty(String type, String key) { String lower = key == null ? "" : key.toLowerCase(); if ("name".equals(lower)) return "Rename with guibuilder_rename_component"; @@ -9221,6 +9468,24 @@ private String mcpApplyGuided(Element element, Element parent, Map tokens = new ArrayList(); @@ -9228,6 +9493,7 @@ private static String mcpValidInsets(String insets) { if (tokens.size() != 4) return "insets must be four values: top right bottom left (e.g. \"12px auto auto 0px\")"; for (String token : tokens) { if ("auto".equals(token) || "baseline".equals(token)) continue; + if (token.indexOf("Infinity") >= 0 || token.indexOf("NaN") >= 0) return "inset " + token + " is not a number"; String number = token; if (number.endsWith("px") || number.endsWith("mm")) number = number.substring(0, number.length() - 2); else if (number.endsWith("%")) number = number.substring(0, number.length() - 1); @@ -9238,7 +9504,7 @@ private static String mcpValidInsets(String insets) { return "inset " + token + " is not a number"; } } - return null; + return mcpLayoutInsetsError(insets.trim()); } // --- events, toolbar, binding ------------------------------------------------------------ @@ -9288,7 +9554,7 @@ private static boolean mcpJavaIdentifier(String name) { + " else enum extends final finally float for goto if implements import instanceof int interface long" + " native new package private protected public return short static strictfp super switch synchronized" + " this throw throws transient try void volatile while true false null _ "; - if (keywords.indexOf(" " + name + " ") >= 0) return false; + if (keywords.indexOf(" " + name + " ") >= 0 || isJavaReserved(name)) return false; for (int i = 0; i < name.length(); i++) { char c = name.charAt(i); boolean letter = c >= 'a' && c <= 'z' || c >= 'A' && c <= 'Z' || c == '_' || c == '$'; @@ -9354,8 +9620,13 @@ private String mcpSetBinding(Map args, Map detai details.put("strategy", strategy); if (strategy.equals(previous)) return null; document.setAttribute(document.root(), "bindingStrategy", strategy); + String before = status == null ? null : status.getText(); syncBindingModel(strategy); - details.put("note", status == null ? null : status.getText()); + // Only what syncBindingModel() itself reported: it says nothing in some cases (no model on + // disk for "none", no source directory), and the status line then still holds whatever came + // before -- a selection, say -- which is no answer to this call. + String after = status == null ? null : status.getText(); + if (after != null && !after.equals(before)) details.put("note", after); return null; } @@ -9388,6 +9659,8 @@ private String mcpCreateFormImpl(Map args, Map d if (binding == null || binding.guiDir() == null || binding.sourceDir() == null) { return "No project is open"; } + String busy = mcpDeveloperBusy(); + if (busy != null) return busy; if (document != null && document.isModified()) return "The active form has unsaved changes; save it first"; if (editorBufferIsDirty()) return "The open editor has unsaved changes; save or close that pane first"; String className = mcpString(args, "className"); diff --git a/scripts/guibuilder/common/src/main/java/com/codename1/guibuilder/model/ComponentCatalog.java b/scripts/guibuilder/common/src/main/java/com/codename1/guibuilder/model/ComponentCatalog.java index 75ce756079c..d608ae6299b 100644 --- a/scripts/guibuilder/common/src/main/java/com/codename1/guibuilder/model/ComponentCatalog.java +++ b/scripts/guibuilder/common/src/main/java/com/codename1/guibuilder/model/ComponentCatalog.java @@ -188,9 +188,15 @@ public static List propertiesFor(String type) { if (isFormLike(type)) { out.add(text("title", "Title")); } else if (hasText(type)) { - out.add(text("text", "Text")); + // What the canvas and the generated source show when the attribute is absent -- which is + // where a reset (null) leaves it -- not an empty string. + String sample = GuiDocument.defaultTextFor(type); + out.add(new Property("text", "Text", KIND_TEXT, sample == null ? "" : sample, 0, 0, null)); + } + if ("TextField".equals(type) || "TextArea".equals(type)) { + String hint = GuiDocument.defaultHintFor(type); + out.add(new Property("hint", "Hint", KIND_TEXT, hint == null ? "" : hint, 0, 0, null)); } - if ("TextField".equals(type) || "TextArea".equals(type)) out.add(text("hint", "Hint")); out.add(new Property("uiid", "UIID (CSS selector)", KIND_TEXT, null, 0, 0, null)); out.add(bool("enabled", "Enabled", true)); out.add(bool("visible", "Visible", true)); diff --git a/scripts/guibuilder/common/src/main/java/com/codename1/guibuilder/model/GuiDocument.java b/scripts/guibuilder/common/src/main/java/com/codename1/guibuilder/model/GuiDocument.java index 918d05a6894..cba3ded9f93 100644 --- a/scripts/guibuilder/common/src/main/java/com/codename1/guibuilder/model/GuiDocument.java +++ b/scripts/guibuilder/common/src/main/java/com/codename1/guibuilder/model/GuiDocument.java @@ -260,9 +260,12 @@ public List commands() { public Element addCommand() { beforeMutation(); Element command = new Element("command"); - command.setAttribute("name", "Command " + (commands().size() + 1)); - command.setAttribute("placement", "right"); - command.setAttribute("actionEvent", "onCommand" + (commands().size() + 1)); + // Through the canonical (lower-case) keys, as every other write: a camel-case key comes back + // lower-case after an undo or rollback re-parses the XML, and the form then reads as modified + // against the copy it was saved with although nothing changed. + setNormalizedAttribute(command, "name", "Command " + (commands().size() + 1)); + setNormalizedAttribute(command, "placement", "right"); + setNormalizedAttribute(command, "actionEvent", "onCommand" + (commands().size() + 1)); root.addChild(command); modified = true; return command; @@ -630,8 +633,11 @@ private void uniquifyPastedNames(Element pasted) { for (Element element : elements) { String current = element.getAttribute("name"); String type = element.getAttribute("type"); - String base = current == null || current.length() == 0 - ? lowerFirst(type == null || type.length() == 0 ? "component" : type) : current; + // Sanitized as a rename is: a pipe splits guidedReferences and whitespace splits the + // positions, so a pasted name holding either broke every relationship anchored to it. + String clean = current == null ? "" : sanitizeName(current); + String base = clean.length() == 0 + ? lowerFirst(type == null || type.length() == 0 ? "component" : type) : clean; String unique = availableName(base, exclude, assigned); assigned.add(unique); if (unique.equals(current)) continue; diff --git a/scripts/guibuilder/common/src/main/java/com/codename1/guibuilder/ui/ComponentPreviewFactory.java b/scripts/guibuilder/common/src/main/java/com/codename1/guibuilder/ui/ComponentPreviewFactory.java index 9b1431ac6e5..b912732f4e1 100644 --- a/scripts/guibuilder/common/src/main/java/com/codename1/guibuilder/ui/ComponentPreviewFactory.java +++ b/scripts/guibuilder/common/src/main/java/com/codename1/guibuilder/ui/ComponentPreviewFactory.java @@ -246,7 +246,16 @@ private static Component sizedContainer(Element element, Element selected, Selec } } if (out.getLayout() instanceof LayeredLayout) GuidedLayoutSupport.apply(element, out); - if (out.getComponentCount() == 0) out.add(emptyContainerHint()); + // A BorderLayout refuses a child without a region, so the hint for an empty one goes in the + // centre: added bare, it threw and left the whole canvas blank as soon as an empty container + // was switched to BorderLayout, from the inspector's picker as much as from anywhere else. + if (out.getComponentCount() == 0) { + if (out.getLayout() instanceof com.codename1.ui.layouts.BorderLayout) { + out.add(com.codename1.ui.layouts.BorderLayout.CENTER, emptyContainerHint()); + } else { + out.add(emptyContainerHint()); + } + } return out; } diff --git a/scripts/guibuilder/javase/src/test/java/com/codename1/guibuilder/McpEditingToolsTest.java b/scripts/guibuilder/javase/src/test/java/com/codename1/guibuilder/McpEditingToolsTest.java index 2ddaee47f04..9578bb42fc8 100644 --- a/scripts/guibuilder/javase/src/test/java/com/codename1/guibuilder/McpEditingToolsTest.java +++ b/scripts/guibuilder/javase/src/test/java/com/codename1/guibuilder/McpEditingToolsTest.java @@ -199,7 +199,8 @@ void insertedXmlCannotTakeABorderRegionThatIsAlreadyUsed() throws Exception { // effectiveBorderConstraint() moves a duplicate region to the first free one, so a clash has // to be caught on the fragment's own constraint, before it lands. CodenameOneGUIBuilder builder = builder(); - call("guibuilder_add_component", "{\"type\":\"Container\",\"name\":\"frame\",\"layout\":{\"layout\":\"BorderLayout\"}}"); + Map frame = call("guibuilder_add_component", "{\"type\":\"Container\",\"name\":\"frame\",\"layout\":{\"layout\":\"BorderLayout\"}}"); + assertEquals(Boolean.TRUE, frame.get("success"), String.valueOf(frame.get("error"))); call("guibuilder_add_component", "{\"type\":\"Label\",\"name\":\"body\",\"parent\":\"frame\"," + "\"constraint\":{\"region\":\"Center\"}}"); String before = builder.documentForTest().toXml(); @@ -320,6 +321,144 @@ void hugeSpansAreRefusedRatherThanOverflowing() throws Exception { assertNull(element(builder, "cell").getAttribute("tableHorizontalSpan")); } + @Test + void createFormWaitsWhileTheDeveloperIsTyping() throws Exception { + // Half-typed inline text is not in the document yet, so the unsaved-changes check cannot see it. + CodenameOneGUIBuilder builder = builder(); + Field inline = CodenameOneGUIBuilder.class.getDeclaredField("inlineEditor"); + inline.setAccessible(true); + inline.set(builder, new com.codename1.ui.TextField("half typ")); + try { + Map refused = call("guibuilder_create_form", "{\"className\":\"com.example.Later\"}"); + assertEquals(Boolean.FALSE, refused.get("success")); + assertFalse(Files.exists(project.resolve("src/main/guibuilder/com/example/Later.gui")), "nothing may be written"); + assertTrue(builder.documentForTest().path().endsWith("EditForm.gui"), "the developer's form stays open"); + } finally { + inline.set(builder, null); + } + } + + @Test + void tablesInsideAFragmentAreCheckedLikeAnyOther() throws Exception { + CodenameOneGUIBuilder builder = builder(); + String before = builder.documentForTest().toXml(); + String[] broken = { + "", + "", + "", + "", + "", + "", + "", + "" + }; + for (String xml : broken) { + Map result = call("guibuilder_insert_xml", "{\"xml\":" + quote(xml) + "}"); + assertEquals(Boolean.FALSE, result.get("success"), "must be refused: " + xml); + assertEquals(before, builder.documentForTest().toXml(), "nothing may land: " + xml); + } + Map ok = call("guibuilder_insert_xml", "{\"xml\":" + quote("" + + "") + "}"); + assertEquals(Boolean.TRUE, ok.get("success"), String.valueOf(ok.get("error"))); + assertNotNull(element(builder, "b").getAttribute("tableRow"), "a child without a cell gets a free one"); + assertNotNull(element(builder, "c").getAttribute("tableRow")); + assertCanvasLinked(builder, "after inserting a nested table"); + } + + @Test + void setBindingReportsOnlyWhatItDid() throws Exception { + CodenameOneGUIBuilder builder = builder(); + call("guibuilder_add_component", "{\"type\":\"Label\",\"name\":\"caption\"}"); + call("guibuilder_select", "{\"component\":\"caption\"}"); + Map on = call("guibuilder_set_binding", "{\"strategy\":\"properties\"}"); + assertEquals(Boolean.TRUE, on.get("success"), String.valueOf(on.get("error"))); + assertTrue(String.valueOf(on.get("note")).contains("model"), "the note says what happens to the model: " + on.get("note")); + Field status = CodenameOneGUIBuilder.class.getDeclaredField("status"); + status.setAccessible(true); + assertEquals(on.get("note"), ((com.codename1.ui.Label) status.get(builder)).getText(), + "the developer sees the binding message, not a generic one"); + Map off = call("guibuilder_set_binding", "{\"strategy\":\"none\"}"); + assertEquals(Boolean.TRUE, off.get("success"), String.valueOf(off.get("error"))); + assertNull(off.get("note"), "no model on disk, nothing to report; the old status line is no answer"); + } + + @Test + void valuesTheCanvasOrTheGeneratedSourceCannotReadAreRefused() throws Exception { + CodenameOneGUIBuilder builder = builder(); + call("guibuilder_add_component", "{\"type\":\"Container\",\"name\":\"free\",\"layout\":{\"layout\":\"LayeredLayout\"}}"); + call("guibuilder_add_component", "{\"type\":\"Button\",\"name\":\"go\",\"parent\":\"free\"}"); + for (String insets : new String[]{"auto auto baseline auto", "1.5px 0px 0px 0px", "Infinity% auto auto auto"}) { + Map result = call("guibuilder_set_constraint", "{\"component\":\"go\",\"insets\":\"" + insets + "\"}"); + assertEquals(Boolean.FALSE, result.get("success"), "LayeredLayout throws on " + insets); + } + assertEquals(Boolean.FALSE, call("guibuilder_set_event_handler", "{\"component\":\"go\",\"handler\":\"record\"}").get("success"), + "the generator renames reserved words, so the reported method would not exist"); + call("guibuilder_add_component", "{\"type\":\"Container\",\"name\":\"grid\",\"layout\":{\"layout\":\"TableLayout\"}}"); + String before = builder.documentForTest().toXml(); + String[][] fragments = { + {"grid", ""}, + {"grid", ""}, + {null, ""}, + {"free", ""}, + {"free", ""}, + {"free", ""}, + {"free", ""} + }; + for (String[] fragment : fragments) { + String parent = fragment[0] == null ? "" : "\"parent\":\"" + fragment[0] + "\","; + Map result = call("guibuilder_insert_xml", "{" + parent + "\"xml\":" + quote(fragment[1]) + "}"); + assertEquals(Boolean.FALSE, result.get("success"), "must be refused: " + fragment[1]); + assertEquals(before, builder.documentForTest().toXml(), "nothing may land: " + fragment[1]); + } + assertCanvasLinked(builder, "after the refusals"); + } + + @Test + void fragmentValuesAreStoredTheWayTheCanvasAndGeneratorReadThem() throws Exception { + CodenameOneGUIBuilder builder = builder(); + Map result = call("guibuilder_insert_xml", "{\"xml\":" + quote("") + "}"); + assertEquals(Boolean.TRUE, result.get("success"), String.valueOf(result.get("error"))); + assertEquals("X", element(builder, "box").getAttribute("boxLayoutAxis")); + assertEquals("true", element(builder, "pw").getAttribute("enabled"), "the generator compares with \"true\" exactly"); + assertEquals("PASSWORD", element(builder, "pw").getAttribute("constraint"), "lower case would show the password in plain text"); + List names = (List) result.get("components"); + for (Object name : names) { + assertFalse(String.valueOf(name).contains("|") || String.valueOf(name).contains(" "), "pasted names are sanitized: " + names); + } + } + + @Test + void aRollbackLeavesASavedFormUnmodified() throws Exception { + CodenameOneGUIBuilder builder = builder(); + call("guibuilder_toolbar_commands", "{\"action\":\"add\",\"label\":\"Share\"}"); + assertEquals(Boolean.TRUE, call("guibuilder_command", "{\"command\":\"save\"}").get("success")); + assertFalse(builder.documentForTest().isModified()); + Map refused = call("guibuilder_add_component", "{\"type\":\"Label\",\"properties\":{\"gap\":-5}}"); + assertEquals(Boolean.FALSE, refused.get("success")); + assertFalse(builder.documentForTest().isModified(), "a rolled-back edit leaves the form as saved"); + assertCanvasLinked(builder, "after the rollback"); + } + + @Test + void theCatalogReportsTheTextAResetActuallyShows() throws Exception { + builder(); + Map catalog = call("guibuilder_component_types", "{}"); + for (Object entry : (List) catalog.get("types")) { + Map type = (Map) entry; + if (!"Button".equals(type.get("type"))) continue; + for (Object property : (List) type.get("properties")) { + if ("text".equals(((Map) property).get("name"))) { + assertEquals("Button", ((Map) property).get("default")); + return; + } + } + } + fail("Button has no text property in the catalog"); + } + @Test void tableCellsCannotOverlap() throws Exception { CodenameOneGUIBuilder builder = builder(); From 1a2cd4b13284187d765286fe59734034f18dd03c Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 11 Nov 2026 09:14:12 +0000 Subject: [PATCH 6/6] GUI Builder MCP: form names that shadow imports, silent guided no-ops, redo of reverted edits - create_form refuses a class name that the generated companion imports (com.codename1.ui.*, ui.layouts.*, properties.* and the single-type imports): a form named Form extended itself, one named Label turned every generated Label field into the form. A test checks the list against the core jar so a new core class cannot slip past it. - Guided alignment and match policies refuse when no reference component is visible on the canvas, instead of reporting success with nothing written. - An edit the canvas cannot show is reverted without a redo entry (GuiDocument.revertLastChange), so Redo cannot bring it back. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01SZMcbjRgY2gZgT5nLW9RVG --- .../guibuilder/CodenameOneGUIBuilder.java | 50 ++++++++++++++++++- .../guibuilder/model/GuiDocument.java | 12 +++++ .../guibuilder/McpEditingToolsTest.java | 48 ++++++++++++++++++ 3 files changed, 108 insertions(+), 2 deletions(-) diff --git a/scripts/guibuilder/common/src/main/java/com/codename1/guibuilder/CodenameOneGUIBuilder.java b/scripts/guibuilder/common/src/main/java/com/codename1/guibuilder/CodenameOneGUIBuilder.java index b430d431019..d371c5e7848 100644 --- a/scripts/guibuilder/common/src/main/java/com/codename1/guibuilder/CodenameOneGUIBuilder.java +++ b/scripts/guibuilder/common/src/main/java/com/codename1/guibuilder/CodenameOneGUIBuilder.java @@ -8610,7 +8610,9 @@ private Map mcpTransaction(String operation, McpEditOperation ed // would fail the same way. Take it back out rather than report it as done. final String failure = rebuildFailure; unattended(() -> { - if (document.undo()) refreshEditor(); + // Not undo(): that would leave the change on the redo stack, and Redo would bring back + // a change the canvas has already shown it cannot draw. + if (document.revertLastChange()) refreshEditor(); return null; }); error = "The canvas could not show that change, so it was undone: " + failure; @@ -9450,9 +9452,17 @@ private String mcpApplyGuided(Element element, Element parent, Map args, Map d return null; } + /** + * Every simple name the generated companion and model sources import: com.codename1.ui.*, + * com.codename1.ui.layouts.*, com.codename1.properties.* and the single-type imports. A form + * class with one of these names shadows the import for everything in its package -- a form + * named Form extends itself, and one named Label turns every generated Label field into it. + * McpEditingToolsTest checks this list against the core jar. + */ + static final String GENERATED_IMPORT_NAMES = + "|AbstractDialog|AbstractEditorComponent|AccessibilityColorVisionDeficiency|Accordion|ActionEvent|" + + "AnimationManager|AutoCompleteTextComponent|AutoCompleteTextField|Bind|BindAttr|Bindable|" + + "Binders|Binding|BlockingDisallowedException|BooleanProperty|BorderLayout|BoxLayout|" + + "BrowserComponent|BrowserWindow|Button|ButtonGroup|ByteProperty|CN|CN1Constants|CSSColor|" + + "Calendar|CharProperty|CheckBox|ClipboardContent|ClipboardDataProvider|CodeCompletion|" + + "CodeCompletionProvider|CodeDiagnostic|CodeEditor|CollectionProperty|ComboBox|Command|" + + "CommonProgressAnimations|Component|ComponentGroup|ComponentImage|ComponentSelector|" + + "ConicGradient|Container|ContextMenu|CoordinateLayout|Desktop|DevicePosture|Dialog|Dimension|" + + "Display|DoubleProperty|DynamicImage|EditField|Editable|EncodedImage|FloatProperty|FlowLayout|" + + "Font|FontImage|Form|GeneratedSVGImage|Gradient|Graphics|GridBagConstraints|GridBagLayout|" + + "GridLayout|GroupLayout|IconHolder|Image|ImageFactory|InfiniteContainer|InputComponent|Insets|" + + "InstantUI|IntProperty|InterFormContainer|Label|LayeredLayout|Layout|LayoutStyle|LinearGradient|" + + "LinearGradientPaint|List|ListProperty|LongProperty|MapAdapter|MapProperty|MenuBar|Monitor|" + + "MultipleGradientPaint|NativeDragAndDrop|NativeDragOperation|NativeDropEvent|NavigationCommand|" + + "NumericProperty|Paint|Painter|PeerComponent|PickerComponent|PreferencesObject|Property|" + + "PropertyBase|PropertyBusinessObject|PropertyChangeListener|PropertyIndex|RGBImage|" + + "RadialGradient|RadioButton|ReleasableComponent|RichTextArea|RichTextClipboardData|" + + "RichTextComponent|RichTextFormat|SQLMap|SelectableIconHolder|SetProperty|Sheet|SideMenuBar|" + + "Slider|SpanLabel|Stroke|SwipeableContainer|TableLayout|Tabs|TextArea|TextComponent|" + + "TextComponentPassword|TextField|TextHolder|TextInputClient|TextInputConfig|TextInputGeometry|" + + "TextInputState|TextModeLayout|TextSelection|Toolbar|TooltipManager|TopLevelContainer|Transform|" + + "UIFragment|URLImage|UiBinding|VirtualInputDevice|Window|"; + private static String mcpFormClassError(String className) { if (className == null || className.length() == 0) return "className is required, e.g. com.example.myapp.LoginForm"; String[] parts = className.split("\\."); @@ -9734,6 +9775,11 @@ private static String mcpFormClassError(String className) { } if (!mcpJavaIdentifier(part)) return part + " is a Java keyword"; } + String simple = parts[parts.length - 1]; + if (GENERATED_IMPORT_NAMES.indexOf("|" + simple + "|") >= 0) { + return simple + " is the name of a Codename One class the generated source imports; choose another," + + " e.g. " + simple + "Form"; + } return null; } diff --git a/scripts/guibuilder/common/src/main/java/com/codename1/guibuilder/model/GuiDocument.java b/scripts/guibuilder/common/src/main/java/com/codename1/guibuilder/model/GuiDocument.java index cba3ded9f93..9ccfee24a77 100644 --- a/scripts/guibuilder/common/src/main/java/com/codename1/guibuilder/model/GuiDocument.java +++ b/scripts/guibuilder/common/src/main/java/com/codename1/guibuilder/model/GuiDocument.java @@ -898,6 +898,18 @@ public boolean undo() { return true; } + /** + * Takes the last change back without offering it to Redo. For a change that must not come back -- + * one the canvas could not show, which Redo would only reinstate to fail again. + * + * @return true when there was a change to take back + */ + public boolean revertLastChange() { + if (!canUndo()) return false; + restore(undo.remove(undo.size() - 1)); + return true; + } + public boolean redo() { if (!canRedo()) return false; undo.add(capture()); diff --git a/scripts/guibuilder/javase/src/test/java/com/codename1/guibuilder/McpEditingToolsTest.java b/scripts/guibuilder/javase/src/test/java/com/codename1/guibuilder/McpEditingToolsTest.java index 9578bb42fc8..72ef98aee69 100644 --- a/scripts/guibuilder/javase/src/test/java/com/codename1/guibuilder/McpEditingToolsTest.java +++ b/scripts/guibuilder/javase/src/test/java/com/codename1/guibuilder/McpEditingToolsTest.java @@ -459,6 +459,54 @@ void theCatalogReportsTheTextAResetActuallyShows() throws Exception { fail("Button has no text property in the catalog"); } + @Test + void aFormCannotBeNamedAfterAClassItsSourceImports() throws Exception { + builder(); + for (String name : new String[]{"Form", "Label", "Button", "Container", "BorderLayout", "ActionEvent"}) { + Map result = call("guibuilder_create_form", "{\"className\":\"com.example.app." + name + "\"}"); + assertEquals(Boolean.FALSE, result.get("success"), name + " would shadow the generated import"); + assertFalse(Files.exists(project.resolve("src/main/guibuilder/com/example/app/" + name + ".gui"))); + } + } + + @Test + void theImportedNameListCoversTheCoreJar() throws Exception { + // GENERATED_IMPORT_NAMES is written out by hand because the builder cannot list a package at + // run time; this fails when core gains a public class the generated source would import. + java.io.File jar = new java.io.File(com.codename1.ui.Form.class.getProtectionDomain().getCodeSource().getLocation().toURI()); + List missing = new ArrayList(); + try (java.util.zip.ZipFile zip = new java.util.zip.ZipFile(jar)) { + java.util.Enumeration entries = zip.entries(); + while (entries.hasMoreElements()) { + String entry = entries.nextElement().getName(); + String directory = entry.substring(0, Math.max(0, entry.lastIndexOf('/'))); + if (!entry.endsWith(".class") || entry.indexOf('$') >= 0) continue; + if (!"com/codename1/ui".equals(directory) && !"com/codename1/ui/layouts".equals(directory) + && !"com/codename1/properties".equals(directory)) continue; + String className = entry.substring(0, entry.length() - 6).replace('/', '.'); + Class type = Class.forName(className, false, getClass().getClassLoader()); + if (!java.lang.reflect.Modifier.isPublic(type.getModifiers()) || "package-info".equals(type.getSimpleName())) continue; + if (CodenameOneGUIBuilder.GENERATED_IMPORT_NAMES.indexOf("|" + type.getSimpleName() + "|") < 0) { + missing.add(type.getSimpleName()); + } + } + } + assertTrue(missing.isEmpty(), "add these to CodenameOneGUIBuilder.GENERATED_IMPORT_NAMES: " + missing); + } + + @Test + void guidedAlignmentWithoutAVisibleReferenceIsNotReportedAsDone() throws Exception { + CodenameOneGUIBuilder builder = builder(); + call("guibuilder_add_component", "{\"type\":\"Container\",\"name\":\"free\",\"layout\":{\"layout\":\"LayeredLayout\"}}"); + call("guibuilder_add_component", "{\"type\":\"Button\",\"name\":\"hidden\",\"parent\":\"free\",\"properties\":{\"visible\":false}}"); + call("guibuilder_add_component", "{\"type\":\"Button\",\"name\":\"go\",\"parent\":\"free\"," + + "\"constraint\":{\"insets\":\"40px auto auto 40px\"}}"); + String before = builder.documentForTest().toXml(); + Map result = call("guibuilder_set_constraint", "{\"component\":\"go\",\"align\":[\"alignLeft\"]}"); + assertFalse(Boolean.TRUE.equals(result.get("success")) && before.equals(builder.documentForTest().toXml()), + "success must mean something changed: " + result.get("error")); + } + @Test void tableCellsCannotOverlap() throws Exception { CodenameOneGUIBuilder builder = builder();