From a040cc1957c1a93df53f7b1da2c25a0350e104e4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fran=C3=A7ois?= Date: Wed, 3 Jun 2026 20:12:11 +0200 Subject: [PATCH] Extract connect into core (app::connect_parts); thin the command + screen. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Move the wiring orchestration — transform lookup, identity-compatibility check, identity NC fill, transform apply, Connection creation/add — out of the `connect` command and the interactive connect screen into core/app/connect.{hpp,cpp}: app::connect_parts(System*, m1,p1, m2,p2) returns a ConnectResult (ok/refused/error, connection_name, transform_name, wires, identity_info, nc_added) with no Print/dialog/FTXUI. Name/pattern resolution and ambiguity reporting stay in the command — that is arg-parsing, not the op. Both frontends now call the one core op, removing the duplicated logic. This also unifies a divergence: the interactive screen previously called CheckIdentityCompatible without the info pointer and so never filled identity NC pins (unlike the command); routing it through app::connect_parts makes the screen fill NCs and surface the same warning, matching the scriptable path. Command output is unchanged. Prune the now-dead transform.hpp / domain connect.hpp includes from the frontends (commands.cpp keeps transform_vpx.hpp only for ValidatePartForKind). Add tests/test_connect.cpp (core, no UI): identity-compatible pair wires, unknown type pairing is refused with nothing created, subset side gets NC pins filled and the warning reported. Co-Authored-By: Claude Opus 4.8 --- src/core/app/connect.cpp | 67 +++++++++++++++++++++++ src/core/app/connect.hpp | 42 +++++++++++++++ src/frontends/tui/commands.cpp | 60 +++++++-------------- src/frontends/tui/screen_connect.cpp | 48 ++++++----------- tests/test_connect.cpp | 79 ++++++++++++++++++++++++++++ 5 files changed, 223 insertions(+), 73 deletions(-) create mode 100644 src/core/app/connect.cpp create mode 100644 src/core/app/connect.hpp create mode 100644 tests/test_connect.cpp diff --git a/src/core/app/connect.cpp b/src/core/app/connect.cpp new file mode 100644 index 0000000..b3484f1 --- /dev/null +++ b/src/core/app/connect.cpp @@ -0,0 +1,67 @@ +#include "core/app/connect.hpp" + +#include "core/domain/connect.hpp" +#include "core/domain/modules.hpp" +#include "core/domain/parts.hpp" +#include "core/domain/system.hpp" +#include "core/domain/transform.hpp" + +#include +#include + +namespace app { + +ConnectResult connect_parts(System *sys, Module *m1, Part *p1, + Module *m2, Part *p2) +{ + ConnectResult r; + + auto ® = TransformRegistry::get(); + Transform *t = reg.lookup(p1->connector_type, p2->connector_type); + bool both_empty = p1->connector_type.empty() && p2->connector_type.empty(); + + if (t == reg.identity()) { + if (!both_empty) { + r.refused = true; + r.error = "no transform for types '" + + (p1->connector_type.empty() ? std::string("(none)") + : p1->connector_type) + + "' ↔ '" + + (p2->connector_type.empty() ? std::string("(none)") + : p2->connector_type) + + "'. Set matching types via 'set-connector-type' first."; + return r; + } + std::string info; + std::string err = CheckIdentityCompatible(p1, p2, &info); + if (!err.empty()) { + r.refused = true; + r.error = err; + return r; + } + if (!info.empty()) { + r.identity_info = info; + r.nc_added = FillIdentityNCs(p1, p2); + } + } + + auto pin_map = t->apply(p1, p2); + + r.connection_name = m1->name + "/" + p1->name + + " <-> " + m2->name + "/" + p2->name; + r.transform_name = t->name; + try { + Connection *c = new Connection(r.connection_name, m1, p1, m2, p2); + c->transform_name = t->name; + c->pin_map = std::move(pin_map); + sys->connections()->add(c); + r.wires = (int)c->pin_map.size(); + r.ok = true; + } catch (const std::exception &e) { + r.error = e.what(); + } + + return r; +} + +} // namespace app diff --git a/src/core/app/connect.hpp b/src/core/app/connect.hpp new file mode 100644 index 0000000..4522486 --- /dev/null +++ b/src/core/app/connect.hpp @@ -0,0 +1,42 @@ +#ifndef _APP_CONNECT_HPP_ +#define _APP_CONNECT_HPP_ + +#include + +class System; +class Module; +class Part; + +// Application layer: UI-independent operations that any frontend (TUI, GUI, …) +// can call. No console, no dialogs, no FTXUI — just System in, result out. +namespace app { + +// Outcome of connecting two parts. The side effects (filling identity NC pins, +// creating the Connection and adding it to the system) all happen in core; the +// caller only renders the fields. +struct ConnectResult { + bool ok = false; ///< a Connection was created and added + bool refused = false; ///< a business rule rejected it (vs. an exception) + std::string error; ///< why refused/failed; empty when ok + + std::string connection_name; + std::string transform_name; + int wires = 0; ///< pin_map size of the created connection + + // Identity-transform path only: the compatibility info line and how many NC + // pins were materialised so both sides match. Empty / 0 otherwise. + std::string identity_info; + int nc_added = 0; +}; + +// Wire part `p1` (in module `m1`) to part `p2` (in module `m2`): look up the +// transform for their connector types, refuse on an unknown pairing or an +// identity-incompatible layout, fill identity NC pins when needed, apply the +// transform and create the Connection. Pure core — no resolution of names or +// patterns (the frontend turns user input into the Module*/Part* it passes). +ConnectResult connect_parts(System *sys, Module *m1, Part *p1, + Module *m2, Part *p2); + +} // namespace app + +#endif // _APP_CONNECT_HPP_ diff --git a/src/frontends/tui/commands.cpp b/src/frontends/tui/commands.cpp index 18e2996..d80c7c7 100644 --- a/src/frontends/tui/commands.cpp +++ b/src/frontends/tui/commands.cpp @@ -13,9 +13,9 @@ #include "core/domain/signals.hpp" #include "core/domain/system.hpp" +#include "core/app/connect.hpp" #include "core/app/verify.hpp" -#include "core/domain/transform.hpp" -#include "core/domain/transform_vpx.hpp" +#include "core/domain/transform_vpx.hpp" // ValidatePartForKind #include #include @@ -560,47 +560,23 @@ void Tui::RegisterCommands() { auto [p2, p2_alts] = resolve_part(m2, args[3]); if (!p2) { report_ambiguous("part in " + m2->name, args[3], p2_alts); return; } - auto ® = TransformRegistry::get(); - Transform *t = reg.lookup(p1->connector_type, p2->connector_type); - bool both_empty = p1->connector_type.empty() && p2->connector_type.empty(); - if (t == reg.identity()) { - if (!both_empty) { - Print("connect refused: no transform for types '" - + (p1->connector_type.empty() ? "(none)" : p1->connector_type) - + "' ↔ '" - + (p2->connector_type.empty() ? "(none)" : p2->connector_type) - + "'. Set matching types via 'set-connector-type' first."); - return; - } - std::string info; - std::string err = CheckIdentityCompatible(p1, p2, &info); - if (!err.empty()) { - Print("connect refused: " + err); - return; - } - if (!info.empty()) { - int added = FillIdentityNCs(p1, p2); - Print("connect: " + info); - if (added > 0) - Print("connect: added " + std::to_string(added) - + " NC pin(s) so both sides match"); - } - } - auto pin_map = t->apply(p1, p2); - - std::string conn_name = m1->name + "/" + p1->name - + " <-> " + m2->name + "/" + p2->name; - try { - Connection *c = new Connection(conn_name, m1, p1, m2, p2); - c->transform_name = t->name; - c->pin_map = std::move(pin_map); - sys->connections()->add(c); - Print("connected: " + conn_name - + " via " + t->name - + " (" + std::to_string(c->pin_map.size()) + " wires)"); - } catch (const std::exception &e) { - Print(std::string("connect failed: ") + e.what()); + // Resolution above is arg-parsing (user text → objects); the wiring + // itself — transform lookup, identity NC fill, Connection creation — + // is app::connect_parts. + app::ConnectResult cr = app::connect_parts(sys.get(), m1, p1, m2, p2); + if (cr.refused) { Print("connect refused: " + cr.error); return; } + if (!cr.identity_info.empty()) { + Print("connect: " + cr.identity_info); + if (cr.nc_added > 0) + Print("connect: added " + std::to_string(cr.nc_added) + + " NC pin(s) so both sides match"); } + if (cr.ok) + Print("connected: " + cr.connection_name + + " via " + cr.transform_name + + " (" + std::to_string(cr.wires) + " wires)"); + else + Print(std::string("connect failed: ") + cr.error); }, /*prompt_for_missing=*/ false, "connect a part across two modules (interactive screen if no args)", diff --git a/src/frontends/tui/screen_connect.cpp b/src/frontends/tui/screen_connect.cpp index bb01b4a..80dbaa7 100644 --- a/src/frontends/tui/screen_connect.cpp +++ b/src/frontends/tui/screen_connect.cpp @@ -1,11 +1,10 @@ #include "frontends/tui/tui.hpp" #include "frontends/tui/tui_helpers.hpp" -#include "core/domain/connect.hpp" +#include "core/app/connect.hpp" #include "core/domain/modules.hpp" #include "core/domain/parts.hpp" #include "core/domain/system.hpp" -#include "core/domain/transform.hpp" #include #include @@ -67,37 +66,24 @@ Component Tui::BuildConnectScreen() { Part *p1 = m1->get(connect_p1_list[connect_p1_idx]); Part *p2 = m2->get(connect_p2_list[connect_p2_idx]); - auto ® = TransformRegistry::get(); - Transform *t = reg.lookup(p1->connector_type, p2->connector_type); - bool both_empty = p1->connector_type.empty() && p2->connector_type.empty(); - if (t == reg.identity()) { - if (!both_empty) { - Print("connect refused: no transform for types '" - + (p1->connector_type.empty() ? "(none)" : p1->connector_type) - + "' ↔ '" - + (p2->connector_type.empty() ? "(none)" : p2->connector_type) - + "'. Set matching types via 'set-connector-type' first."); - screen_idx = 0; - return; - } - std::string err = CheckIdentityCompatible(p1, p2); - if (!err.empty()) { - Print("connect refused: " + err); - screen_idx = 0; - return; + // Same wiring op as the `connect` command — see app::connect_parts. + app::ConnectResult cr = app::connect_parts(sys.get(), m1, p1, m2, p2); + if (cr.refused) { + Print("connect refused: " + cr.error); + } else { + if (!cr.identity_info.empty()) { + Print("connect: " + cr.identity_info); + if (cr.nc_added > 0) + Print("connect: added " + std::to_string(cr.nc_added) + + " NC pin(s) so both sides match"); } + if (cr.ok) + Print("connected: " + cr.connection_name + + " via " + cr.transform_name + + " (" + std::to_string(cr.wires) + " wires)"); + else + Print(std::string("connect failed: ") + cr.error); } - auto pin_map = t->apply(p1, p2); - - std::string conn_name = m1->name + "/" + p1->name - + " <-> " + m2->name + "/" + p2->name; - Connection *c = new Connection(conn_name, m1, p1, m2, p2); - c->transform_name = t->name; - c->pin_map = std::move(pin_map); - sys->connections()->add(c); - Print("connected: " + conn_name - + " via " + t->name - + " (" + std::to_string(c->pin_map.size()) + " wires)"); } catch (const std::exception &e) { Print(std::string("connect failed: ") + e.what()); } diff --git a/tests/test_connect.cpp b/tests/test_connect.cpp new file mode 100644 index 0000000..30041d6 --- /dev/null +++ b/tests/test_connect.cpp @@ -0,0 +1,79 @@ +#include + +#include "core/app/connect.hpp" +#include "core/domain/connect.hpp" +#include "core/domain/modules.hpp" +#include "core/domain/parts.hpp" +#include "core/domain/pins.hpp" +#include "core/domain/system.hpp" + +// app::connect_parts is pure core: given two already-resolved parts it looks up +// the transform, fills identity NC pins, creates the Connection and returns a +// ConnectResult. No Print/dialog/FTXUI. These tests drive it directly. + +namespace { + +// A part with the given pin names, attached to a fresh module. +Part *make_part(System &sys, const std::string &mod, const std::string &part, + std::initializer_list pins, + const std::string &type = "") +{ + Module *m = sys.modules()->merge(mod); + Part *p = new Part(part); + p->connector_type = type; + m->add(p); + for (const char *pn : pins) p->add(new Pin(pn)); + return p; +} + +} // namespace + +TEST_CASE("connect_parts wires an identity-compatible pair") { + System sys; + Module *a = sys.modules()->merge("A"); + Module *b = sys.modules()->merge("B"); + Part *p1 = make_part(sys, "A", "J1", {"1", "2"}); + Part *p2 = make_part(sys, "B", "P1", {"1", "2"}); + + app::ConnectResult r = app::connect_parts(&sys, a, p1, b, p2); + + CHECK(r.ok); + CHECK_FALSE(r.refused); + CHECK(r.transform_name == "identity"); + CHECK(r.wires == 2); + CHECK(r.identity_info.empty()); // identical sets → no NC fill, no warning + CHECK(r.nc_added == 0); + CHECK(r.connection_name == "A/J1 <-> B/P1"); + CHECK(sys.connections()->size() == 1); +} + +TEST_CASE("connect_parts refuses an unknown connector-type pairing") { + System sys; + Module *a = sys.modules()->merge("A"); + Module *b = sys.modules()->merge("B"); + Part *p1 = make_part(sys, "A", "J1", {"1"}, "foo"); + Part *p2 = make_part(sys, "B", "P1", {"1"}, "bar"); + + app::ConnectResult r = app::connect_parts(&sys, a, p1, b, p2); + + CHECK_FALSE(r.ok); + CHECK(r.refused); + CHECK(r.error.find("no transform") != std::string::npos); + CHECK(sys.connections()->size() == 0); // nothing created +} + +TEST_CASE("connect_parts fills NC pins on the subset side and reports it") { + System sys; + Module *a = sys.modules()->merge("A"); + Module *b = sys.modules()->merge("B"); + Part *p1 = make_part(sys, "A", "J1", {"1", "2", "3"}); // larger side + Part *p2 = make_part(sys, "B", "P1", {"1", "2"}); // missing "3" + + app::ConnectResult r = app::connect_parts(&sys, a, p1, b, p2); + + CHECK(r.ok); + CHECK_FALSE(r.identity_info.empty()); // subset path surfaces a warning + CHECK(r.nc_added == 1); // pin "3" materialised on B + CHECK(r.wires == 3); // all three now wired + CHECK(p2->size() == 3); // the NC pin really got added +}