Skip to content

Commit afd5456

Browse files
committed
Added explicit support for folder with an index field
1 parent f4904ef commit afd5456

File tree

3 files changed

+319
-14
lines changed

3 files changed

+319
-14
lines changed

src/Elastic.Documentation.Configuration/DocSet/DocumentationSetFile.cs

Lines changed: 58 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -222,16 +222,55 @@ private static ITableOfContentsItem ResolveFileRef(
222222
{
223223
var fullPath = string.IsNullOrEmpty(parentPath) ? fileRef.Path : $"{parentPath}/{fileRef.Path}";
224224

225+
// Special validation for FolderIndexFileRef (folder+file combination)
226+
// Validate BEFORE early return so we catch cases with no children
227+
if (fileRef is FolderIndexFileRef)
228+
{
229+
var fileName = fileRef.Path;
230+
var fileWithoutExtension = fileName.Replace(".md", "");
231+
232+
// Validate: deep linking is NOT supported for folder+file combination
233+
// The file path should be simple (no '/'), or at most folder/file.md after prepending
234+
if (fileName.Contains('/'))
235+
{
236+
collector.EmitError(context,
237+
$"Deep linking on folder 'file' is not supported. Found file path '{fileName}' with '/'. Use simple file name only.");
238+
}
239+
240+
// Best practice: file name should match folder name (from parentPath)
241+
// Only check if we're in a folder context (parentPath is not empty)
242+
if (!string.IsNullOrEmpty(parentPath) && fileName != "index.md")
243+
{
244+
// Extract just the folder name from parentPath (in case it's nested like "guides/getting-started")
245+
var folderName = parentPath.Contains('/') ? parentPath.Split('/')[^1] : parentPath;
246+
247+
// Normalize for comparison: remove hyphens, underscores, and lowercase
248+
// This allows "getting-started" to match "GettingStarted" or "getting_started"
249+
var normalizedFile = fileWithoutExtension.Replace("-", "", StringComparison.Ordinal).Replace("_", "", StringComparison.Ordinal).ToLowerInvariant();
250+
var normalizedFolder = folderName.Replace("-", "", StringComparison.Ordinal).Replace("_", "", StringComparison.Ordinal).ToLowerInvariant();
251+
252+
if (!normalizedFile.Equals(normalizedFolder, StringComparison.Ordinal))
253+
{
254+
collector.EmitHint(context,
255+
$"File name '{fileName}' does not match folder name '{folderName}'. Best practice is to name the file the same as the folder (e.g., 'folder: {folderName}, file: {folderName}.md').");
256+
}
257+
}
258+
}
259+
225260
if (fileRef.Children.Count == 0)
226261
{
227-
return fileRef is IndexFileRef
228-
? new IndexFileRef(fullPath, fileRef.Hidden, [], context)
229-
: new FileRef(fullPath, fileRef.Hidden, [], context);
262+
// Preserve specific types even when there are no children
263+
return fileRef switch
264+
{
265+
FolderIndexFileRef => new FolderIndexFileRef(fullPath, fileRef.Hidden, [], context),
266+
IndexFileRef => new IndexFileRef(fullPath, fileRef.Hidden, [], context),
267+
_ => new FileRef(fullPath, fileRef.Hidden, [], context)
268+
};
230269
}
231270

232271
// Emit hint if file has children and uses deep-linking (path contains '/')
233272
// This suggests using 'folder' instead of 'file' would be better
234-
if (fileRef.Path.Contains('/') && fileRef.Children.Count > 0)
273+
if (fileRef.Path.Contains('/') && fileRef.Children.Count > 0 && fileRef is not FolderIndexFileRef)
235274
{
236275
collector.EmitHint(context,
237276
$"File '{fileRef.Path}' uses deep-linking with children. Consider using 'folder' instead of 'file' for better navigation structure. Virtual files are primarily intended to group sibling files together.");
@@ -543,18 +582,24 @@ public class TocItemYamlConverter : IYamlTypeConverter
543582
// Context will be set during LoadAndResolve, use empty string as placeholder during deserialization
544583
const string placeholderContext = "";
545584

546-
// Check for folder+file combination (e.g., folder: path, file: index.md)
547-
// This represents a folder with an index file - treat as FolderIndexFileRef with combined path
548-
// This special type ensures children resolve relative to the folder path
585+
// Check for folder+file combination (e.g., folder: getting-started, file: getting-started.md)
586+
// This represents a folder with a specific index file
587+
// The file becomes a child of the folder (as FolderIndexFileRef), and user-specified children follow
549588
if (dictionary.TryGetValue("folder", out var folderPath) && folderPath is string folder &&
550589
dictionary.TryGetValue("file", out var filePath) && filePath is string file)
551590
{
552-
// Combine folder and file paths
553-
var combinedPath = $"{folder}/{file}";
554-
// Use FolderIndexFileRef for index.md, regular FileRef for other files
555-
return file == "index.md"
556-
? new FolderIndexFileRef(combinedPath, false, children, placeholderContext)
557-
: new FileRef(combinedPath, false, children, placeholderContext);
591+
// Create the index file reference (FolderIndexFileRef to mark it as the folder's index)
592+
// Store ONLY the file name - the folder path will be prepended during resolution
593+
// This allows validation to check if the file itself has deep paths
594+
var indexFile = new FolderIndexFileRef(file, false, [], placeholderContext);
595+
596+
// Create a list with the index file first, followed by user-specified children
597+
var folderChildren = new List<ITableOfContentsItem> { indexFile };
598+
folderChildren.AddRange(children);
599+
600+
// Return a FolderRef with the index file and children
601+
// The folder path can be deep (e.g., "guides/getting-started"), that's OK
602+
return new FolderRef(folder, folderChildren, placeholderContext);
558603
}
559604

560605
// Check for file reference (file: or hidden:)

src/Elastic.Documentation.Navigation/README.md

Lines changed: 28 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,33 @@ toc:
4343
* `children` paths are scope to the folder.
4444
* Here we are including the files `blocks.md` and `index.md` in the `syntax` folder.
4545

46+
### Folders with a file
47+
48+
If you don't want to follow the `folder/index.md` pattern but instead want to have the index file one level up e.g
49+
50+
```
51+
getting-started.md
52+
getting-started/
53+
install.md
54+
```
55+
56+
You can do this by specifying the `file` property directly on the folder.
57+
58+
59+
```yaml
60+
toc:
61+
- folder: getting-started # /getting-started
62+
file: getting-started.md
63+
children:
64+
- file: install.md # /getting-started/install
65+
```
66+
67+
* `file` is the index file for the folder.
68+
* `children` paths are scope to the folder.
69+
* Here we are including the files `install.md` in the `getting-started` folder.
70+
* deep linking on the folder `file` is NOT supported
71+
* It's best practice to name the file like the folder. We emit a hint if this is not the case.
72+
4673
### Virtual Files
4774

4875
```yaml
@@ -79,7 +106,7 @@ toc:
79106
While supported, this is not recommended.
80107
* Favor `folder` over `file` when possible.
81108
* Navigation should follow the file structure as much as possible.
82-
* Virtual files are primarely intended to group sibling files together.
109+
* Virtual files are primarily intended to group sibling files together.
83110

84111
`docs-builder` will hint when these guidelines are not followed.
85112

Lines changed: 233 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,233 @@
1+
// Licensed to Elasticsearch B.V under one or more agreements.
2+
// Elasticsearch B.V licenses this file to you under the Apache 2.0 License.
3+
// See the LICENSE file in the project root for more information
4+
5+
using System.IO.Abstractions.TestingHelpers;
6+
using Elastic.Documentation.Configuration.DocSet;
7+
using Elastic.Documentation.Diagnostics;
8+
using Elastic.Documentation.Extensions;
9+
using Elastic.Documentation.Navigation.Isolated;
10+
using FluentAssertions;
11+
12+
namespace Elastic.Documentation.Navigation.Tests.Isolation;
13+
14+
public class FolderIndexFileRefTests(ITestOutputHelper output) : DocumentationSetNavigationTestBase(output)
15+
{
16+
[Fact]
17+
public async Task FolderWithFileCreatesCorrectStructure()
18+
{
19+
// language=yaml
20+
var yaml = """
21+
project: 'test-project'
22+
toc:
23+
- folder: getting-started
24+
file: getting-started.md
25+
children:
26+
- file: install.md
27+
- file: configure.md
28+
""";
29+
30+
var fileSystem = new MockFileSystem();
31+
fileSystem.AddDirectory("/docs");
32+
var context = CreateContext(fileSystem);
33+
var docSet = DocumentationSetFile.LoadAndResolve(context.Collector, yaml, fileSystem.NewDirInfo("docs"));
34+
_ = context.Collector.StartAsync(TestContext.Current.CancellationToken);
35+
36+
var navigation = new DocumentationSetNavigation<IDocumentationFile>(docSet, context, GenericDocumentationFileFactory.Instance);
37+
38+
await context.Collector.StopAsync(TestContext.Current.CancellationToken);
39+
40+
// Should create a FolderNavigation with the file as index
41+
navigation.NavigationItems.Should().HaveCount(1);
42+
var folder = navigation.NavigationItems.First().Should().BeOfType<FolderNavigation>().Subject;
43+
44+
// Children should be scoped to the folder
45+
folder.NavigationItems.Should().HaveCount(3); // getting-started.md, install.md, configure.md
46+
47+
// Verify no errors
48+
context.Collector.Errors.Should().Be(0);
49+
}
50+
51+
[Fact]
52+
public async Task FolderWithFileChildrenPathsAreScopedToFolder()
53+
{
54+
// language=yaml
55+
var yaml = """
56+
project: 'test-project'
57+
toc:
58+
- folder: getting-started
59+
file: getting-started.md
60+
children:
61+
- file: install.md
62+
""";
63+
64+
var fileSystem = new MockFileSystem();
65+
fileSystem.AddDirectory("/docs");
66+
var context = CreateContext(fileSystem);
67+
var docSet = DocumentationSetFile.LoadAndResolve(context.Collector, yaml, fileSystem.NewDirInfo("docs"));
68+
_ = context.Collector.StartAsync(TestContext.Current.CancellationToken);
69+
70+
_ = new DocumentationSetNavigation<IDocumentationFile>(docSet, context, GenericDocumentationFileFactory.Instance);
71+
72+
await context.Collector.StopAsync(TestContext.Current.CancellationToken);
73+
74+
// Verify that the FileRef for getting-started.md is a FolderIndexFileRef
75+
var folderItem = docSet.TableOfContents.First().Should().BeOfType<FolderRef>().Subject;
76+
folderItem.Children.Should().HaveCount(2); // getting-started.md and install.md
77+
78+
var indexFile = folderItem.Children.First().Should().BeOfType<FolderIndexFileRef>().Subject;
79+
indexFile.Path.Should().Be("getting-started/getting-started.md");
80+
81+
var childFile = folderItem.Children.ElementAt(1).Should().BeOfType<FileRef>().Subject;
82+
childFile.Path.Should().Be("getting-started/install.md");
83+
}
84+
85+
[Fact]
86+
public async Task FolderWithFileEmitsHintWhenFileNameDoesNotMatchFolder()
87+
{
88+
// language=yaml
89+
var yaml = """
90+
project: 'test-project'
91+
toc:
92+
- folder: getting-started
93+
file: intro.md
94+
children:
95+
- file: install.md
96+
""";
97+
98+
var fileSystem = new MockFileSystem();
99+
fileSystem.AddDirectory("/docs");
100+
var context = CreateContext(fileSystem);
101+
var docSet = DocumentationSetFile.LoadAndResolve(context.Collector, yaml, fileSystem.NewDirInfo("docs"));
102+
_ = context.Collector.StartAsync(TestContext.Current.CancellationToken);
103+
104+
_ = new DocumentationSetNavigation<IDocumentationFile>(docSet, context, GenericDocumentationFileFactory.Instance);
105+
106+
await context.Collector.StopAsync(TestContext.Current.CancellationToken);
107+
108+
// Should emit hint about file name not matching folder name
109+
context.Collector.Hints.Should().BeGreaterThan(0);
110+
var diagnostics = context.Diagnostics;
111+
diagnostics.Should().Contain(d =>
112+
d.Severity == Severity.Hint &&
113+
d.Message.Contains("intro.md") &&
114+
d.Message.Contains("getting-started") &&
115+
d.Message.Contains("Best practice"));
116+
}
117+
118+
[Fact]
119+
public async Task FolderWithFileDoesNotEmitHintWhenFileNameMatchesFolder()
120+
{
121+
// language=yaml
122+
var yaml = """
123+
project: 'test-project'
124+
toc:
125+
- folder: getting-started
126+
file: getting-started.md
127+
children:
128+
- file: install.md
129+
""";
130+
131+
var fileSystem = new MockFileSystem();
132+
fileSystem.AddDirectory("/docs");
133+
var context = CreateContext(fileSystem);
134+
var docSet = DocumentationSetFile.LoadAndResolve(context.Collector, yaml, fileSystem.NewDirInfo("docs"));
135+
_ = context.Collector.StartAsync(TestContext.Current.CancellationToken);
136+
137+
_ = new DocumentationSetNavigation<IDocumentationFile>(docSet, context, GenericDocumentationFileFactory.Instance);
138+
139+
await context.Collector.StopAsync(TestContext.Current.CancellationToken);
140+
141+
// Should not emit any hints
142+
context.Collector.Hints.Should().Be(0);
143+
context.Diagnostics.Should().BeEmpty();
144+
}
145+
146+
[Fact]
147+
public async Task FolderWithFileEmitsErrorForDeepLinkingInFile()
148+
{
149+
// language=yaml
150+
var yaml = """
151+
project: 'test-project'
152+
toc:
153+
- folder: getting-started
154+
file: intro/file.md
155+
children:
156+
- file: install.md
157+
""";
158+
159+
var fileSystem = new MockFileSystem();
160+
fileSystem.AddDirectory("/docs");
161+
var context = CreateContext(fileSystem);
162+
var docSet = DocumentationSetFile.LoadAndResolve(context.Collector, yaml, fileSystem.NewDirInfo("docs"));
163+
_ = context.Collector.StartAsync(TestContext.Current.CancellationToken);
164+
165+
_ = new DocumentationSetNavigation<IDocumentationFile>(docSet, context, GenericDocumentationFileFactory.Instance);
166+
167+
await context.Collector.StopAsync(TestContext.Current.CancellationToken);
168+
169+
// Should emit error about deep linking in the file attribute
170+
context.Collector.Errors.Should().BeGreaterThan(0);
171+
var diagnostics = context.Diagnostics;
172+
diagnostics.Should().Contain(d =>
173+
d.Severity == Severity.Error &&
174+
d.Message.Contains("Deep linking on folder 'file' is not supported") &&
175+
d.Message.Contains("intro/file.md"));
176+
}
177+
178+
[Fact]
179+
public async Task FolderWithIndexMdFileDoesNotNeedToMatchFolderName()
180+
{
181+
// language=yaml
182+
var yaml = """
183+
project: 'test-project'
184+
toc:
185+
- folder: getting-started
186+
file: index.md
187+
children:
188+
- file: install.md
189+
""";
190+
191+
var fileSystem = new MockFileSystem();
192+
fileSystem.AddDirectory("/docs");
193+
var context = CreateContext(fileSystem);
194+
var docSet = DocumentationSetFile.LoadAndResolve(context.Collector, yaml, fileSystem.NewDirInfo("docs"));
195+
_ = context.Collector.StartAsync(TestContext.Current.CancellationToken);
196+
197+
_ = new DocumentationSetNavigation<IDocumentationFile>(docSet, context, GenericDocumentationFileFactory.Instance);
198+
199+
await context.Collector.StopAsync(TestContext.Current.CancellationToken);
200+
201+
// index.md is a special case - should not emit hint
202+
// (Though the hint check doesn't exclude index.md, it's a reasonable best practice to allow it)
203+
context.Collector.Errors.Should().Be(0);
204+
}
205+
206+
[Fact]
207+
public async Task FolderWithFileCaseInsensitiveMatch()
208+
{
209+
// language=yaml
210+
var yaml = """
211+
project: 'test-project'
212+
toc:
213+
- folder: GettingStarted
214+
file: getting-started.md
215+
children:
216+
- file: install.md
217+
""";
218+
219+
var fileSystem = new MockFileSystem();
220+
fileSystem.AddDirectory("/docs");
221+
var context = CreateContext(fileSystem);
222+
var docSet = DocumentationSetFile.LoadAndResolve(context.Collector, yaml, fileSystem.NewDirInfo("docs"));
223+
_ = context.Collector.StartAsync(TestContext.Current.CancellationToken);
224+
225+
_ = new DocumentationSetNavigation<IDocumentationFile>(docSet, context, GenericDocumentationFileFactory.Instance);
226+
227+
await context.Collector.StopAsync(TestContext.Current.CancellationToken);
228+
229+
// Case-insensitive match should not emit hint
230+
context.Collector.Hints.Should().Be(0);
231+
context.Diagnostics.Should().BeEmpty();
232+
}
233+
}

0 commit comments

Comments
 (0)