diff --git a/CLAUDE.md b/CLAUDE.md index 96dab05..0cb9313 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -540,6 +540,36 @@ records its tabs nor takes a selection from outside. A panel behind it - the cla diagnostics list - is tested by drawing it directly in a `WidgetHarness`, which is what the tab's own delegate does. +### What a Linux-only suite stops seeing + +The frame-driven suite runs on Linux alone, and the two tests that first failed elsewhere were +failing for reasons that had nothing to do with the operating system - which is exactly what a +one-platform suite is bad at telling you. + +`FileBrowserTests` expected the path it made its scratch directory under. macOS reaches the +temporary directory through `/var`, a symbolic link to `/private/var`, and the working directory +resolves the link, so the browser answered with the other spelling of the same directory. The test +now reads the working directory back after setting it, which is a fact about symbolic links rather +than about macOS. + +`MenuTests` clicked a recent file whose menu label was its whole path. A menu is as wide as its +widest label, and a submenu that will not fit beside the menu that opened it is placed **on top of +that menu** - which puts the item that opened it under the submenu rather than under the pointer, +so ImGui takes the pointer to have left and closes the submenu on the frame after it opens. The +temporary directory being 44 characters deeper on macOS is all that picked which platform noticed: +any schema saved somewhere deep enough did the same everywhere, and Open Recent was unusable for +whoever owned it. `SchemaEditor.ElideRecentFileLabel` bounds the label and the tooltip keeps the +path. Both halves are pinned - the mechanism in `MenuTests.ARecentFileTooDeepToLabelWholeStillOpens`, +and the elision itself in `Schema.Editor.Test/RecentFileLabelTests.cs`, which needs no frame and so +runs on every platform. That is the answer to promoting the suite: what was platform-sensitive here +was a string, and a string can be tested where the rasterizer cannot go. + +`EditorHarness.Click` asked whether the probe had **ever** recorded a name, not whether the item was +on screen now, so an item drawn once and then gone still satisfied the wait and the click failed +later on a stale rectangle. It now asks `IsOnScreen` on both sides of the settle frames. That is a +better message, not a fix: with the wrong question it failed inside the click, with the right one it +fails at the wait, and only the elision makes it pass. + ## Dependencies - **ktsu.Semantics.Strings/Paths** - Type-safe string and path wrappers diff --git a/Schema.Editor.Test/RecentFileLabelTests.cs b/Schema.Editor.Test/RecentFileLabelTests.cs new file mode 100644 index 0000000..c0e399c --- /dev/null +++ b/Schema.Editor.Test/RecentFileLabelTests.cs @@ -0,0 +1,68 @@ +// Copyright (c) 2023-2026 ktsu-dev contributors + +namespace ktsu.Schema.Editor.Test; + +/// +/// The label a recent file is offered under in the File menu. +/// +/// +/// +/// Offering the whole path was not a display preference that happened to be verbose. A menu is as +/// wide as its widest label, and a submenu that will not fit beside the menu that opened it is +/// placed on top of that menu instead - which puts the item that opened it underneath the submenu +/// rather than under the pointer, so ImGui takes the pointer to have left and closes the submenu +/// on the frame after it opened. Open Recent was therefore unusable for anyone whose schemas lived +/// deep enough, and how deep that was depended on the window's width. +/// +/// +/// Frameless, so it runs on every platform. That matters here more than usual: this was found by +/// two tests failing on macOS alone, for no reason macOS was responsible for - its temporary +/// directory is simply 44 characters deeper than /tmp. +/// +/// +[TestClass] +public sealed class RecentFileLabelTests +{ + [TestMethod] + public void APathThatFitsIsOfferedWhole() + { + string path = new('x', SchemaEditor.MaxRecentFileLabelLength); + + Assert.AreEqual(path, SchemaEditor.ElideRecentFileLabel(path)); + } + + [TestMethod] + public void ALongerPathIsCutToTheSameWidth() + { + string label = SchemaEditor.ElideRecentFileLabel(new string('x', SchemaEditor.MaxRecentFileLabelLength * 4)); + + Assert.AreEqual(SchemaEditor.MaxRecentFileLabelLength, label.Length, "A label past the budget is what makes the menu wider than the window."); + } + + /// + /// The end rather than the beginning: what tells two recent files apart is the file name and + /// the directory holding it, and the root they share is the part worth losing. + /// + [TestMethod] + public void WhatIsKeptIsTheEndOfThePath() + { + string path = $"/a/very/long/root/that/nobody/needs/to/read/again/and/again/and/again/{new string('d', 20)}/schema.json"; + + string label = SchemaEditor.ElideRecentFileLabel(path); + + StringAssert.EndsWith(label, $"{new string('d', 20)}/schema.json", StringComparison.Ordinal); + StringAssert.StartsWith(label, "…", StringComparison.Ordinal); + } + + /// + /// A file name longer than the whole budget still leaves a label of the right width, rather + /// than a negative slice or a label that is once again as wide as the path. + /// + [TestMethod] + public void AFileNameLongerThanTheBudgetIsCutToo() + { + string path = $"/schemas/{new string('n', SchemaEditor.MaxRecentFileLabelLength * 2)}.schema.json"; + + Assert.AreEqual(SchemaEditor.MaxRecentFileLabelLength, SchemaEditor.ElideRecentFileLabel(path).Length); + } +} diff --git a/Schema.Editor/SchemaEditor.Files.cs b/Schema.Editor/SchemaEditor.Files.cs index ef79c11..85ae8b1 100644 --- a/Schema.Editor/SchemaEditor.Files.cs +++ b/Schema.Editor/SchemaEditor.Files.cs @@ -38,6 +38,42 @@ public partial class SchemaEditor ? "Untitled schema" : Path.GetFileName(CurrentSchemaPath); + /// + /// The longest label a recent file is offered under, in characters. + /// + /// + /// + /// A menu is as wide as its widest label, and Open Recent's labels are absolute paths, so its + /// width was whatever depth the user happened to have saved at. That is not merely untidy: a + /// submenu that does not fit beside the menu that opened it is placed on top of it, and the + /// item that opened it is then underneath the submenu rather than under the pointer, which + /// ImGui reads as the pointer having left - so the submenu closes on the frame after it opens + /// and the menu cannot be used at all. + /// + /// + /// Forty is measured rather than chosen for looks: the submenu opens a little past the File + /// menu's own width and grows by about eight pixels a character, so forty leaves it inside the + /// 700-pixel window the narrowest of these tests drives, where sixty does not. The whole path + /// is still a hover away. + /// + /// + internal const int MaxRecentFileLabelLength = 40; + + /// + /// Shortens a path to for display, keeping its end. + /// + /// + /// The end rather than the beginning, because what tells two recent files apart is the file + /// name and the directory holding it. What is dropped is the root they are most likely to + /// share. + /// + /// The path to label. + /// The path itself if it fits, or its last characters behind an ellipsis. + internal static string ElideRecentFileLabel(string path) => + path.Length <= MaxRecentFileLabelLength + ? path + : $"…{path[^(MaxRecentFileLabelLength - 1)..]}"; + private void ShowRecentFilesMenu() { IReadOnlyList recent = [.. Options.RecentFiles]; @@ -58,12 +94,23 @@ private void ShowRecentFilesMenu() } anyShown = true; - bool clicked = ImGui.MenuItem(path); + + // The whole path is the id and the elided path is the label, so two files with the + // same name stay separate items however alike they read. + ImGui.PushID(path); + bool clicked = ImGui.MenuItem(ElideRecentFileLabel(path)); // Recorded under the file name rather than the whole path, which a probe name would // otherwise read as a chain of scopes because both are separated by slashes. ImGuiProbes.MarkItem("recent", Path.GetFileName(path)); + if (ImGui.IsItemHovered()) + { + ImGui.SetTooltip(path); + } + + ImGui.PopID(); + if (clicked) { AbsoluteFilePath captured = path; diff --git a/tests/Schema.Editor.UITests/EditorHarness.cs b/tests/Schema.Editor.UITests/EditorHarness.cs index 670c990..f40d808 100644 --- a/tests/Schema.Editor.UITests/EditorHarness.cs +++ b/tests/Schema.Editor.UITests/EditorHarness.cs @@ -100,16 +100,27 @@ internal void StepUntil(Func condition, string description, int maxFrames /// Waits for a marked item to be drawn, then clicks it. /// /// + /// /// The frames between the item first appearing and the click are not padding. A modal sizes /// itself from its contents on the frame it appears and is centred on the next, so the /// rectangle recorded for a control on its first frame is not where that control ends up; /// clicking there hits the background instead. + /// + /// + /// Both waits ask rather than whether the probe has ever + /// recorded the name, and the second one is why there are two: an item can be drawn once and + /// then go away again while those frames run, and the name it left behind is enough to satisfy + /// a wait that only asks whether the probe has heard of it. Asked this way, a test that clicks + /// something no longer there says so, instead of failing inside the click on a rectangle that + /// has since been taken by whatever moved into it. + /// /// /// A marked name, or the trailing part of one. internal void Click(string item) { - StepUntil(() => App.Probe.Matches(item).Count > 0, $"'{item}' appearing"); + StepUntil(() => IsOnScreen(item), $"'{item}' appearing"); App.Step(3); + StepUntil(() => IsOnScreen(item), $"'{item}' still being drawn once it had settled"); App.Click(item); App.Step(2); } diff --git a/tests/Schema.Editor.UITests/FileBrowserTests.cs b/tests/Schema.Editor.UITests/FileBrowserTests.cs index 3aad4fd..12cb4da 100644 --- a/tests/Schema.Editor.UITests/FileBrowserTests.cs +++ b/tests/Schema.Editor.UITests/FileBrowserTests.cs @@ -39,6 +39,12 @@ public void StartEditor() previousWorkingDirectory = Directory.GetCurrentDirectory(); Directory.SetCurrentDirectory(scratchDirectory); + // Read the directory back rather than keeping the name it was created under. On macOS the + // temporary directory is reached through /var, which is a symbolic link to /private/var, + // and the working directory resolves the link - so the browser, which opens on the working + // directory, answers with the spelling that the name it was created under is not. + scratchDirectory = Directory.GetCurrentDirectory().As(); + harness = EditorHarness.Start(); } diff --git a/tests/Schema.Editor.UITests/MenuTests.cs b/tests/Schema.Editor.UITests/MenuTests.cs index 5a56212..e3415c2 100644 --- a/tests/Schema.Editor.UITests/MenuTests.cs +++ b/tests/Schema.Editor.UITests/MenuTests.cs @@ -225,6 +225,44 @@ public void OpeningARecentFileLoadsIt() Assert.AreEqual("Recalled", harness.Editor.CurrentClass?.Name.ToString()); } + /// + /// A schema saved somewhere deep is still offered and still opens. + /// + /// + /// Labelled with its whole path, such a file made Open Recent wider than the room beside the + /// File menu, and a submenu that does not fit beside the menu that opened it is placed on top + /// of it - so the item that opened it sat under the submenu rather than under the pointer, + /// ImGui took the pointer to have left, and the submenu closed on the frame after it opened. + /// This is the case that found it, in the only form that says what it was: the two tests it + /// broke failed on macOS alone, whose temporary directory is 44 characters deeper than + /// /tmp and which had nothing else to do with it. + /// + [TestMethod] + public void ARecentFileTooDeepToLabelWholeStillOpens() + { + AbsoluteDirectoryPath deep = scratchDirectory; + for (int level = 0; level < 3; level++) + { + deep /= $"a-directory-named-at-length-{level}".As(); + } + + Directory.CreateDirectory(deep); + + Schema source = new(); + source.AddClass("Deep".As()); + AbsoluteFilePath path = deep / "deep.schema.json".As(); + File.WriteAllText(path, SchemaSerializer.Serialize(source)); + harness.Editor.Options.RecordRecentFile(path); + + Assert.IsGreaterThan(SchemaEditor.MaxRecentFileLabelLength, path.ToString().Length, "The path was not long enough to be the case this pins."); + + harness.ChooseMenuItem("File", "Open Recent"); + harness.Click("recent/deep.schema.json"); + + Assert.AreEqual(path, harness.Editor.CurrentSchemaPath); + Assert.AreEqual("Deep", harness.Editor.CurrentClass?.Name.ToString()); + } + [TestMethod] public void UndoFromTheEditMenuRevertsTheLastEdit() {