From 969116b094b3c8221650801d5b70a1ec76ce755e Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 9 Sep 2026 07:02:58 +0000 Subject: [PATCH 1/2] [minor] Draw the inspector's properties as a property grid Every property of the selected node is now a row of ktsu.ImGui.Widgets' PropertyGrid rather than an individually labelled control: the names line up in a column of their own, the editors fill the width left over rather than whatever the widest label made room for, and the divider between the two is the user's to drag. Which rows a node has is still AstFields' answer, so the panel offers what it always did. What changed beside the layout is that a whole number and a fraction get the grid's number rows rather than a text box, so the editor steps with the arrow keys and refuses what is not a number before AstFields.TryWrite has to; the value still travels as text between the two, which is what keeps one undoable command able to record any field. Two rows the widget has no method for - a choice between arbitrary labelled values, and a count with the buttons that change it - are composed into the grid's own table, so they sit in the same two columns as the rest. Both are named for the probes the way the grid names its own rows, which is what lets the new panel tests pick an operator by the label it reads as. ktsu.ImGui.Widgets goes to 3.31.0, where the property grid arrived, and ktsu.ImGui.Probes comes with it. The rest of the suite stays at 3.28.0: the releases in between also retuned the force-directed layout enough to move a graph past what Editor_KeepsTheLayoutInsideTheView allows, which is a change to take on its own. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01UNhZ9nTJqzyTmCySe2ZE5z --- CLAUDE.md | 2 + Coder.Graph/AstGraphEditor.cs | 222 ++++++++++++++++-- Coder.Graph/Coder.Graph.csproj | 5 + .../AstGraphEditorInspectorPanelTests.cs | 210 +++++++++++++++++ Directory.Packages.props | 6 +- 5 files changed, 418 insertions(+), 27 deletions(-) create mode 100644 Coder.Test/Graph/AstGraphEditorInspectorPanelTests.cs diff --git a/CLAUDE.md b/CLAUDE.md index 1851d71..4530385 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -64,6 +64,8 @@ source in four target languages. The solution uses: - `ktsu.DeepClone` — cloning support for AST nodes - `YamlDotNet` — YAML serialization - `ktsu.ImGui.NodeEditor`, `Hexa.NET.ImGui`, `Hexa.NET.ImNodes` — the node-graph editor +- `ktsu.ImGui.Widgets` — the editor's divider panes, and the property grid the inspector's rows are +- `ktsu.ImGui.Probes` — names the two inspector rows composed by hand, so a headless test finds them - `ktsu.ImGui.App` — the desktop application shell - `ktsu.UndoRedo.Core` — the graph editor's undo stack - `ktsu.Essentials` — filesystem and persistence providers the editor reads and writes through diff --git a/Coder.Graph/AstGraphEditor.cs b/Coder.Graph/AstGraphEditor.cs index 8b09d2f..8834913 100644 --- a/Coder.Graph/AstGraphEditor.cs +++ b/Coder.Graph/AstGraphEditor.cs @@ -4,6 +4,7 @@ namespace ktsu.Coder.Graph; using System; using System.Collections.Generic; +using System.Globalization; using System.Linq; using System.Numerics; using Hexa.NET.ImGui; @@ -11,6 +12,8 @@ namespace ktsu.Coder.Graph; using ktsu.Coder.Ast; using ktsu.ForceDirectedLayout; using ktsu.ImGui.NodeEditor; +using ktsu.ImGui.Probes; +using ktsu.ImGui.Widgets; using ktsu.UndoRedo; using ktsu.UndoRedo.Contracts; using ktsu.UndoRedo.Core.Services; @@ -37,6 +40,28 @@ public sealed class AstGraphEditor(AstNode root) { private readonly NodeEditorRenderer renderer = new(); + /// + /// How the inspector's property grid is laid out. + /// + /// + /// A name column of very nearly half the panel, rather than the roughly a third the widget's + /// default leaves it. The weight is a share of the value column's own rather than of the whole, + /// so 0.95 against that column's 1 is the even split a panel this narrow needs: the inspector + /// sits beside the graph, and the names it draws — Visibility, Parameters — are + /// longer than most of the values beside them. It stays the user's to drag either way. + /// + /// Fractions are spelled to fifteen significant digits rather than the widget's six decimal + /// places. A literal is a value the generated source will carry, so the row has to show what the + /// document holds: 3.14 has to read as 3.14 rather than as 3.140000, and a value with more digits + /// than that has to keep them when the user opens the box to change something else about it. + /// + /// + private static readonly ImGuiWidgets.PropertyGridOptions InspectorGridOptions = new() + { + LabelColumnWeight = 0.95f, + DoubleFormat = "%.15g", + }; + private string statusMessage = string.Empty; private string fieldBuffer = string.Empty; @@ -323,14 +348,28 @@ public void DrawInspector(Vector2 size) ImGui.TextUnformatted(AstSchema.Describe(node)); - foreach (AstField field in AstFields.Of(node)) + // One property grid rather than a column of individually labelled controls: the names line up + // in a column of their own, every editor fills the width left over instead of whatever the + // widest label made room for, and the divider between the two is the user's to drag. Which + // rows a node has is still AstFields' answer, so what the panel offers has not changed — only + // how it is laid out, which is the widget's to decide. + using (ImGuiWidgets.PropertyGrid grid = new("ast-inspector-properties", InspectorGridOptions)) { - DrawField(node, field); - } + // A grid whose table never opened is one the panel is too small to show, and each of its + // rows is a no-op. Its own rows know that; the two composed below have to be told, and so + // does the commit that reads back the item a row left behind. + if (grid.IsDrawing) + { + foreach (AstField field in AstFields.Of(node)) + { + DrawField(grid, node, field); + } - foreach (AstSlot slot in AstSchema.SlotsOf(node).Where(slot => slot.Cardinality == AstSlotCardinality.Many)) - { - DrawSlotCount(node, slot); + foreach (AstSlot slot in AstSchema.SlotsOf(node).Where(slot => slot.Cardinality == AstSlotCardinality.Many)) + { + DrawSlotCount(node, slot); + } + } } DrawConversions(node); @@ -417,35 +456,44 @@ private void DrawConversions(AstNode node) } /// - /// Draws one editable property, using the widget its kind calls for. + /// Draws one editable property as a row of the inspector's grid, using the editor its kind calls + /// for. /// + /// The grid the row is drawn in. /// The node being edited. /// The property to draw. /// - /// Text is committed when the box is left or the user presses enter rather than on every + /// A flag is committed as it is clicked, since a checkbox has nowhere to be half-set. Everything + /// that is typed is committed when the box is left or the user presses enter rather than on every /// keystroke, so typing a name puts one step on the undo stack instead of one per character. /// - private void DrawField(AstNode node, AstField field) + private void DrawField(ImGuiWidgets.PropertyGrid grid, AstNode node, AstField field) { - ImGui.SetNextItemWidth(180f); - switch (field.Kind) { case AstFieldKind.Flag: bool flag = string.Equals(field.Value, "true", StringComparison.Ordinal); - if (ImGui.Checkbox(field.Name, ref flag)) + if (grid.Value(field.Name, ref flag)) { SetField(node, field.Name, flag ? "true" : "false"); } break; + case AstFieldKind.Number: + DrawNumberField(grid, node, field); + break; + + case AstFieldKind.Fraction: + DrawFractionField(grid, node, field); + break; + case AstFieldKind.Choice: DrawChoiceField(node, field); break; default: - DrawTextField(node, field); + DrawTextField(grid, node, field); break; } } @@ -453,32 +501,130 @@ private void DrawField(AstNode node, AstField field) /// /// Draws a property the user types into. /// + /// The grid the row is drawn in. + /// The node being edited. + /// The property to draw. + private void DrawTextField(ImGuiWidgets.PropertyGrid grid, AstNode node, AstField field) + { + string buffer = ShownValue(field); + + if (grid.Value(field.Name, ref buffer)) + { + HoldEdit(field, buffer); + } + + CommitWhenLeft(node, field); + } + + /// + /// Draws a property that holds a whole number. + /// + /// The grid the row is drawn in. /// The node being edited. /// The property to draw. /// + /// A number row rather than a text one, so the box steps with the arrow keys and refuses what is + /// not a number before has to. The value still travels as text + /// between the two, which is what lets one undoable command record any field whatever it holds. + /// + private void DrawNumberField(ImGuiWidgets.PropertyGrid grid, AstNode node, AstField field) + { + int value = int.TryParse(ShownValue(field), NumberStyles.Integer, CultureInfo.InvariantCulture, out int number) + ? number + : 0; + + if (grid.Value(field.Name, ref value)) + { + HoldEdit(field, value.ToString(CultureInfo.InvariantCulture)); + } + + CommitWhenLeft(node, field); + } + + /// + /// Draws a property that holds a number with a fractional part. + /// + /// The grid the row is drawn in. + /// The node being edited. + /// The property to draw. + private void DrawFractionField(ImGuiWidgets.PropertyGrid grid, AstNode node, AstField field) + { + double value = double.TryParse(ShownValue(field), NumberStyles.Float, CultureInfo.InvariantCulture, out double fraction) + ? fraction + : 0d; + + if (grid.Value(field.Name, ref value)) + { + HoldEdit(field, value.ToString(CultureInfo.InvariantCulture)); + } + + CommitWhenLeft(node, field); + } + + /// + /// Gets the value a field's editor should show: what is being typed into it, or what the document + /// holds when it is not the field being typed into. + /// + /// The property being drawn. + /// The value to show, as text. + /// /// One buffer is shared across every field, keyed by which one is being typed into: only one box /// can have the keyboard at a time, so a buffer per field would be state to keep in step with the /// document for no gain. A field that is not being typed into shows the document's value, so an /// undo while the box is open is reflected rather than overwritten. /// - private void DrawTextField(AstNode node, AstField field) - { - bool editing = string.Equals(editingField, field.Name, StringComparison.Ordinal); - string buffer = editing ? fieldBuffer : field.Value; + private string ShownValue(AstField field) => IsBeingEdited(field) ? fieldBuffer : field.Value; - if (ImGui.InputText(field.Name, ref buffer, 256)) - { - editingField = field.Name; - fieldBuffer = buffer; - } + /// Gets whether a field is the one currently being typed into. + /// The property being drawn. + /// True when the shared buffer holds this field's half-finished value. + private bool IsBeingEdited(AstField field) => string.Equals(editingField, field.Name, StringComparison.Ordinal); + + /// Remembers what has been typed into a field, which is not the document's value yet. + /// The property being edited. + /// What the editor now holds, as text. + private void HoldEdit(AstField field, string value) + { + editingField = field.Name; + fieldBuffer = value; + } - if (ImGui.IsItemDeactivatedAfterEdit()) + /// + /// Writes a held edit to the document once the user has left the editor that made it. + /// + /// The node being edited. + /// The property being drawn, whose editor is the most recent item. + private void CommitWhenLeft(AstNode node, AstField field) + { + if (IsBeingEdited(field) && ImGui.IsItemDeactivatedAfterEdit()) { - SetField(node, field.Name, buffer); + SetField(node, field.Name, fieldBuffer); editingField = null; } } + /// + /// Starts a row the property grid has no method for, in the table it is already drawing. + /// + /// The row's name, drawn in the first column. + /// + /// Two of the inspector's rows are ones the widget does not offer: a choice between arbitrary + /// labelled values, and a count with the buttons that change it. Its combo row spells an option + /// by its enumeration name, where an operator here reads as its symbol beside its name and a + /// visibility the language does not spell reads as "(language default)"; and it has no row that + /// ends in buttons at all. A row is two cells of the table the grid is already inside, though, so + /// composing one leaves these properties in the same two columns, either side of the same + /// divider, as every row the widget draws itself. + /// + private static void BeginComposedRow(string label) + { + ImGui.TableNextRow(); + ImGui.TableSetColumnIndex(0); + ImGui.AlignTextToFramePadding(); + ImGui.TextUnformatted(label); + ImGui.TableSetColumnIndex(1); + } + /// /// Draws a property the user picks from a fixed set, which is how an operator is chosen. /// @@ -486,10 +632,19 @@ private void DrawTextField(AstNode node, AstField field) /// The property to draw. private void DrawChoiceField(AstNode node, AstField field) { + BeginComposedRow(field.Name); + AstFieldChoice? current = field.Choices.FirstOrDefault( choice => string.Equals(choice.Value, field.Value, StringComparison.Ordinal)); - if (!ImGui.BeginCombo(field.Name, current?.Label ?? field.Value)) + ImGui.SetNextItemWidth(ImGui.GetContentRegionAvail().X); + bool open = ImGui.BeginCombo($"##{field.Name}", current?.Label ?? field.Value); + + // Named for the probes the same way the grid names its own rows, so a test finds this one + // beside them rather than having to know it was composed rather than drawn by the widget. + ImGuiProbes.MarkItem(field.Name); + + if (!open) { return; } @@ -501,6 +656,10 @@ private void DrawChoiceField(AstNode node, AstField field) { SetField(node, field.Name, choice.Value); } + + // The options are named for the probes too, so a test picks an operator the way a user + // does — by the label it reads as — rather than by where the list happens to put it. + ImGuiProbes.MarkItem(choice.Label); } ImGui.EndCombo(); @@ -511,10 +670,18 @@ private void DrawChoiceField(AstNode node, AstField field) /// /// The node whose slot to draw. /// The slot to draw. + /// + /// The count is read rather than typed: adding a parameter and removing the last one are the two + /// edits the document knows how to undo, and a box that accepts any number would be asking for a + /// third that neither button can make. + /// private void DrawSlotCount(AstNode node, AstSlot slot) { + BeginComposedRow(slot.Name); + int count = AstSchema.ChildrenOf(node, slot).Count; - ImGui.TextUnformatted($"{slot.Name}: {count}"); + ImGui.AlignTextToFramePadding(); + ImGui.TextUnformatted(count.ToString(CultureInfo.InvariantCulture)); ImGui.SameLine(); if (ImGui.Button($"+##add-{slot.Name}")) @@ -522,6 +689,8 @@ private void DrawSlotCount(AstNode node, AstSlot slot) AddChild(node, slot); } + ImGuiProbes.MarkItem($"Add {slot.Name}"); + ImGui.SameLine(); ImGui.BeginDisabled(count == 0); if (ImGui.Button($"-##remove-{slot.Name}")) @@ -529,6 +698,7 @@ private void DrawSlotCount(AstNode node, AstSlot slot) RemoveLastChild(node, slot); } + ImGuiProbes.MarkItem($"Remove {slot.Name}"); ImGui.EndDisabled(); } diff --git a/Coder.Graph/Coder.Graph.csproj b/Coder.Graph/Coder.Graph.csproj index bfcdebc..79ff441 100644 --- a/Coder.Graph/Coder.Graph.csproj +++ b/Coder.Graph/Coder.Graph.csproj @@ -15,6 +15,11 @@ inherited through the node editor. --> + + + + diff --git a/Coder.Test/Graph/AstGraphEditorInspectorPanelTests.cs b/Coder.Test/Graph/AstGraphEditorInspectorPanelTests.cs new file mode 100644 index 0000000..bd38877 --- /dev/null +++ b/Coder.Test/Graph/AstGraphEditorInspectorPanelTests.cs @@ -0,0 +1,210 @@ +// Copyright (c) 2023-2026 ktsu-dev contributors + +namespace ktsu.Coder.Test.Graph; + +using System.Numerics; + +using Hexa.NET.ImGui; + +using ktsu.Coder.Ast; +using ktsu.Coder.Graph; +using ktsu.ImGui.App; +using ktsu.ImGui.App.Testing; + +using Microsoft.VisualStudio.TestTools.UnitTesting; + +/// +/// Covers the inspector as the user meets it: rows of a property grid, reached with the mouse and +/// the keyboard rather than by calling the editor's methods. +/// +/// +/// What each edit does to the document is covered headlessly in +/// ; what is covered here is that the panel's rows reach +/// those edits at all. The rows come from ktsu.ImGui.Widgets' property grid, which names each of +/// them for the probes, so a test finds a property by the label the user reads. +/// +/// ImGui contexts are process-global, so only one harness can be live at a time and this class must +/// not run its methods in parallel. +/// +/// +[TestClass] +[DoNotParallelize] +public sealed class AstGraphEditorInspectorPanelTests +{ + private static readonly HarnessOptions Options = new() { Width = 1280, Height = 800 }; + + private static FunctionDeclaration SampleFunction() + { + FunctionDeclaration function = new("total") { ReturnType = "int" }; + function.Parameters.Add(new Parameter("a", "int")); + function.Body.Add(new ReturnStatement( + new BinaryExpression(new VariableReference("a"), BinaryOperator.Add, Literal.Number(1)))); + return function; + } + + /// + /// Draws the whole editor, because selecting a node is ImNodes' business and it faults inside + /// native code when its context is missing rather than throwing. + /// + /// The editor to draw. + /// The configuration to hand the harness. + private static ImGuiAppConfig ConfigFor(AstGraphEditor editor) => new() + { + Title = "AST inspector", + OnRender = delta => + { + ImGui.Begin("graph"); + editor.Draw(new Vector2(1200, 700), delta); + ImGui.End(); + }, + }; + + private static bool IsVisible(ImGuiAppHarness harness, string name) => + harness.Probe.WasSeenInFrame(name, harness.FrameCount - 1); + + /// + /// Starts a harness with a node already selected, which is the state the inspector has anything + /// to draw in. + /// + /// The editor to draw. + /// The node to select once the graph has been drawn. + /// The running harness, for the caller to dispose. + private static ImGuiAppHarness Inspecting(AstGraphEditor editor, AstNode node) + { + ImGuiAppHarness harness = ImGuiAppHarness.Start(ConfigFor(editor), Options); + harness.Step(2); + + Assert.IsTrue(editor.Select(node), "the node to inspect should be in the graph"); + harness.Step(2); + return harness; + } + + /// + /// Tests that every property of the selected node is a row of the grid, named for the probes the + /// way it is labelled on screen. + /// + [TestMethod] + public void Inspector_DrawsARowPerProperty() + { + FunctionDeclaration function = SampleFunction(); + AstGraphEditor editor = new(function); + + using ImGuiAppHarness harness = Inspecting(editor, function); + + foreach (string row in new[] { "Name", "ReturnType", "Visibility" }) + { + Assert.IsTrue(IsVisible(harness, row), $"The inspector is missing its '{row}' row."); + } + + // A variadic slot's row is named for the buttons that change it, since the count beside them + // is text rather than an editor the user can reach. + foreach (string slot in new[] { "Parameters", "Body" }) + { + Assert.IsTrue(IsVisible(harness, $"Add {slot}"), $"The inspector is missing its '{slot}' row."); + } + } + + /// + /// Tests that a name typed into its row reaches the document when the box is left, and does so as + /// one undoable step rather than one per character. + /// + [TestMethod] + public void Inspector_CommitsTypedTextWhenTheBoxIsLeft() + { + FunctionDeclaration function = SampleFunction(); + AstGraphEditor editor = new(function); + + using ImGuiAppHarness harness = Inspecting(editor, function); + + harness.Click("Name"); + harness.Step(2); + + harness.Keyboard.Press(ImGuiKey.A, ctrl: true); + harness.Keyboard.Type("renamed"); + harness.Step(2); + + Assert.AreEqual("total", function.Name, "typing should not reach the document until the box is left"); + + harness.Keyboard.Press(ImGuiKey.Enter); + harness.Step(2); + + Assert.AreEqual("renamed", function.Name); + Assert.IsTrue(editor.History.CanUndo); + + editor.Undo(); + Assert.AreEqual("total", function.Name); + Assert.IsFalse(editor.History.CanUndo, "renaming should have been one step, not one per character"); + } + + /// + /// Tests that a flag row is committed as it is clicked, since a checkbox has nowhere to hold a + /// half-made change. + /// + [TestMethod] + public void Inspector_CommitsAFlagAsItIsClicked() + { + FunctionDeclaration function = SampleFunction(); + Parameter parameter = function.Parameters[0]; + AstGraphEditor editor = new(function); + + using ImGuiAppHarness harness = Inspecting(editor, parameter); + + harness.Click("Optional"); + harness.Step(2); + + Assert.IsTrue(parameter.IsOptional); + + editor.Undo(); + Assert.IsFalse(parameter.IsOptional); + } + + /// + /// Tests that an operator is picked by the label it reads as, which is why the choice row is + /// composed rather than drawn by the grid's own enumeration row. + /// + [TestMethod] + public void Inspector_PicksAnOperatorByItsReadableLabel() + { + FunctionDeclaration function = SampleFunction(); + BinaryExpression binary = (BinaryExpression)((ReturnStatement)function.Body[0]).Expression!; + AstGraphEditor editor = new(function); + + using ImGuiAppHarness harness = Inspecting(editor, binary); + + Assert.IsTrue(IsVisible(harness, "Operator"), "The inspector is missing its operator row."); + + harness.Click("Operator"); + harness.Step(2); + + harness.Click("* Multiply"); + harness.Step(2); + + Assert.AreEqual(BinaryOperator.Multiply, binary.Operator); + + editor.Undo(); + Assert.AreEqual(BinaryOperator.Add, binary.Operator); + } + + /// + /// Tests that the buttons on a variadic slot's row grow and shrink it, which is the half of + /// editing that dragging a link cannot express. + /// + [TestMethod] + public void Inspector_AddsAndRemovesChildrenFromASlotRow() + { + FunctionDeclaration function = SampleFunction(); + AstGraphEditor editor = new(function); + + using ImGuiAppHarness harness = Inspecting(editor, function); + + harness.Click("Add Parameters"); + harness.Step(2); + + Assert.AreEqual(2, function.Parameters.Count); + + harness.Click("Remove Parameters"); + harness.Step(2); + + Assert.AreEqual(1, function.Parameters.Count); + } +} diff --git a/Directory.Packages.props b/Directory.Packages.props index 6e9fc6d..a3e1d88 100644 --- a/Directory.Packages.props +++ b/Directory.Packages.props @@ -14,7 +14,11 @@ - + + + From c929aa03988dab94265faac8785a7f5175b5b287 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 9 Sep 2026 07:16:04 +0000 Subject: [PATCH 2/2] Count the parameters with the collection assert MSTEST0037 through Sonar: Assert.HasCount says what is being counted, where Assert.AreEqual against a Count reports only two numbers when it fails. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01UNhZ9nTJqzyTmCySe2ZE5z --- Coder.Test/Graph/AstGraphEditorInspectorPanelTests.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Coder.Test/Graph/AstGraphEditorInspectorPanelTests.cs b/Coder.Test/Graph/AstGraphEditorInspectorPanelTests.cs index bd38877..f48721c 100644 --- a/Coder.Test/Graph/AstGraphEditorInspectorPanelTests.cs +++ b/Coder.Test/Graph/AstGraphEditorInspectorPanelTests.cs @@ -200,11 +200,11 @@ public void Inspector_AddsAndRemovesChildrenFromASlotRow() harness.Click("Add Parameters"); harness.Step(2); - Assert.AreEqual(2, function.Parameters.Count); + Assert.HasCount(2, function.Parameters); harness.Click("Remove Parameters"); harness.Step(2); - Assert.AreEqual(1, function.Parameters.Count); + Assert.HasCount(1, function.Parameters); } }