Skip to content

Add table of contents scope management and refactor diagnostics #774

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.

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

Merged
merged 4 commits into from
Mar 19, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
5 changes: 4 additions & 1 deletion docs/development/index.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,4 +4,7 @@

# Development Guide

TODO write development documentation here
TODO write development documentation here


![Image outside of scope](../images/great-drawing-of-new-structure.png)

Check notice on line 10 in docs/development/index.md

View workflow job for this annotation

GitHub Actions / build

Image '../images/great-drawing-of-new-structure.png' is referenced out of table of contents scope '/home/runner/work/docs-builder/docs-builder/docs/development'.
Original file line number Diff line number Diff line change
Expand Up @@ -132,8 +132,7 @@ public static void EmitWarning(this IBlockExtension block, string message)
block.Build.Collector.Channel.Write(d);
}


public static void EmitError(this InlineProcessor processor, LinkInline inline, string message)
private static void LinkDiagnostic(InlineProcessor processor, Severity severity, LinkInline inline, string message)
{
var url = inline.Url;
var line = inline.Line + 1;
Expand All @@ -145,36 +144,22 @@ public static void EmitError(this InlineProcessor processor, LinkInline inline,
return;
var d = new Diagnostic
{
Severity = Severity.Error,
Severity = severity,
File = processor.GetContext().MarkdownSourcePath.FullName,
Column = column,
Column = Math.Max(column, 1),
Line = line,
Message = message,
Length = length
};
context.Build.Collector.Channel.Write(d);
}

public static void EmitError(this InlineProcessor processor, LinkInline inline, string message) =>
LinkDiagnostic(processor, Severity.Error, inline, message);

public static void EmitWarning(this InlineProcessor processor, LinkInline inline, string message)
{
var url = inline.Url;
var line = inline.Line + 1;
var column = inline.Column;
var length = url?.Length ?? 1;
public static void EmitWarning(this InlineProcessor processor, LinkInline inline, string message) =>
LinkDiagnostic(processor, Severity.Warning, inline, message);

var context = processor.GetContext();
if (context.SkipValidation)
return;
var d = new Diagnostic
{
Severity = Severity.Warning,
File = processor.GetContext().MarkdownSourcePath.FullName,
Column = column,
Line = line,
Message = message,
Length = length
};
context.Build.Collector.Channel.Write(d);
}
public static void EmitHint(this InlineProcessor processor, LinkInline inline, string message) =>
LinkDiagnostic(processor, Severity.Hint, inline, message);
}
Original file line number Diff line number Diff line change
Expand Up @@ -97,11 +97,11 @@ HashSet<string> files
if (f.Extension == ".toml")
{
var rule = DetectionRule.From(f);
return new RuleReference(relativePath, detectionRules, true, [], rule);
return new RuleReference(Build.Configuration, relativePath, detectionRules, true, [], rule);
}

_ = files.Add(relativePath);
return new FileReference(relativePath, true, false, []);
return new FileReference(Build.Configuration, relativePath, true, false, []);
})
.OrderBy(d => d is RuleReference r ? r.Rule.Name : null, StringComparer.OrdinalIgnoreCase)
.ToArray();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,13 @@

namespace Elastic.Markdown.Extensions.DetectionRules;

public record RulesFolderReference(string Path, bool Found, IReadOnlyCollection<ITocItem> Children) : ITocItem;
public record RulesFolderReference(ITableOfContentsScope TableOfContentsScope, string Path, bool Found, IReadOnlyCollection<ITocItem> Children) : ITocItem;

public record RuleReference(string Path, string SourceDirectory, bool Found, IReadOnlyCollection<ITocItem> Children, DetectionRule Rule)
: FileReference(Path, Found, false, Children);
public record RuleReference(
ITableOfContentsScope TableOfContentsScope,
string Path,
string SourceDirectory,
bool Found,
IReadOnlyCollection<ITocItem> Children, DetectionRule Rule
)
: FileReference(TableOfContentsScope, Path, Found, false, Children);
8 changes: 6 additions & 2 deletions src/Elastic.Markdown/IO/Configuration/ConfigurationFile.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
// Elasticsearch B.V licenses this file to you under the Apache 2.0 License.
// See the LICENSE file in the project root for more information

using System.IO.Abstractions;
using DotNet.Globbing;
using Elastic.Markdown.Diagnostics;
using Elastic.Markdown.Extensions;
Expand All @@ -10,7 +11,7 @@

namespace Elastic.Markdown.IO.Configuration;

