Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: TOC items order issue #10585

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open

fix: TOC items order issue #10585

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
53b74f3
fix: toc items orders issue
filzrev Mar 7, 2025
fd6774c
test(snapshot): update snapshots d643f62958c90da131e35a613d8dc476ab8a…
filzrev Mar 11, 2025
9097727
chore: add test class for issue10332
filzrev Mar 11, 2025
1b04af4
test(snapshot): update snapshots d6e53ef16df420422b345380b5e6b4e8ed73…
filzrev Mar 11, 2025
42e8d00
Update test/Docfx.Dotnet.Tests/Docfx.Dotnet.Tests.csproj
filzrev Mar 27, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
41 changes: 37 additions & 4 deletions samples/seed/dotnet/project/Project/Class1.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ public class Test<T> { }
/// </summary>
/// <include file='../../docs.xml' path='docs/members[@name="MyTests"]/Test/*'/>
public void XmlCommentIncludeTag() { }

/// <summary>
/// Test
/// </summary>
Expand Down Expand Up @@ -101,7 +101,7 @@ public void Issue2723() { }
/// </remarks>
public void Issue4392() { }

public void Issue8764<T>() where T: unmanaged { }
public void Issue8764<T>() where T : unmanaged { }

public class Issue8665
{
Expand Down Expand Up @@ -191,6 +191,39 @@ public enum Issue9260
[Obsolete("Use Value")]
OldAndUnusedValue2,
}

public class Issue10332
{
public void IRoutedView() { }
public void IRoutedViewModel() { }
public void IRoutedView<TViewModel>() { }

public void Null(object? obj) { }
public void Null<T>(T obj) { }
public void NullOrEmpty(string text) { }

public void Method() { }
public void Method(int a) { }
public void Method(int a, int b) { }

public enum SampleEnum
{
/// <summary>
/// 3rd element when sorted by alphabetic order
/// </summary>
AAA,

/// <summary>
/// 2nd element when sorted by alphabetic order
/// </summary>
aaa,

/// <summary>
/// 1st element when sorted by alphabetic order
/// </summary>
_aaa,
}
}
}

class ExperimentalAttribute : Attribute
Expand All @@ -209,10 +242,10 @@ public ExperimentalAttribute(string diagnosticId)
public class Issue8725
{
/// <summary>A nice operation</summary>
public void MyOperation() {}
public void MyOperation() { }

/// <summary>Another nice operation</summary>
public void MoreOperations() {}
public void MoreOperations() { }
}

/// <summary>
Expand Down
216 changes: 216 additions & 0 deletions src/Docfx.Dotnet/Helpers/SymbolStringComparer.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,216 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

#nullable enable

namespace Docfx.Dotnet;

/// <summary>
/// StringComparer that simulate <see cref="StringComparer.InvariantCulture"> behavior for ASCII chars.
/// </summary>
/// <remarks>
/// .NET StringComparer ignores non-printable chars on string comparison
/// (e.g. StringComparer.InvariantCulture.Compare("\x0000 ZZZ \x0000"," ZZZ ")) returns 0).
/// This feature is not implement by this comparer.
/// </remarks>
internal sealed class SymbolStringComparer : IComparer<string>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems to be occurred when using "outputFormat": "mref" options.
When using apiPage or markdown options. It works as expected.

The apiPage output does not use a custom string comparer and seems to sort by name directly, does it work to sort by name also for mref output?

Copy link
Contributor Author

@filzrev filzrev Mar 28, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ApiPage resolve names with SymbolFormatter.GetName(symbol, SyntaxLanguage.CSharp)
So name is represented as followings.

Class1
Class1.IRoutedView
Class1.IRoutedView<T>
Class1.IRoutedViewModel
Class1.IRoutedViewModelBase

So generics are sorted as expected when it's sorted by ordinal .
Because < char code is smaller than alphabet char.

On the other hand, when using Mref output format.
name is represented as followings.

