From 800efc6e105ce4a94f25a335938c53927f3cb4b6 Mon Sep 17 00:00:00 2001 From: Siegfried Pammer Date: Mon, 27 Jul 2026 19:02:41 +0200 Subject: [PATCH] Fix #749: toggle a member's fold together with its documentation Toggle folding picked the innermost fold containing the offset, but a member's logical region is fragmented: the body fold starts at the opening brace, the XML documentation has an independent fold, and the header line belongs to neither, so toggling there collapsed the whole enclosing type. Visual Studio's source editor keeps documentation regions independent, but its metadata-as-source view treats the member's leading trivia as one hideable unit; for a read-only decompiler view the grouped behavior is the intuitive one. The writer now records where an entity declaration begins, and the definition's fold carries that logical start. Toggling targets the fold whose logical region innermost-contains the offset, so the header line targets the member rather than the type, and leading documentation folds follow the member fold's new state. Inside the documentation the doc fold itself is the innermost region and still toggles alone. Toggle all folding now follows Visual Studio's Toggle All Outlining parity: a mixed state expands everything, a uniform state flips. Assisted-by: Claude:claude-fable-5:Claude Code --- .../Output/TextTokenWriterTests.cs | 4 + ICSharpCode.Decompiler/Output/ITextOutput.cs | 6 + .../Output/PlainTextOutput.cs | 11 +- .../Output/TextTokenWriter.cs | 7 + ILSpy.Tests/Editor/FoldingGroupTests.cs | 182 ++++++++++++++++++ ILSpy/TextView/AvaloniaEditTextOutput.cs | 32 ++- ILSpy/TextView/DecompilerTextView.axaml.cs | 59 ++++-- ILSpy/TextView/DefinitionNewFolding.cs | 33 ++++ 8 files changed, 313 insertions(+), 21 deletions(-) create mode 100644 ILSpy.Tests/Editor/FoldingGroupTests.cs create mode 100644 ILSpy/TextView/DefinitionNewFolding.cs diff --git a/ICSharpCode.Decompiler.Tests/Output/TextTokenWriterTests.cs b/ICSharpCode.Decompiler.Tests/Output/TextTokenWriterTests.cs index 04320f810..20d3109f3 100644 --- a/ICSharpCode.Decompiler.Tests/Output/TextTokenWriterTests.cs +++ b/ICSharpCode.Decompiler.Tests/Output/TextTokenWriterTests.cs @@ -107,6 +107,10 @@ namespace ICSharpCode.Decompiler.Tests.Output LocalReferences.Add((text, reference, isDefinition)); } + public void MarkDefinitionStart() + { + } + public void MarkFoldStart(string collapsedText = "...", bool defaultCollapsed = false, bool isDefinition = false) { FoldStartDefaultCollapsed.Add(defaultCollapsed); diff --git a/ICSharpCode.Decompiler/Output/ITextOutput.cs b/ICSharpCode.Decompiler/Output/ITextOutput.cs index 08f881fe2..83d0ea89c 100644 --- a/ICSharpCode.Decompiler/Output/ITextOutput.cs +++ b/ICSharpCode.Decompiler/Output/ITextOutput.cs @@ -39,6 +39,12 @@ namespace ICSharpCode.Decompiler void WriteLocalReference(string text, object reference, bool isDefinition = false, bool isHoverOnly = false); void MarkFoldStart(string collapsedText = "...", bool defaultCollapsed = false, bool isDefinition = false); + /// + /// Marks the position where an entity declaration begins. The next fold marked with + /// isDefinition: true logically extends back to this position, so that leading + /// documentation comments and attributes count as part of the definition's region. + /// + void MarkDefinitionStart(); void MarkFoldEnd(); } diff --git a/ICSharpCode.Decompiler/Output/PlainTextOutput.cs b/ICSharpCode.Decompiler/Output/PlainTextOutput.cs index f4fa3aaba..f24203fed 100644 --- a/ICSharpCode.Decompiler/Output/PlainTextOutput.cs +++ b/ICSharpCode.Decompiler/Output/PlainTextOutput.cs @@ -152,6 +152,10 @@ namespace ICSharpCode.Decompiler Write(text); } + void ITextOutput.MarkDefinitionStart() + { + } + void ITextOutput.MarkFoldStart(string collapsedText, bool defaultCollapsed, bool isDefinition) { } @@ -199,9 +203,14 @@ namespace ICSharpCode.Decompiler actions.Add(target => target.MarkFoldEnd()); } + public void MarkDefinitionStart() + { + actions.Add(target => target.MarkDefinitionStart()); + } + public void MarkFoldStart(string collapsedText = "...", bool defaultCollapsed = false, bool isDefinition = false) { - actions.Add(target => target.MarkFoldStart(collapsedText, defaultCollapsed)); + actions.Add(target => target.MarkFoldStart(collapsedText, defaultCollapsed, isDefinition)); } public void Unindent() diff --git a/ICSharpCode.Decompiler/Output/TextTokenWriter.cs b/ICSharpCode.Decompiler/Output/TextTokenWriter.cs index 187e6ea8a..134fcc725 100644 --- a/ICSharpCode.Decompiler/Output/TextTokenWriter.cs +++ b/ICSharpCode.Decompiler/Output/TextTokenWriter.cs @@ -534,6 +534,13 @@ namespace ICSharpCode.Decompiler lastUsingDeclaration = false; } } + if (node is EntityDeclaration) + { + // The declaration's logical region starts here, before its documentation + // comments and attributes are written; the body fold marked later refers + // back to this position for group toggling in the UI. + output.MarkDefinitionStart(); + } nodeStack.Push(node); } diff --git a/ILSpy.Tests/Editor/FoldingGroupTests.cs b/ILSpy.Tests/Editor/FoldingGroupTests.cs new file mode 100644 index 000000000..f430534ce --- /dev/null +++ b/ILSpy.Tests/Editor/FoldingGroupTests.cs @@ -0,0 +1,182 @@ +// Copyright (c) 2026 Siegfried Pammer +// +// Permission is hereby granted, free of charge, to any person obtaining a copy of this +// software and associated documentation files (the "Software"), to deal in the Software +// without restriction, including without limitation the rights to use, copy, modify, merge, +// publish, distribute, sublicense, and/or sell copies of the Software, and to permit persons +// to whom the Software is furnished to do so, subject to the following conditions: +// +// The above copyright notice and this permission notice shall be included in all copies or +// substantial portions of the Software. +// +// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR IMPLIED, +// INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS FOR A PARTICULAR +// PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE +// FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR +// OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER +// DEALINGS IN THE SOFTWARE. + +using System; +using System.IO; +using System.Linq; +using System.Threading.Tasks; + +using Avalonia.Headless.NUnit; +using Avalonia.VisualTree; + +using AwesomeAssertions; + +using ICSharpCode.ILSpy.AppEnv; +using ICSharpCode.ILSpy.TextView; +using ICSharpCode.ILSpy.TreeNodes; +using ICSharpCode.ILSpy.ViewModels; +using ICSharpCode.ILSpy.Views; + +using NUnit.Framework; + +namespace ICSharpCode.ILSpy.Tests.TextView; + +/// +/// Sample type decompiled by . Its XML documentation is +/// supplied by a hand-written documentation file placed next to the test assembly, so the +/// decompiled view renders a '///' fold above . +/// +public class FoldGroupSample +{ + public void Documented(int value) + { + Console.WriteLine(value); + } + + public void Plain() + { + Console.WriteLine(); + } +} + +/// +/// Pins the grouped fold-toggle behavior (issue #749): a member and its XML documentation +/// comment toggle as one logical unit, the toggle targets the member (not the enclosing +/// type) when invoked on the header line, the doc fold alone toggles when invoked inside +/// it, and Toggle All uses VS parity (mixed state expands everything). +/// +[TestFixture] +public class FoldingGroupTests +{ + [OneTimeSetUp] + public void WriteDocumentationFile() + { + string assemblyPath = typeof(FoldGroupSample).Assembly.Location; + string xmlPath = Path.ChangeExtension(assemblyPath, ".xml"); + string assemblyName = Path.GetFileNameWithoutExtension(assemblyPath); + File.WriteAllText(xmlPath, $""" + + + {assemblyName} + + + Summary line used by FoldingGroupTests. + Parameter line used by FoldingGroupTests. + + + + """); + } + + static async Task<(DecompilerTextView View, string Text)> SetupAsync() + { + var (window, vm) = await TestHarness.BootAsync(); + // Start from a fully expanded document so every toggle direction is deterministic. + var displaySettings = AppComposition.Current.GetExport().DisplaySettings; + displaySettings.ExpandMemberDefinitions = true; + displaySettings.ExpandXmlDocumentationComments = true; + await vm.OpenAssemblyAsync(typeof(FoldGroupSample).Assembly.Location); + var typeNode = vm.AssemblyTreeModel.FindNode( + "ILSpy.Tests", + "ICSharpCode.ILSpy.Tests.TextView", + "ICSharpCode.ILSpy.Tests.TextView.FoldGroupSample"); + vm.AssemblyTreeModel.SelectNode(typeNode); + var tab = await vm.DockWorkspace.WaitForDecompiledTextAsync(); + var view = window.GetVisualDescendants().OfType().First(); + string text = view.Editor.Document.Text; + text.Should().Contain("Summary line used by FoldingGroupTests", + "the hand-written documentation file must be picked up for these tests to be meaningful"); + return (view, text); + } + + static int OffsetOf(string text, string needle) + { + int offset = text.IndexOf(needle, StringComparison.Ordinal); + offset.Should().BeGreaterThan(-1, $"the decompiled text must contain '{needle}'"); + return offset; + } + + [AvaloniaTest] + public async Task Toggling_In_The_Body_Folds_Member_And_Documentation_Together() + { + var (view, text) = await SetupAsync(); + int docOffset = OffsetOf(text, "Summary line"); + int bodyOffset = OffsetOf(text, "Console.WriteLine(value)"); + int plainBodyOffset = OffsetOf(text, "Console.WriteLine()"); + + view.ToggleFoldingAt(bodyOffset); + + view.IsFoldedAt(bodyOffset).Should().BeTrue("the member body must fold"); + view.IsFoldedAt(docOffset).Should().BeTrue("the documentation comment must fold together with its member"); + view.IsFoldedAt(plainBodyOffset).Should().BeFalse("the sibling member is not part of the group"); + + view.ToggleFoldingAt(bodyOffset); + + view.IsFoldedAt(bodyOffset).Should().BeFalse("toggling again must unfold the member body"); + view.IsFoldedAt(docOffset).Should().BeFalse("toggling again must unfold the documentation comment"); + } + + [AvaloniaTest] + public async Task Toggling_On_The_Header_Line_Targets_The_Member_Not_The_Type() + { + var (view, text) = await SetupAsync(); + int headerOffset = OffsetOf(text, "void Documented"); + int bodyOffset = OffsetOf(text, "Console.WriteLine(value)"); + int plainBodyOffset = OffsetOf(text, "Console.WriteLine()"); + + view.ToggleFoldingAt(headerOffset); + + view.IsFoldedAt(bodyOffset).Should().BeTrue("the member body must fold from its header line"); + view.IsFoldedAt(plainBodyOffset).Should().BeFalse( + "the enclosing type must not fold when the toggle targets a member header"); + } + + [AvaloniaTest] + public async Task Toggling_Inside_The_Documentation_Folds_Only_The_Documentation() + { + var (view, text) = await SetupAsync(); + int docOffset = OffsetOf(text, "Summary line"); + int bodyOffset = OffsetOf(text, "Console.WriteLine(value)"); + + view.ToggleFoldingAt(docOffset); + + view.IsFoldedAt(docOffset).Should().BeTrue("the documentation fold must collapse"); + view.IsFoldedAt(bodyOffset).Should().BeFalse("the member body stays open when only the docs are toggled"); + } + + [AvaloniaTest] + public async Task Toggle_All_Expands_Everything_From_A_Mixed_State() + { + var (view, text) = await SetupAsync(); + int bodyOffset = OffsetOf(text, "Console.WriteLine(value)"); + + // Create a mixed state: one group folded, the rest open. + view.ToggleFoldingAt(bodyOffset); + view.FoldedFoldingCount.Should().BeGreaterThan(0); + view.GetFoldingsForTest().Should().Contain(f => !f.IsFolded, "the state must be mixed for this test"); + + view.ToggleAllFoldings(); + + view.FoldedFoldingCount.Should().Be(0, "VS parity: a mixed state expands all folds"); + + view.ToggleAllFoldings(); + + view.GetFoldingsForTest().Should().OnlyContain(f => f.IsFolded, + "a uniformly expanded document collapses everything"); + } +} diff --git a/ILSpy/TextView/AvaloniaEditTextOutput.cs b/ILSpy/TextView/AvaloniaEditTextOutput.cs index b4927b029..71e076651 100644 --- a/ILSpy/TextView/AvaloniaEditTextOutput.cs +++ b/ILSpy/TextView/AvaloniaEditTextOutput.cs @@ -254,17 +254,35 @@ namespace ICSharpCode.ILSpy.TextView }); } + int pendingDefinitionStart = -1; + + public void MarkDefinitionStart() + { + pendingDefinitionStart = builder.Length; + } + public void MarkFoldStart(string collapsedText = "...", bool defaultCollapsed = false, bool isDefinition = false) { WriteIndentIfNeeded(); - openFoldings.Push(( - new NewFolding { + NewFolding folding; + if (isDefinition && pendingDefinitionStart >= 0) + { + // The definition's logical region reaches back to the entity's first output + // character, so leading documentation folds count as part of the group. + folding = new DefinitionNewFolding { StartOffset = builder.Length, - Name = collapsedText, - DefaultClosed = defaultCollapsed, - IsDefinition = isDefinition, - }, - lineNumber)); + DefinitionStartOffset = pendingDefinitionStart, + }; + pendingDefinitionStart = -1; + } + else + { + folding = new NewFolding { StartOffset = builder.Length }; + } + folding.Name = collapsedText; + folding.DefaultClosed = defaultCollapsed; + folding.IsDefinition = isDefinition; + openFoldings.Push((folding, lineNumber)); } public void MarkFoldEnd() diff --git a/ILSpy/TextView/DecompilerTextView.axaml.cs b/ILSpy/TextView/DecompilerTextView.axaml.cs index 1d94dfaa5..e987f9f55 100644 --- a/ILSpy/TextView/DecompilerTextView.axaml.cs +++ b/ILSpy/TextView/DecompilerTextView.axaml.cs @@ -457,8 +457,22 @@ namespace ICSharpCode.ILSpy.TextView /// Toggles the innermost fold containing the caret (the "Toggle folding" command / Ctrl+M). public void ToggleFoldingAtCaret() => ToggleFoldingAt(Editor.TextArea.Caret.Offset); - /// Toggles the innermost fold containing . The right-click menu - /// passes the offset under the pointer so it acts on the clicked line, not the caret line. + /// The offset where a fold's logical region begins: definition folds reach back to + /// their entity's first character (leading documentation comments and attributes included); + /// every other fold starts at its own first character. + static int GetLogicalStart(FoldingSection folding) + { + return folding.Tag is DefinitionNewFolding definition + ? Math.Min(definition.DefinitionStartOffset, folding.StartOffset) + : folding.StartOffset; + } + + /// Toggles the fold whose logical region innermost-contains . + /// The right-click menu passes the offset under the pointer so it acts on the clicked line, + /// not the caret line. A definition fold's logical region includes its header and leading + /// documentation, so toggling from the header line targets the member (not the enclosing + /// type), and the member's documentation folds toggle together with it. With the caret + /// inside the documentation, the doc fold itself is the innermost region and toggles alone. public void ToggleFoldingAt(int offset) { if (activeFoldingManager is not { } mgr) @@ -466,30 +480,49 @@ namespace ICSharpCode.ILSpy.TextView FoldingSection? target = null; foreach (var f in mgr.AllFoldings) { - if (f.StartOffset <= offset && offset <= f.EndOffset) + if (GetLogicalStart(f) > offset || offset > f.EndOffset) + continue; + if (target == null + || GetLogicalStart(f) > GetLogicalStart(target) + || (GetLogicalStart(f) == GetLogicalStart(target) && f.EndOffset < target.EndOffset)) + { + target = f; + } + } + if (target == null) + return; + bool folded = !target.IsFolded; + target.IsFolded = folded; + // Drag the attached leading folds (XML documentation) along with their member. + int logicalStart = GetLogicalStart(target); + if (logicalStart < target.StartOffset) + { + foreach (var f in mgr.AllFoldings) { - if (target == null || f.StartOffset > target.StartOffset) - target = f; + if (f != target && f.StartOffset >= logicalStart && f.EndOffset <= target.StartOffset) + f.IsFolded = folded; } } - if (target != null) - target.IsFolded = !target.IsFolded; } - /// Collapses every fold when any is open, otherwise expands them all ("Toggle all folding" - /// / Ctrl+Shift+M). + /// Sets all folds to the same state ("Toggle all folding" / Ctrl+Shift+M), with + /// Visual Studio's Toggle All Outlining parity: a mixed state expands everything, a uniform + /// state flips. public void ToggleAllFoldings() { if (activeFoldingManager is not { } mgr) return; - bool anyOpen = false; + bool anyOpen = false, anyFolded = false; foreach (var f in mgr.AllFoldings) { - if (!f.IsFolded) - { anyOpen = true; break; } + if (f.IsFolded) + anyFolded = true; + else + anyOpen = true; } + bool folded = anyOpen && anyFolded ? false : anyOpen; foreach (var f in mgr.AllFoldings) - f.IsFolded = anyOpen; + f.IsFolded = folded; } void OnEditorKeyDownForZoom(object? sender, KeyEventArgs e) diff --git a/ILSpy/TextView/DefinitionNewFolding.cs b/ILSpy/TextView/DefinitionNewFolding.cs new file mode 100644 index 000000000..8a9c396f7 --- /dev/null +++ b/ILSpy/TextView/DefinitionNewFolding.cs @@ -0,0 +1,33 @@ +// Copyright (c) 2026 Siegfried Pammer +// +// Permission is hereby granted, free of charge, to any person obtaining a copy of this +// software and associated documentation files (the "Software"), to deal in the Software +// without restriction, including without limitation the rights to use, copy, modify, merge, +// publish, distribute, sublicense, and/or sell copies of the Software, and to permit persons +// to whom the Software is furnished to do so, subject to the following conditions: +// +// The above copyright notice and this permission notice shall be included in all copies or +// substantial portions of the Software. +// +// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR IMPLIED, +// INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS FOR A PARTICULAR +// PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE +// FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR +// OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER +// DEALINGS IN THE SOFTWARE. + +using AvaloniaEdit.Folding; + +namespace ICSharpCode.ILSpy.TextView +{ + /// + /// A definition-body fold whose logical region starts before the fold itself: at the first + /// output character of its entity declaration, so leading XML documentation comments and + /// attributes belong to the same toggle group. AvaloniaEdit keeps the instance reachable + /// through . + /// + sealed class DefinitionNewFolding : NewFolding + { + public int DefinitionStartOffset { get; set; } + } +}