public record ConfigurationFile : DocumentationFile
public record ConfigurationFile : DocumentationFile, ITableOfContentsScope
{
private readonly BuildContext _context;

Expand Down Expand Up @@ -44,6 +45,8 @@ public record ConfigurationFile : DocumentationFile
private FeatureFlags? _featureFlags;
public FeatureFlags Features => _featureFlags ??= new FeatureFlags(_features);

public IDirectoryInfo ScopeDirectory { get; }

/// This is a documentation set that is not linked to by assembler.
/// Setting this to true relaxes a few restrictions such as mixing toc references with file and folder reference
public bool DevelopmentDocs { get; }
Expand All @@ -57,6 +60,7 @@ public ConfigurationFile(BuildContext context)
: base(context.ConfigurationPath, context.DocumentationSourceDirectory)
{
_context = context;
ScopeDirectory = context.ConfigurationPath.Directory!;
if (!context.ConfigurationPath.Exists)
{
Project = "unknown";
Expand Down Expand Up @@ -122,7 +126,7 @@ public ConfigurationFile(BuildContext context)
switch (entry.Key)
{
case "toc":
var toc = new TableOfContentsConfiguration(this, _context, 0, "");
var toc = new TableOfContentsConfiguration(this, ScopeDirectory, _context, 0, "");
var children = toc.ReadChildren(reader, entry.Entry);
var tocEntries = children.OfType<TocReference>().ToArray();
if (!DevelopmentDocs && !IsNarrativeDocs && tocEntries.Length > 0 && children.Count != tocEntries.Length)
Expand Down
14 changes: 10 additions & 4 deletions src/Elastic.Markdown/IO/Configuration/ITocItem.cs
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,16 @@

namespace Elastic.Markdown.IO.Configuration;

public interface ITocItem;
public interface ITocItem
{
ITableOfContentsScope TableOfContentsScope { get; }
}

public record FileReference(string Path, bool Found, bool Hidden, IReadOnlyCollection<ITocItem> Children) : ITocItem;
public record FileReference(ITableOfContentsScope TableOfContentsScope, string Path, bool Found, bool Hidden, IReadOnlyCollection<ITocItem> Children)
: ITocItem;

public record FolderReference(string Path, bool Found, bool InNav, IReadOnlyCollection<ITocItem> Children) : ITocItem;
public record FolderReference(ITableOfContentsScope TableOfContentsScope, string Path, bool Found, bool InNav, IReadOnlyCollection<ITocItem> Children)
: ITocItem;

public record TocReference(string Path, bool Found, bool InNav, IReadOnlyCollection<ITocItem> Children) : FolderReference(Path, Found, InNav, Children);
public record TocReference(ITableOfContentsScope TableOfContentsScope, string Path, bool Found, bool InNav, IReadOnlyCollection<ITocItem> Children)
: FolderReference(TableOfContentsScope, Path, Found, InNav, Children);
Original file line number Diff line number Diff line change
Expand Up @@ -9,22 +9,32 @@

namespace Elastic.Markdown.IO.Configuration;

public record TableOfContentsConfiguration
public interface ITableOfContentsScope
{
IDirectoryInfo ScopeDirectory { get; }
}

public record TableOfContentsConfiguration : ITableOfContentsScope
{
private readonly ConfigurationFile _configuration;
private readonly BuildContext _context;
private readonly int _maxTocDepth;
private readonly int _depth;
private readonly string _parentPath;
private readonly IDirectoryInfo _rootPath;
private readonly ConfigurationFile _configuration;

public HashSet<string> Files { get; } = new(StringComparer.OrdinalIgnoreCase);

public IReadOnlyCollection<ITocItem> TableOfContents { get; private set; } = [];

public TableOfContentsConfiguration(ConfigurationFile configuration, BuildContext context, int depth, string parentPath)
public IDirectoryInfo ScopeDirectory { get; }

public TableOfContentsConfiguration(ConfigurationFile configuration, IDirectoryInfo scope, BuildContext context, int depth, string parentPath)
{
_rootPath = context.DocumentationSourceDirectory;
_configuration = configuration;
ScopeDirectory = scope;
_maxTocDepth = configuration.MaxTocDepth;
_rootPath = context.DocumentationSourceDirectory;
_context = context;
_depth = depth;
_parentPath = parentPath;
Expand All @@ -33,7 +43,7 @@ public TableOfContentsConfiguration(ConfigurationFile configuration, BuildContex
public IReadOnlyCollection<ITocItem> ReadChildren(YamlStreamReader reader, KeyValuePair<YamlNode, YamlNode> entry, string? parentPath = null)
{
parentPath ??= _parentPath;
if (_depth > _configuration.MaxTocDepth)
if (_depth > _maxTocDepth)
{
reader.EmitError($"toc.yml files may only be linked from docset.yml", entry.Key);
return [];
Expand Down Expand Up @@ -112,7 +122,7 @@ public IReadOnlyCollection<ITocItem> ReadChildren(YamlStreamReader reader, KeyVa
foreach (var f in toc.Files)
_ = Files.Add(f);

return [new TocReference($"{parentPath}".TrimStart(Path.DirectorySeparatorChar), folderFound, inNav, toc.TableOfContents)];
return [new TocReference(this, $"{parentPath}".TrimStart(Path.DirectorySeparatorChar), folderFound, inNav, toc.TableOfContents)];
}

if (file is not null)
Expand All @@ -133,15 +143,15 @@ public IReadOnlyCollection<ITocItem> ReadChildren(YamlStreamReader reader, KeyVa
children = extension.CreateTableOfContentItems(parentPath, detectionRules, Files);
}
}
return [new FileReference($"{parentPath}{Path.DirectorySeparatorChar}{file}".TrimStart(Path.DirectorySeparatorChar), fileFound, hiddenFile, children ?? [])];
return [new FileReference(this, $"{parentPath}{Path.DirectorySeparatorChar}{file}".TrimStart(Path.DirectorySeparatorChar), fileFound, hiddenFile, children ?? [])];
}

if (folder is not null)
{
if (children is null)
_ = _configuration.ImplicitFolders.Add(parentPath.TrimStart(Path.DirectorySeparatorChar));

return [new FolderReference($"{parentPath}".TrimStart(Path.DirectorySeparatorChar), folderFound, inNav, children ?? [])];
return [new FolderReference(this, $"{parentPath}".TrimStart(Path.DirectorySeparatorChar), folderFound, inNav, children ?? [])];
}

return null;
Expand Down Expand Up @@ -235,7 +245,7 @@ public IReadOnlyCollection<ITocItem> ReadChildren(YamlStreamReader reader, KeyVa
switch (kv.Key)
{
case "toc":
var nestedConfiguration = new TableOfContentsConfiguration(_configuration, _context, _depth + 1, fullTocPath);
var nestedConfiguration = new TableOfContentsConfiguration(_configuration, source.Directory!, _context, _depth + 1, fullTocPath);
_ = nestedConfiguration.ReadChildren(reader, kv.Entry);
return nestedConfiguration;
}
Expand Down
14 changes: 10 additions & 4 deletions src/Elastic.Markdown/IO/MarkdownFile.cs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
using System.Runtime.InteropServices;
using Elastic.Markdown.Diagnostics;
using Elastic.Markdown.Helpers;
using Elastic.Markdown.IO.Configuration;
using Elastic.Markdown.IO.Navigation;
using Elastic.Markdown.Myst;
using Elastic.Markdown.Myst.Directives;
Expand Down Expand Up @@ -45,6 +46,9 @@ DocumentationSet set
_configurationFile = build.Configuration.SourceFile;
_globalSubstitutions = build.Configuration.Substitutions;
_set = set;
//may be updated by DocumentationGroup.ProcessTocItems
//todo refactor mutability of MarkdownFile as a whole
ScopeDirectory = build.Configuration.ScopeDirectory;
}

public string Id { get; } = Guid.NewGuid().ToString("N")[..8];
Expand Down Expand Up @@ -80,8 +84,8 @@ public string? NavigationTitle
}

//indexed by slug
private readonly Dictionary<string, PageTocItem> _tableOfContent = new(StringComparer.OrdinalIgnoreCase);
public IReadOnlyDictionary<string, PageTocItem> TableOfContents => _tableOfContent;
private readonly Dictionary<string, PageTocItem> _pageTableOfContent = new(StringComparer.OrdinalIgnoreCase);
public IReadOnlyDictionary<string, PageTocItem> PageTableOfContent => _pageTableOfContent;

private readonly HashSet<string> _anchors = new(StringComparer.OrdinalIgnoreCase);
public IReadOnlySet<string> Anchors => _anchors;
Expand Down Expand Up @@ -146,6 +150,8 @@ public string[] YieldParentGroups()
/// because we need to minimally parse to see the anchors anchor validation is deferred.
public IReadOnlyDictionary<string, string?>? AnchorRemapping { get; set; }

public IDirectoryInfo ScopeDirectory { get; set; }

private void ValidateAnchorRemapping()
{
if (AnchorRemapping is null)
Expand Down Expand Up @@ -224,9 +230,9 @@ protected void ReadDocumentInstructions(MarkdownDocument document)

var toc = GetAnchors(_set, MarkdownParser, YamlFrontMatter, document, subs, out var anchors);

_tableOfContent.Clear();
_pageTableOfContent.Clear();
Copy link
Member

Choose a reason for hiding this comment

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

While reading some navigation UX patterns I encountered the the term "On Page Table of Contents", which I think is very clear.

Copy link
Member

Choose a reason for hiding this comment

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

I think it removes a lot of ambiguity, since in the HTML it also says "On this page"

foreach (var t in toc)
_tableOfContent[t.Slug] = t;
_pageTableOfContent[t.Slug] = t;


foreach (var label in anchors)
Expand Down
10 changes: 6 additions & 4 deletions src/Elastic.Markdown/IO/Navigation/DocumentationGroup.cs
Original file line number Diff line number Diff line change
Expand Up @@ -112,13 +112,15 @@ internal DocumentationGroup(
continue;


foreach (var extension in context.Configuration.EnabledExtensions)
extension.Visit(d, tocItem);

md.Parent = this;
md.Hidden = file.Hidden;
var navigationIndex = Interlocked.Increment(ref fileIndex);
md.NavigationIndex = navigationIndex;
md.ScopeDirectory = file.TableOfContentsScope.ScopeDirectory;

foreach (var extension in context.Configuration.EnabledExtensions)
extension.Visit(d, tocItem);


if (file.Children.Count > 0 && d is MarkdownFile virtualIndex)
{
Expand Down Expand Up @@ -153,7 +155,7 @@ internal DocumentationGroup(
children =
[
.. documentationFiles
.Select(d => new FileReference(d.RelativePath, true, false, []))
.Select(d => new FileReference(folder.TableOfContentsScope, d.RelativePath, true, false, []))
];
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,6 @@ public class DiagnosticLinkInlineParser : LinkInlineParser
public override bool Match(InlineProcessor processor, ref StringSlice slice)
{
var match = base.Match(processor, ref slice);

if (!match || processor.Inline is not LinkInline link)
return match;

Expand All @@ -72,6 +71,9 @@ public override bool Match(InlineProcessor processor, ref StringSlice slice)

private static void ParseStylingInstructions(LinkInline link)
{
if (!link.IsImage)
return;

if (string.IsNullOrWhiteSpace(link.Title) || link.Title.IndexOf('=') < 0)
return;

Expand Down Expand Up @@ -189,7 +191,16 @@ private static void ProcessInternalLink(LinkInline link, InlineProcessor process
var includeFrom = GetIncludeFromPath(url, context);
var file = ResolveFile(context, url);
ValidateInternalUrl(processor, url, includeFrom, link, context);
ProcessLinkText(processor, link, context, url, anchor, file);

if (link.IsImage && context.DocumentationFileLookup(context.MarkdownSourcePath) is MarkdownFile currentMarkdown)
{
//TODO make this an error once all offending repositories have been updated
if (!file.Directory!.FullName.StartsWith(currentMarkdown.ScopeDirectory.FullName + Path.DirectorySeparatorChar))
processor.EmitHint(link, $"Image '{url}' is referenced out of table of contents scope '{currentMarkdown.ScopeDirectory}'.");
}

var linkMarkdown = context.DocumentationFileLookup(file) as MarkdownFile;
ProcessLinkText(processor, link, linkMarkdown, anchor, url, file);
UpdateLinkUrl(link, url, context, anchor, file);
}

Expand All @@ -214,14 +225,12 @@ private static void ValidateInternalUrl(InlineProcessor processor, string url, s
processor.EmitError(link, $"`{url}` does not exist. resolved to `{pathOnDisk}");
}

private static void ProcessLinkText(InlineProcessor processor, LinkInline link, ParserContext context, string url, string? anchor, IFileInfo file)
private static void ProcessLinkText(InlineProcessor processor, LinkInline link, MarkdownFile? markdown, string? anchor, string url, IFileInfo file)
{
if (link.FirstChild != null && string.IsNullOrEmpty(anchor))
return;

var markdown = context.DocumentationFileLookup(file) as MarkdownFile;

if (markdown == null && link.FirstChild == null)
if (markdown is null && link.FirstChild == null)
{
processor.EmitWarning(link,
$"'{url}' could not be resolved to a markdown file while creating an auto text link, '{file.FullName}' does not exist.");
Expand All @@ -234,7 +243,7 @@ private static void ProcessLinkText(InlineProcessor processor, LinkInline link,
{
if (markdown is not null)
ValidateAnchor(processor, markdown, anchor, link);
if (link.FirstChild == null && (markdown?.TableOfContents.TryGetValue(anchor, out var heading) ?? false))
if (link.FirstChild == null && (markdown?.PageTableOfContent.TryGetValue(anchor, out var heading) ?? false))
title += " > " + heading.Heading;
}

Expand Down
2 changes: 1 addition & 1 deletion src/Elastic.Markdown/Slices/HtmlWriter.cs
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ public async Task<string> RenderLayout(MarkdownFile markdown, MarkdownDocument d
Title = markdown.Title ?? "[TITLE NOT SET]",
TitleRaw = markdown.TitleRaw ?? "[TITLE NOT SET]",
MarkdownHtml = html,
PageTocItems = [.. markdown.TableOfContents.Values],
PageTocItems = [.. markdown.PageTableOfContent.Values],
Tree = DocumentationSet.Tree,
CurrentDocument = markdown,
PreviousDocument = previous,
Expand Down
Loading
Loading