BuildFromProject.Class1
BuildFromProject.Class1.IRoutedView
BuildFromProject.Class1.IRoutedViewModel
BuildFromProject.Class1.IRoutedViewModelBase
BuildFromProject.Class1.IRoutedView`1

So it requires SymbolStringComparer to get expected results.
I thought SymbolStringComparer should be also applied to ApiPage/Markdown output format.

{
public static readonly SymbolStringComparer Instance = new();

private SymbolStringComparer() { }

public int Compare(string? x, string? y)
{
if (ReferenceEquals(x, y)) return 0;
if (x == null) return -1;
if (y == null) return 1;

var xSpan = x.AsSpan();
var ySpan = y.AsSpan();

int minLength = Math.Min(xSpan.Length, ySpan.Length);

int savedFirstDiff = 0;
for (int i = 0; i < minLength; ++i)
{
var xChar = xSpan[i];
var yChar = ySpan[i];

if (xChar == yChar)
continue;

if (char.IsAscii(xChar) && char.IsAscii(yChar))
{
// Gets custom char order
var xOrder = AsciiCharSortOrders[xChar];
var yOrder = AsciiCharSortOrders[yChar];

var result = xOrder.CompareTo(yOrder);
if (result == 0)
continue;

// Custom order logics for method parameters.
if ((xChar == ',' && yChar == ')') || (xChar == ')' && yChar == ','))
return -result; // Returns result with inverse sign.

// Save first char case comparison result. (To simulate `StringComparer.InvariantCulture` behavior).
if (char.ToUpper(xChar) == char.ToUpper(yChar))
{
if (savedFirstDiff == 0)
savedFirstDiff = result;
continue;
}

return result;
}
else
{
// Compare non-ASCII char with ordinal order
int result = xChar.CompareTo(yChar);
if (result != 0)
return result;
}
}

// Return saved result if case difference exists and string length is same.
if (savedFirstDiff != 0 && x.Length == y.Length)
return savedFirstDiff;

// Otherwise compare text length.
return x.Length.CompareTo(y.Length);
}

// ASCII character order lookup table.
// This table is based on StringComparer.InvariantCulture's charactor sort order.
private static readonly byte[] AsciiCharSortOrders =
[
0, // NUL
0, // SOH
0, // STX
0, // ETX
0, // EOT
0, // ENQ
0, // ACK
0, // BEL
0, // BS
28, // HT (Horizontal Tab)
29, // LF (Line Feed)
30, // VT (Vertical Tab)
31, // FF (Form Feed)
32, // CR (Carriage Return)
0, // SO
0, // SI
0, // DLE
0, // DC1
0, // DC2
0, // DC3
0, // DC4
0, // NAK
0, // SYN
0, // ETB
0, // CAN
0, // EM
0, // SUB
0, // ESC
0, // FS
0, // GS
0, // RS
0, // US
33, // SP (Space)
39, // !
43, // "
55, // #
65, // $
56, // %
54, // &
42, // '
44, // (
45, // )
51, // *
59, // +
36, // ,
35, // -
41, // .
52, // /
66, // 0
67, // 1
68, // 2
69, // 3
70, // 4
71, // 5
72, // 6
73, // 7
74, // 8
75, // 9
38, // :
37, // ;
60, // <
61, // =
62, // >
40, // ?
50, // @
77, // A
79, // B
81, // C
83, // D
85, // E
87, // F
89, // G
91, // H
93, // I
95, // J
97, // K
99, // L
101, // M
103, // N
105, // O
107, // P
109, // Q
111, // R
113, // S
115, // T
117, // U
119, // V
121, // W
123, // X
125, // Y
127, // Z
46, // [
53, // \
47, // ]
58, // ^
34, // _
57, // `
76, // a
78, // b
80, // c
82, // d
84, // e
86, // f
88, // g
90, // h
92, // i
94, // j
96, // k
98, // l
100, // m
102, // n
104, // o
106, // p
108, // q
110, // r
112, // s
114, // t
116, // u
118, // v
120, // w
122, // x
124, // y
126, // z
48, // {
63, // |
49, // }
64, // ~
0, // ESC
];
}
16 changes: 7 additions & 9 deletions src/Docfx.Dotnet/YamlViewModelExtensions.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -114,15 +114,12 @@ public static List<TocItemViewModel> ToTocViewModel(this MetadataItem item, stri
{
case MemberType.Toc:
case MemberType.Namespace:
var result = new List<TocItemViewModel>();
foreach (var child in item.Items
return item.Items
.OrderBy(x => x.Type == MemberType.Namespace ? 0 : 1)
.ThenBy(x => x.Name)
)
{
result.Add(child.ToTocItemViewModel(parentNamespace));
}
return result;
.ThenBy(x => x.Name, SymbolStringComparer.Instance)
.Select(x => x.ToTocItemViewModel(parentNamespace))
.ToList();

default:
return null;
}
Expand Down Expand Up @@ -239,7 +236,8 @@ public static ItemViewModel ToItemViewModel(this MetadataItem model, ExtractMeta

var children = model.Type is MemberType.Enum && config.EnumSortOrder is EnumSortOrder.DeclaringOrder
? model.Items?.Select(x => x.Name).ToList()
: model.Items?.Select(x => x.Name).OrderBy(s => s, StringComparer.Ordinal).ToList();
: model.Items?.Select(x => x.Name).OrderBy(s => s, SymbolStringComparer.Instance).ToList();


var result = new ItemViewModel
{
Expand Down
6 changes: 6 additions & 0 deletions test/Docfx.Dotnet.Tests/Docfx.Dotnet.Tests.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,10 @@
<Project Sdk="Microsoft.NET.Sdk">

<PropertyGroup>
<!-- Use following setting when running tests without Globalization Invariant Mode. -->
<!--<InvariantGlobalization>false</InvariantGlobalization>-->
</PropertyGroup>

<ItemGroup>
<None Include="TestDataReferences\**" CopyToOutputDirectory="PreserveNewest" />
</ItemGroup>
Expand Down
53 changes: 53 additions & 0 deletions test/Docfx.Dotnet.Tests/SymbolStringComparerTest.TestData.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using Xunit;

#nullable enable

namespace Docfx.Dotnet.Tests;

public partial class SymbolStringComparerTest
{
private static class TestData
{
/// <summary>
/// Test data for string array order tests.
/// </summary>
public static TheoryData<string[]> StringArrays =>
[
// Contains underscore
[
"__",
"__a",
"_1",
"1_",
"a_a",
"A_a",
"a_aa",
"a_ab",
"aaa"
],
// Case differences
[
"aaa",
"AAA",
"AAA<ABC>",
"AAAA",
"aaab",
],
// Mixed generics
[
"IRoutedView",
"IRoutedView_`1",
"IRoutedView`1",
"IRoutedView<TViewModel>",
"IRoutedView1",
"IRoutedViewModel",
"Null(object? obj)",
"Null<T>(T obj)",
"NullOrEmpty(string? text)",
],
];
}
}
Loading