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..f48721c --- /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.HasCount(2, function.Parameters); + + harness.Click("Remove Parameters"); + harness.Step(2); + + Assert.HasCount(1, function.Parameters); + } +} diff --git a/Directory.Packages.props b/Directory.Packages.props index e906696..70e9c08 100644 --- a/Directory.Packages.props +++ b/Directory.Packages.props @@ -10,14 +10,15 @@ - + - - - + + + + - - + +