From 673c7b5b5d9735352581a3d0c00b21d0821bab90 Mon Sep 17 00:00:00 2001 From: Martijn Laarman Date: Mon, 27 Jan 2025 21:43:49 +0100 Subject: [PATCH 1/6] Add anchors to link json artifact Refactored `LinkReference` to use a dictionary for better anchor tracking and adjusted anchor validations. Added extensive test coverage, revamped test setup, and improved generator logic for markdown files.``` --- .../DocumentationGenerator.cs | 47 ++++++----- .../IO/Discovery/GitCheckoutInformation.cs | 7 +- src/Elastic.Markdown/IO/MarkdownFile.cs | 16 ++-- .../IO/State/LinkReference.cs | 6 +- .../DiagnosticLinkInlineParser.cs | 6 +- .../DocSet/LinkReferenceTests.cs | 2 +- tests/authoring/Container/DefinitionLists.fs | 4 +- .../Framework/ErrorCollectorAssertions.fs | 6 +- tests/authoring/Framework/HtmlAssertions.fs | 35 +++++---- .../Framework/MarkdownResultsAssertions.fs | 77 +++++++++++++++++++ tests/authoring/Framework/Setup.fs | 70 +++++++++++++---- tests/authoring/Framework/TestValues.fs | 65 ++++++++++++---- .../authoring/Generator/LinkReferenceFile.fs | 71 +++++++++++++++++ tests/authoring/authoring.fsproj | 2 + 14 files changed, 326 insertions(+), 88 deletions(-) create mode 100644 tests/authoring/Framework/MarkdownResultsAssertions.fs create mode 100644 tests/authoring/Generator/LinkReferenceFile.fs diff --git a/src/Elastic.Markdown/DocumentationGenerator.cs b/src/Elastic.Markdown/DocumentationGenerator.cs index bb6f71a2..517f847e 100644 --- a/src/Elastic.Markdown/DocumentationGenerator.cs +++ b/src/Elastic.Markdown/DocumentationGenerator.cs @@ -50,8 +50,12 @@ ILoggerFactory logger } - public async Task ResolveDirectoryTree(Cancel ctx) => + public async Task ResolveDirectoryTree(Cancel ctx) + { + _logger.LogInformation("Resolving tree"); await DocumentationSet.Tree.Resolve(ctx); + _logger.LogInformation("Resolved tree"); + } public async Task GenerateAll(Cancel ctx) { @@ -62,11 +66,30 @@ public async Task GenerateAll(Cancel ctx) if (CompilationNotNeeded(generationState, out var offendingFiles, out var outputSeenChanges)) return; - _logger.LogInformation("Resolving tree"); await ResolveDirectoryTree(ctx); - _logger.LogInformation("Resolved tree"); + await ProcessDocumentationFiles(offendingFiles, outputSeenChanges, ctx); + + await ExtractEmbeddedStaticResources(ctx); + + + _logger.LogInformation($"Completing diagnostics channel"); + Context.Collector.Channel.TryComplete(); + _logger.LogInformation($"Generating documentation compilation state"); + await GenerateDocumentationState(ctx); + _logger.LogInformation($"Generating links.json"); + await GenerateLinkReference(ctx); + + _logger.LogInformation($"Completing diagnostics channel"); + + await Context.Collector.StopAsync(ctx); + + _logger.LogInformation($"Completed diagnostics channel"); + } + + private async Task ProcessDocumentationFiles(HashSet offendingFiles, DateTimeOffset outputSeenChanges, Cancel ctx) + { var processedFileCount = 0; var exceptionCount = 0; _ = Context.Collector.StartAsync(ctx); @@ -91,7 +114,10 @@ await Parallel.ForEachAsync(DocumentationSet.Files, ctx, async (file, token) => if (processedFiles % 100 == 0) _logger.LogInformation($"-> Handled {processedFiles} files"); }); + } + private async Task ExtractEmbeddedStaticResources(Cancel ctx) + { _logger.LogInformation($"Copying static files to output directory"); var embeddedStaticFiles = Assembly.GetExecutingAssembly() .GetManifestResourceNames() @@ -111,21 +137,6 @@ await Parallel.ForEachAsync(DocumentationSet.Files, ctx, async (file, token) => await resourceStream.CopyToAsync(stream, ctx); _logger.LogInformation($"Copied static embedded resource {path}"); } - - - _logger.LogInformation($"Completing diagnostics channel"); - Context.Collector.Channel.TryComplete(); - - _logger.LogInformation($"Generating documentation compilation state"); - await GenerateDocumentationState(ctx); - _logger.LogInformation($"Generating links.json"); - await GenerateLinkReference(ctx); - - _logger.LogInformation($"Completing diagnostics channel"); - - await Context.Collector.StopAsync(ctx); - - _logger.LogInformation($"Completed diagnostics channel"); } private async Task ProcessFile(HashSet offendingFiles, DocumentationFile file, DateTimeOffset outputSeenChanges, CancellationToken token) diff --git a/src/Elastic.Markdown/IO/Discovery/GitCheckoutInformation.cs b/src/Elastic.Markdown/IO/Discovery/GitCheckoutInformation.cs index bfed03b3..5a17e4ff 100644 --- a/src/Elastic.Markdown/IO/Discovery/GitCheckoutInformation.cs +++ b/src/Elastic.Markdown/IO/Discovery/GitCheckoutInformation.cs @@ -39,19 +39,18 @@ public string? RepositoryName // manual read because libgit2sharp is not yet AOT ready public static GitCheckoutInformation Create(IFileSystem fileSystem) { - // filesystem is not real so return a dummy - var fakeRef = Guid.NewGuid().ToString().Substring(0, 16); if (fileSystem is not FileSystem) { return new GitCheckoutInformation { - Branch = $"test-{fakeRef}", + Branch = $"test-e35fcb27-5f60-4e", Remote = "elastic/docs-builder", - Ref = fakeRef, + Ref = "e35fcb27-5f60-4e", RepositoryName = "docs-builder" }; } + var fakeRef = Guid.NewGuid().ToString()[..16]; var gitConfig = Git(".git/config"); if (!gitConfig.Exists) return Unavailable; diff --git a/src/Elastic.Markdown/IO/MarkdownFile.cs b/src/Elastic.Markdown/IO/MarkdownFile.cs index c1089815..e72b91e6 100644 --- a/src/Elastic.Markdown/IO/MarkdownFile.cs +++ b/src/Elastic.Markdown/IO/MarkdownFile.cs @@ -63,8 +63,8 @@ public string? NavigationTitle private readonly Dictionary _tableOfContent = new(StringComparer.OrdinalIgnoreCase); public IReadOnlyDictionary TableOfContents => _tableOfContent; - private readonly HashSet _additionalLabels = new(StringComparer.OrdinalIgnoreCase); - public IReadOnlySet AdditionalLabels => _additionalLabels; + private readonly HashSet _anchors = new(StringComparer.OrdinalIgnoreCase); + public IReadOnlySet Anchors => _anchors; public string FilePath { get; } public string FileName { get; } @@ -171,22 +171,22 @@ private void ReadDocumentInstructions(MarkdownDocument document) Slug = (h.Item2 ?? h.Item1).Slugify() }) .ToList(); + _tableOfContent.Clear(); foreach (var t in contents) _tableOfContent[t.Slug] = t; - var labels = document.Descendants() + var anchors = document.Descendants() .Select(b => b.CrossReferenceName) .Where(l => !string.IsNullOrWhiteSpace(l)) .Select(s => s.Slugify()) .Concat(document.Descendants().Select(a => a.Anchor)) + .Concat(_tableOfContent.Values.Select(t => t.Slug)) + .Where(anchor => !string.IsNullOrEmpty(anchor)) .ToArray(); - foreach (var label in labels) - { - if (!string.IsNullOrEmpty(label)) - _additionalLabels.Add(label); - } + foreach (var label in anchors) + _anchors.Add(label); _instructionsParsed = true; } diff --git a/src/Elastic.Markdown/IO/State/LinkReference.cs b/src/Elastic.Markdown/IO/State/LinkReference.cs index 83d19908..301e9cbf 100644 --- a/src/Elastic.Markdown/IO/State/LinkReference.cs +++ b/src/Elastic.Markdown/IO/State/LinkReference.cs @@ -15,8 +15,9 @@ public record LinkReference [JsonPropertyName("url_path_prefix")] public required string? UrlPathPrefix { get; init; } + /// Mapping of relative filepath and all the page's anchors for deeplinks [JsonPropertyName("links")] - public required string[] Links { get; init; } = []; + public required Dictionary Links { get; init; } = []; [JsonPropertyName("cross_links")] public required string[] CrossLinks { get; init; } = []; @@ -25,7 +26,8 @@ public static LinkReference Create(DocumentationSet set) { var crossLinks = set.Context.Collector.CrossLinks.ToHashSet().ToArray(); var links = set.MarkdownFiles.Values - .Select(m => m.RelativePath).ToArray(); + .Select(m => (m.RelativePath, m.Anchors)) + .ToDictionary(k => k.RelativePath, v => v.Anchors.ToArray()); return new LinkReference { UrlPathPrefix = set.Context.UrlPathPrefix, diff --git a/src/Elastic.Markdown/Myst/InlineParsers/DiagnosticLinkInlineParser.cs b/src/Elastic.Markdown/Myst/InlineParsers/DiagnosticLinkInlineParser.cs index b6d8472a..b69f82ee 100644 --- a/src/Elastic.Markdown/Myst/InlineParsers/DiagnosticLinkInlineParser.cs +++ b/src/Elastic.Markdown/Myst/InlineParsers/DiagnosticLinkInlineParser.cs @@ -135,11 +135,9 @@ public override bool Match(InlineProcessor processor, ref StringSlice slice) if (!string.IsNullOrEmpty(anchor)) { - if (markdown == null || (!markdown.TableOfContents.TryGetValue(anchor, out var heading) - && !markdown.AdditionalLabels.Contains(anchor))) + if (markdown == null || !markdown.Anchors.Contains(anchor)) processor.EmitError(line, column, length, $"`{anchor}` does not exist in {markdown?.FileName}."); - - else if (link.FirstChild == null && heading != null) + else if (link.FirstChild == null && markdown.TableOfContents.TryGetValue(anchor, out var heading)) title += " > " + heading.Heading; } diff --git a/tests/Elastic.Markdown.Tests/DocSet/LinkReferenceTests.cs b/tests/Elastic.Markdown.Tests/DocSet/LinkReferenceTests.cs index b40f3b8d..16329df9 100644 --- a/tests/Elastic.Markdown.Tests/DocSet/LinkReferenceTests.cs +++ b/tests/Elastic.Markdown.Tests/DocSet/LinkReferenceTests.cs @@ -26,7 +26,7 @@ public void EmitsLinks() => [Fact] public void ShouldNotIncludeSnippets() => - Reference.Links.Should().NotContain(l => l.Contains("_snippets/")); + Reference.Links.Should().NotContain(l => l.Key.Contains("_snippets/")); } public class GitCheckoutInformationTests(ITestOutputHelper output) : NavigationTestsBase(output) diff --git a/tests/authoring/Container/DefinitionLists.fs b/tests/authoring/Container/DefinitionLists.fs index be25a5bb..b593fd83 100644 --- a/tests/authoring/Container/DefinitionLists.fs +++ b/tests/authoring/Container/DefinitionLists.fs @@ -41,7 +41,7 @@ This is my `definition` """ [] - let ``validate HTML`` () = + let ``validate HTML 2`` () = markdown |> convertsToHtml """
This is my definition
@@ -56,4 +56,4 @@ This is my `definition`
""" [] - let ``has no errors`` () = markdown |> hasNoErrors + let ``has no errors 2`` () = markdown |> hasNoErrors diff --git a/tests/authoring/Framework/ErrorCollectorAssertions.fs b/tests/authoring/Framework/ErrorCollectorAssertions.fs index 3c530fa6..9ceff469 100644 --- a/tests/authoring/Framework/ErrorCollectorAssertions.fs +++ b/tests/authoring/Framework/ErrorCollectorAssertions.fs @@ -14,11 +14,13 @@ open Swensen.Unquote module DiagnosticsCollectorAssertions = [] - let hasNoErrors (actual: GenerateResult) = + let hasNoErrors (actual: Lazy) = + let actual = actual.Value test <@ actual.Context.Collector.Errors = 0 @> [] - let hasError (expected: string) (actual: GenerateResult) = + let hasError (expected: string) (actual: Lazy) = + let actual = actual.Value actual.Context.Collector.Errors |> shouldBeGreaterThan 0 let errorDiagnostics = actual.Context.Collector.Diagnostics .Where(fun d -> d.Severity = Severity.Error) diff --git a/tests/authoring/Framework/HtmlAssertions.fs b/tests/authoring/Framework/HtmlAssertions.fs index 341bea2d..dc55543a 100644 --- a/tests/authoring/Framework/HtmlAssertions.fs +++ b/tests/authoring/Framework/HtmlAssertions.fs @@ -89,35 +89,36 @@ actual: {actual} |> Seq.iter _.ToHtml(sw, PrettyMarkupFormatter()) sw.ToString() - [] - let convertsToHtml ([]expected: string) (actual: GenerateResult) = + let private createDiff expected actual = let diffs = DiffBuilder - .Compare(actual.Html) + .Compare(actual) .WithTest(expected) .Build() - let diff = htmlDiffString diffs - match diff with + let deepComparision = htmlDiffString diffs + match deepComparision with | s when String.IsNullOrEmpty s -> () | s -> let expectedHtml = prettyHtml expected - let actualHtml = prettyHtml actual.Html - let textDiff = - InlineDiffBuilder.Diff(expectedHtml, actualHtml).Lines - |> Seq.map(fun l -> - match l.Type with - | ChangeType.Deleted -> "- " + l.Text - | ChangeType.Modified -> "+ " + l.Text - | ChangeType.Inserted -> "+ " + l.Text - | _ -> " " + l.Text - ) - |> String.concat "\n" + let actualHtml = prettyHtml actual + let textDiff = diff expectedHtml actualHtml let msg = $"""Html was not equal {textDiff} -{diff} +{deepComparision} """ raise (XunitException(msg)) + [] + let convertsToHtml ([]expected: string) (actual: Lazy) = + let actual = actual.Value + + let defaultFile = actual.MarkdownResults |> Seq.head + createDiff expected defaultFile.Html + + [] + let toHtml ([]expected: string) (actual: MarkdownResult) = + createDiff expected actual.Html + diff --git a/tests/authoring/Framework/MarkdownResultsAssertions.fs b/tests/authoring/Framework/MarkdownResultsAssertions.fs new file mode 100644 index 00000000..46b2a532 --- /dev/null +++ b/tests/authoring/Framework/MarkdownResultsAssertions.fs @@ -0,0 +1,77 @@ +namespace authoring + +open System.Diagnostics +open System.Text.Json +open DiffPlex.DiffBuilder +open DiffPlex.DiffBuilder.Model +open FsUnit.Xunit +open JetBrains.Annotations +open Xunit.Sdk + +[] +module ResultsAssertions = + + let diff expected actual = + let diffLines = InlineDiffBuilder.Diff(expected, actual).Lines + + let mutatedCount = + diffLines + |> Seq.filter (fun l -> + match l.Type with + | ChangeType.Modified -> true + | ChangeType.Inserted -> true + | _ -> false + ) + |> Seq.length + + let actualLineLength = actual.Split("\n").Length + match mutatedCount with + | 0 -> "" + | _ when mutatedCount >= actualLineLength -> $"Mutations {mutatedCount} on all {actualLineLength} showing actual: \n\n{actual}" + | _ -> + diffLines + |> Seq.map(fun l -> + match l.Type with + | ChangeType.Deleted -> "- " + l.Text + | ChangeType.Modified -> "+ " + l.Text + | ChangeType.Inserted -> "+ " + l.Text + | _ -> " " + l.Text + ) + |> String.concat "\n" + + + [] + let converts file (results: Lazy) = + let results = results.Value + + let result = + results.MarkdownResults + |> Seq.tryFind (fun m -> m.File.RelativePath = file) + + match result with + | None -> + raise (XunitException($"{file} not part of the markdown results")) + | Some result -> result + +[] +module JsonAssertions = + + [] + let convertsToJson artifact ([]expected: string) (actual: Lazy) = + let actual = actual.Value + let fs = actual.Context.ReadFileSystem + + let fi = fs.FileInfo.New(artifact) + if not <| fi.Exists then + raise (XunitException($"{artifact} is not part of the output")) + + let actual = fs.File.ReadAllText(fi.FullName) + use actualJson = JsonDocument.Parse(actual); + let actual = JsonSerializer.Serialize(actualJson, JsonSerializerOptions(WriteIndented = true)) + + use expectedJson = JsonDocument.Parse(expected); + let expected = JsonSerializer.Serialize(expectedJson, JsonSerializerOptions(WriteIndented = true)) + + diff expected actual |> should be NullOrEmptyString + + diff --git a/tests/authoring/Framework/Setup.fs b/tests/authoring/Framework/Setup.fs index 0debb0b2..6df08b7f 100644 --- a/tests/authoring/Framework/Setup.fs +++ b/tests/authoring/Framework/Setup.fs @@ -4,6 +4,7 @@ namespace authoring + open System.Collections.Generic open System.IO open System.IO.Abstractions.TestingHelpers @@ -11,6 +12,23 @@ open System.Threading.Tasks open Elastic.Markdown open Elastic.Markdown.IO open JetBrains.Annotations +open Xunit + +[] +do() + +type Markdown = string + +[] +type TestFile = + | File of name: string * contents: string + | MarkdownFile of name: string * markdown: Markdown + + static member IndexFile([] m) = + MarkdownFile("index.md" , m) + + static member Markdown([] m) = + MarkdownFile("index.md" , m) type Setup = @@ -37,9 +55,16 @@ type Setup = fileSystem.AddFile(Path.Combine(root.FullName, "docset.yml"), MockFileData(yaml.ToString())); - static let Generate ([]m: string) : Task = + static member Generator (files: TestFile seq) : Task = + + let d = files + |> Seq.map (fun f -> + match f with + | File(name, contents) -> ($"docs/{name}", MockFileData(contents)) + | MarkdownFile(name, markdown) -> ($"docs/{name}", MockFileData(markdown)) + ) + |> Map.ofSeq - let d = dict [ ("docs/index.md", MockFileData(m)) ] let opts = MockFileSystemOptions(CurrentDirectory=Paths.Root.FullName) let fileSystem = MockFileSystem(d, opts) @@ -48,34 +73,49 @@ type Setup = let collector = TestDiagnosticsCollector(); let context = BuildContext(fileSystem, Collector=collector) let set = DocumentationSet(context); - let file = - match set.GetMarkdownFile(fileSystem.FileInfo.New("docs/index.md")) with - | NonNull f -> f - | _ -> failwithf "docs/index.md could not be located" + let generator = DocumentationGenerator(set, new TestLoggerFactory()) + + let markdownFiles = + files + |> Seq.map (fun f -> + match f with + | File _ -> None + | MarkdownFile(name, _) -> Some $"docs/{name}" + ) + |> Seq.choose id + |> Seq.map (fun f -> + match set.GetMarkdownFile(fileSystem.FileInfo.New(f)) with + | NonNull m -> Some m + | _ -> None + ) + |> Seq.choose id let context = { - File = file + MarkdownFiles = markdownFiles Collector = collector Set = set + Generator = generator ReadFileSystem = fileSystem WriteFileSystem = fileSystem } context.Bootstrap() + /// Pass several files to the test setup + static member Generate files = + lazy (task { return! Setup.Generator files } |> Async.AwaitTask |> Async.RunSynchronously) + /// Pass a full documentation page to the test setup static member Document ([]m: string) = - let g = task { return! Generate m } - g |> Async.AwaitTask |> Async.RunSynchronously + lazy (task { return! Setup.Generator [IndexFile m] } |> Async.AwaitTask |> Async.RunSynchronously) /// Pass a markdown fragment to the test setup static member Markdown ([]m: string) = // language=markdown - let m = $""" -# Test Document + let m = $"""# Test Document {m} """ - let g = task { - return! Generate m - } - g |> Async.AwaitTask |> Async.RunSynchronously + lazy ( + task { return! Setup.Generator [IndexFile m] } + |> Async.AwaitTask |> Async.RunSynchronously + ) diff --git a/tests/authoring/Framework/TestValues.fs b/tests/authoring/Framework/TestValues.fs index 3c77f2c6..1023574f 100644 --- a/tests/authoring/Framework/TestValues.fs +++ b/tests/authoring/Framework/TestValues.fs @@ -6,9 +6,14 @@ namespace authoring open System open System.IO.Abstractions +open System.Threading +open System.Threading.Tasks +open Elastic.Markdown open Elastic.Markdown.Diagnostics open Elastic.Markdown.IO open Markdig.Syntax +open Microsoft.Extensions.Logging +open Microsoft.FSharp.Core open Xunit @@ -34,37 +39,67 @@ type TestDiagnosticsCollector() = member _.Diagnostics = diagnostics.AsReadOnly() - override this.HandleItem diagnostic = diagnostics.Add(diagnostic); + override this.HandleItem diagnostic = diagnostics.Add(diagnostic) -type GenerateResult = { +type TestLogger () = + + interface ILogger with + member this.IsEnabled(logger) = true + member this.BeginScope(scope) = null + member this.Log(logLevel, eventId, state, ex, formatter) = + match TestContext.Current.TestOutputHelper with + | NonNull logger -> + let formatted = formatter.Invoke(state, ex) + logger.WriteLine formatted + | _ -> () + +type TestLoggerFactory () = + + interface ILoggerFactory with + member this.AddProvider(provider) = () + member this.CreateLogger(categoryName) = TestLogger() + member this.Dispose() = () + + +type MarkdownResult = { + File: MarkdownFile Document: MarkdownDocument Html: string Context: MarkdownTestContext } +and GeneratorResults = { + Context: MarkdownTestContext + MarkdownResults: MarkdownResult seq +} and MarkdownTestContext = { - File: MarkdownFile + MarkdownFiles: MarkdownFile seq Collector: TestDiagnosticsCollector Set: DocumentationSet + Generator: DocumentationGenerator ReadFileSystem: IFileSystem WriteFileSystem: IFileSystem } member this.Bootstrap () = backgroundTask { let! ctx = Async.CancellationToken - let _ = this.Collector.StartAsync(ctx) - do! this.Set.ResolveDirectoryTree(ctx) - - let! document = this.File.ParseFullAsync(ctx) - - let html = this.File.CreateHtml(document); - this.Collector.Channel.TryComplete() - do! this.Collector.StopAsync(ctx) - return { Context = this; Document = document; Html = html } + do! this.Generator.GenerateAll(ctx) + + let results = + this.MarkdownFiles + |> Seq.map (fun (f: MarkdownFile) -> task { + // technically we do this work twice since generate all also does it + let! document = f.ParseFullAsync(ctx) + let html = f.CreateHtml(document) + return { File = f; Document = document; Html = html; Context = this } + }) + // this is not great code, refactor or depend on FSharp.Control.TaskSeq + // for now this runs without issue + |> Seq.map (fun t -> t |> Async.AwaitTask |> Async.RunSynchronously) + + return { Context = this; MarkdownResults = results } } interface IDisposable with - member this.Dispose() = () - - + member this.Dispose() = () \ No newline at end of file diff --git a/tests/authoring/Generator/LinkReferenceFile.fs b/tests/authoring/Generator/LinkReferenceFile.fs new file mode 100644 index 00000000..7ca43ec6 --- /dev/null +++ b/tests/authoring/Generator/LinkReferenceFile.fs @@ -0,0 +1,71 @@ +// Licensed to Elasticsearch B.V under one or more agreements. +// 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 + +module ``documentation generator``.``links json artifact`` + +open Xunit +open authoring + +type ``two pages with anchors end up in artifact`` () = + + static let generator = Setup.Generate [ + IndexFile """ +# A Document that lives at the root + +*Welcome* to this documentation + +## This anchor is autogenerated + +### Several pages can be created [#and-anchored] + +Through various means $$$including-this-inline-syntax$$$ +""" + MarkdownFile ("file.md", "*hello* world") + ] + + [] + let ``validate index.md HTML`` () = + generator |> converts "index.md" |> toHtml """ +

Welcometo this documentation

+
+

This anchor is autogenerated + +

+
+
+

Several pages can be created + +

+
+

Through various means

+ """ + + [] + let ``validate file.md HTML`` () = + generator |> converts "file.md" |> toHtml "

helloworld

" + + [] + let ``has no errors`` () = generator |> hasNoErrors + + [] + let ``validate links.json`` () = + generator |> convertsToJson ".artifacts/docs/html/links.json" """ + { + "origin": { + "branch": "test-e35fcb27-5f60-4e", + "remote": "elastic/docs-builder", + "ref": "e35fcb27-5f60-4e" + }, + "url_path_prefix": "", + "links": { + "file.md": [], + "index.md": [ + "including-this-inline-syntax", + "this-anchor-is-autogenerated", + "and-anchored" + ] + }, + "cross_links": [] + } + """ diff --git a/tests/authoring/authoring.fsproj b/tests/authoring/authoring.fsproj index 3c058305..fcfe76be 100644 --- a/tests/authoring/authoring.fsproj +++ b/tests/authoring/authoring.fsproj @@ -26,6 +26,7 @@ + @@ -39,6 +40,7 @@ + From 92448660ddce874d930aa83762c96b765ae63938 Mon Sep 17 00:00:00 2001 From: Martijn Laarman Date: Mon, 27 Jan 2025 22:06:08 +0100 Subject: [PATCH 2/6] rename union creation methods --- tests/authoring/Framework/Setup.fs | 10 +++++----- tests/authoring/Generator/LinkReferenceFile.fs | 4 ++-- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/tests/authoring/Framework/Setup.fs b/tests/authoring/Framework/Setup.fs index 6df08b7f..debfea61 100644 --- a/tests/authoring/Framework/Setup.fs +++ b/tests/authoring/Framework/Setup.fs @@ -24,11 +24,11 @@ type TestFile = | File of name: string * contents: string | MarkdownFile of name: string * markdown: Markdown - static member IndexFile([] m) = + static member Index ([] m) = MarkdownFile("index.md" , m) - static member Markdown([] m) = - MarkdownFile("index.md" , m) + static member Markdown path ([] m) = + MarkdownFile(path , m) type Setup = @@ -106,7 +106,7 @@ type Setup = /// Pass a full documentation page to the test setup static member Document ([]m: string) = - lazy (task { return! Setup.Generator [IndexFile m] } |> Async.AwaitTask |> Async.RunSynchronously) + lazy (task { return! Setup.Generator [Index m] } |> Async.AwaitTask |> Async.RunSynchronously) /// Pass a markdown fragment to the test setup static member Markdown ([]m: string) = @@ -115,7 +115,7 @@ type Setup = {m} """ lazy ( - task { return! Setup.Generator [IndexFile m] } + task { return! Setup.Generator [Index m] } |> Async.AwaitTask |> Async.RunSynchronously ) diff --git a/tests/authoring/Generator/LinkReferenceFile.fs b/tests/authoring/Generator/LinkReferenceFile.fs index 7ca43ec6..19adf24b 100644 --- a/tests/authoring/Generator/LinkReferenceFile.fs +++ b/tests/authoring/Generator/LinkReferenceFile.fs @@ -10,7 +10,7 @@ open authoring type ``two pages with anchors end up in artifact`` () = static let generator = Setup.Generate [ - IndexFile """ + Index """ # A Document that lives at the root *Welcome* to this documentation @@ -21,7 +21,7 @@ type ``two pages with anchors end up in artifact`` () = Through various means $$$including-this-inline-syntax$$$ """ - MarkdownFile ("file.md", "*hello* world") + Markdown "file.md" "*hello* world" ] [] From 745780ae30a954587bc7ab68721e85587713c43b Mon Sep 17 00:00:00 2001 From: Martijn Laarman Date: Mon, 27 Jan 2025 22:11:46 +0100 Subject: [PATCH 3/6] add license headers --- tests/authoring/Framework/MarkdownResultsAssertions.fs | 4 ++++ tests/authoring/Framework/TestValues.fs | 2 -- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/tests/authoring/Framework/MarkdownResultsAssertions.fs b/tests/authoring/Framework/MarkdownResultsAssertions.fs index 46b2a532..4749d1a6 100644 --- a/tests/authoring/Framework/MarkdownResultsAssertions.fs +++ b/tests/authoring/Framework/MarkdownResultsAssertions.fs @@ -1,3 +1,7 @@ +// Licensed to Elasticsearch B.V under one or more agreements. +// 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 + namespace authoring open System.Diagnostics diff --git a/tests/authoring/Framework/TestValues.fs b/tests/authoring/Framework/TestValues.fs index 1023574f..2bffd76c 100644 --- a/tests/authoring/Framework/TestValues.fs +++ b/tests/authoring/Framework/TestValues.fs @@ -6,8 +6,6 @@ namespace authoring open System open System.IO.Abstractions -open System.Threading -open System.Threading.Tasks open Elastic.Markdown open Elastic.Markdown.Diagnostics open Elastic.Markdown.IO From 14c14a1ba4836c28cc8a08c99bcea0819cb193f4 Mon Sep 17 00:00:00 2001 From: Martijn Laarman Date: Tue, 28 Jan 2025 12:05:04 +0100 Subject: [PATCH 4/6] make links on object holding anchors --- src/Elastic.Markdown/IO/State/LinkReference.cs | 15 +++++++++++++-- tests/authoring/Generator/LinkReferenceFile.fs | 14 ++++++++------ 2 files changed, 21 insertions(+), 8 deletions(-) diff --git a/src/Elastic.Markdown/IO/State/LinkReference.cs b/src/Elastic.Markdown/IO/State/LinkReference.cs index 301e9cbf..18cad4c0 100644 --- a/src/Elastic.Markdown/IO/State/LinkReference.cs +++ b/src/Elastic.Markdown/IO/State/LinkReference.cs @@ -7,6 +7,13 @@ namespace Elastic.Markdown.IO.State; +public record LinkMetadata +{ + [JsonPropertyName("anchors")] + [JsonIgnore(Condition = JsonIgnoreCondition.WhenWritingNull)] + public required string[]? Anchors { get; init; } = []; +} + public record LinkReference { [JsonPropertyName("origin")] @@ -17,7 +24,7 @@ public record LinkReference /// Mapping of relative filepath and all the page's anchors for deeplinks [JsonPropertyName("links")] - public required Dictionary Links { get; init; } = []; + public required Dictionary Links { get; init; } = []; [JsonPropertyName("cross_links")] public required string[] CrossLinks { get; init; } = []; @@ -27,7 +34,11 @@ public static LinkReference Create(DocumentationSet set) var crossLinks = set.Context.Collector.CrossLinks.ToHashSet().ToArray(); var links = set.MarkdownFiles.Values .Select(m => (m.RelativePath, m.Anchors)) - .ToDictionary(k => k.RelativePath, v => v.Anchors.ToArray()); + .ToDictionary(k => k.RelativePath, v => + { + var anchors = v.Anchors.Count == 0 ? null : v.Anchors.ToArray(); + return new LinkMetadata { Anchors = anchors }; + }); return new LinkReference { UrlPathPrefix = set.Context.UrlPathPrefix, diff --git a/tests/authoring/Generator/LinkReferenceFile.fs b/tests/authoring/Generator/LinkReferenceFile.fs index 19adf24b..21d916d0 100644 --- a/tests/authoring/Generator/LinkReferenceFile.fs +++ b/tests/authoring/Generator/LinkReferenceFile.fs @@ -59,12 +59,14 @@ Through various means $$$including-this-inline-syntax$$$ }, "url_path_prefix": "", "links": { - "file.md": [], - "index.md": [ - "including-this-inline-syntax", - "this-anchor-is-autogenerated", - "and-anchored" - ] + "file.md": {}, + "index.md": { + "anchors":[ + "including-this-inline-syntax", + "this-anchor-is-autogenerated", + "and-anchored" + ] + } }, "cross_links": [] } From b7711c03c3b6f454bfac3a0da9bb2a2e7e5af119 Mon Sep 17 00:00:00 2001 From: Martijn Laarman Date: Tue, 28 Jan 2025 12:07:30 +0100 Subject: [PATCH 5/6] dotnet format --- src/Elastic.Markdown/IO/State/LinkReference.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Elastic.Markdown/IO/State/LinkReference.cs b/src/Elastic.Markdown/IO/State/LinkReference.cs index 18cad4c0..caf29f4c 100644 --- a/src/Elastic.Markdown/IO/State/LinkReference.cs +++ b/src/Elastic.Markdown/IO/State/LinkReference.cs @@ -37,7 +37,7 @@ public static LinkReference Create(DocumentationSet set) .ToDictionary(k => k.RelativePath, v => { var anchors = v.Anchors.Count == 0 ? null : v.Anchors.ToArray(); - return new LinkMetadata { Anchors = anchors }; + return new LinkMetadata { Anchors = anchors }; }); return new LinkReference { From 81d20d2013f6b70d03018734af651e1540889d4a Mon Sep 17 00:00:00 2001 From: Martijn Laarman Date: Tue, 28 Jan 2025 14:02:31 +0100 Subject: [PATCH 6/6] Add tests for nested and indented inline anchors Introduced new tests to verify inline anchor generation within definition lists and indented code blocks. Ensures proper HTML output and validates parsing behavior to prevent future regressions. This removes indented code support from our parsers. The problem was that during our minimal parse phase anchors that lived in indented code (lists/definition lists) was not discovered because they were marked as part of an indented code block. Removing support for indented code blocks since all code should be included through fenced blocks. --- src/Elastic.Markdown/IO/MarkdownFile.cs | 6 +- .../IO/Navigation/DocumentationGroup.cs | 4 +- src/Elastic.Markdown/Myst/MarkdownParser.cs | 77 +++++++++++------ .../Framework/ErrorCollectorAssertions.fs | 3 +- tests/authoring/Framework/HtmlAssertions.fs | 31 ++++++- .../Framework/MarkdownDocumentAssertions.fs | 25 ++++++ tests/authoring/Framework/TestValues.fs | 4 +- tests/authoring/Inline/InlineAnchors.fs | 85 +++++++++++++++++++ tests/authoring/authoring.fsproj | 1 + 9 files changed, 199 insertions(+), 37 deletions(-) create mode 100644 tests/authoring/Framework/MarkdownDocumentAssertions.fs diff --git a/src/Elastic.Markdown/IO/MarkdownFile.cs b/src/Elastic.Markdown/IO/MarkdownFile.cs index e72b91e6..75d1a835 100644 --- a/src/Elastic.Markdown/IO/MarkdownFile.cs +++ b/src/Elastic.Markdown/IO/MarkdownFile.cs @@ -89,7 +89,7 @@ public MarkdownFile[] YieldParents() return parents.ToArray(); } - public async Task MinimalParse(Cancel ctx) + public async Task MinimalParseAsync(Cancel ctx) { var document = await MarkdownParser.MinimalParseAsync(SourceFile, ctx); ReadDocumentInstructions(document); @@ -99,7 +99,7 @@ public async Task MinimalParse(Cancel ctx) public async Task ParseFullAsync(Cancel ctx) { if (!_instructionsParsed) - await MinimalParse(ctx); + await MinimalParseAsync(ctx); var document = await MarkdownParser.ParseAsync(SourceFile, YamlFrontMatter, ctx); return document; @@ -176,6 +176,8 @@ private void ReadDocumentInstructions(MarkdownDocument document) foreach (var t in contents) _tableOfContent[t.Slug] = t; + var inlineAnchros = document.Descendants().ToList(); + var anchors = document.Descendants() .Select(b => b.CrossReferenceName) .Where(l => !string.IsNullOrWhiteSpace(l)) diff --git a/src/Elastic.Markdown/IO/Navigation/DocumentationGroup.cs b/src/Elastic.Markdown/IO/Navigation/DocumentationGroup.cs index 3a80550a..3dd78363 100644 --- a/src/Elastic.Markdown/IO/Navigation/DocumentationGroup.cs +++ b/src/Elastic.Markdown/IO/Navigation/DocumentationGroup.cs @@ -146,10 +146,10 @@ public async Task Resolve(Cancel ctx = default) if (_resolved) return; - await Parallel.ForEachAsync(FilesInOrder, ctx, async (file, token) => await file.MinimalParse(token)); + await Parallel.ForEachAsync(FilesInOrder, ctx, async (file, token) => await file.MinimalParseAsync(token)); await Parallel.ForEachAsync(GroupsInOrder, ctx, async (group, token) => await group.Resolve(token)); - await (Index?.MinimalParse(ctx) ?? Task.CompletedTask); + await (Index?.MinimalParseAsync(ctx) ?? Task.CompletedTask); _resolved = true; } diff --git a/src/Elastic.Markdown/Myst/MarkdownParser.cs b/src/Elastic.Markdown/Myst/MarkdownParser.cs index 3f6a956e..fd731090 100644 --- a/src/Elastic.Markdown/Myst/MarkdownParser.cs +++ b/src/Elastic.Markdown/Myst/MarkdownParser.cs @@ -14,6 +14,7 @@ using Elastic.Markdown.Myst.Substitution; using Markdig; using Markdig.Extensions.EmphasisExtras; +using Markdig.Parsers; using Markdig.Syntax; namespace Elastic.Markdown.Myst; @@ -28,34 +29,56 @@ public class MarkdownParser( private BuildContext Context { get; } = context; - public static MarkdownPipeline MinimalPipeline { get; } = - new MarkdownPipelineBuilder() - .UseYamlFrontMatter() - .UseInlineAnchors() - .UseHeadingsWithSlugs() - .UseDirectives() - .Build(); + // ReSharper disable once InconsistentNaming + private static MarkdownPipeline? _minimalPipeline; + public static MarkdownPipeline MinimalPipeline + { + get + { + if (_minimalPipeline is not null) return _minimalPipeline; + var builder = new MarkdownPipelineBuilder() + .UseYamlFrontMatter() + .UseInlineAnchors() + .UseHeadingsWithSlugs() + .UseDirectives(); + + builder.BlockParsers.TryRemove(); + _minimalPipeline = builder.Build(); + return _minimalPipeline; + + } + } + + // ReSharper disable once InconsistentNaming + private static MarkdownPipeline? _pipeline; + public static MarkdownPipeline Pipeline { + get + { + if (_pipeline is not null) return _pipeline; - public static MarkdownPipeline Pipeline { get; } = - new MarkdownPipelineBuilder() - .EnableTrackTrivia() - .UseInlineAnchors() - .UsePreciseSourceLocation() - .UseDiagnosticLinks() - .UseHeadingsWithSlugs() - .UseEmphasisExtras(EmphasisExtraOptions.Default) - .UseSoftlineBreakAsHardlineBreak() - .UseSubstitution() - .UseComments() - .UseYamlFrontMatter() - .UseGridTables() - .UsePipeTables() - .UseDirectives() - .UseDefinitionLists() - .UseEnhancedCodeBlocks() - .DisableHtml() - .UseHardBreaks() - .Build(); + var builder = new MarkdownPipelineBuilder() + .EnableTrackTrivia() + .UseInlineAnchors() + .UsePreciseSourceLocation() + .UseDiagnosticLinks() + .UseHeadingsWithSlugs() + .UseEmphasisExtras(EmphasisExtraOptions.Default) + .UseSoftlineBreakAsHardlineBreak() + .UseSubstitution() + .UseComments() + .UseYamlFrontMatter() + .UseGridTables() + .UsePipeTables() + .UseDirectives() + .UseDefinitionLists() + .UseEnhancedCodeBlocks() + .DisableHtml() + .UseHardBreaks(); + builder.BlockParsers.TryRemove(); + _pipeline = builder.Build(); + return _pipeline; + } + } public ConfigurationFile Configuration { get; } = configuration; diff --git a/tests/authoring/Framework/ErrorCollectorAssertions.fs b/tests/authoring/Framework/ErrorCollectorAssertions.fs index 9ceff469..1366f159 100644 --- a/tests/authoring/Framework/ErrorCollectorAssertions.fs +++ b/tests/authoring/Framework/ErrorCollectorAssertions.fs @@ -16,7 +16,8 @@ module DiagnosticsCollectorAssertions = [] let hasNoErrors (actual: Lazy) = let actual = actual.Value - test <@ actual.Context.Collector.Errors = 0 @> + let errors = actual.Context.Collector.Errors + test <@ errors = 0 @> [] let hasError (expected: string) (actual: Lazy) = diff --git a/tests/authoring/Framework/HtmlAssertions.fs b/tests/authoring/Framework/HtmlAssertions.fs index dc55543a..e9a1836e 100644 --- a/tests/authoring/Framework/HtmlAssertions.fs +++ b/tests/authoring/Framework/HtmlAssertions.fs @@ -14,6 +14,7 @@ open AngleSharp.Html.Parser open DiffPlex.DiffBuilder open DiffPlex.DiffBuilder.Model open JetBrains.Annotations +open Swensen.Unquote open Xunit.Sdk [] @@ -87,7 +88,7 @@ actual: {actual} use sw = new StringWriter() document.Body.Children |> Seq.iter _.ToHtml(sw, PrettyMarkupFormatter()) - sw.ToString() + sw.ToString().TrimStart('\n') let private createDiff expected actual = let diffs = @@ -110,15 +111,37 @@ actual: {actual} """ raise (XunitException(msg)) + [] + let toHtml ([]expected: string) (actual: MarkdownResult) = + createDiff expected actual.Html + [] let convertsToHtml ([]expected: string) (actual: Lazy) = let actual = actual.Value let defaultFile = actual.MarkdownResults |> Seq.head - createDiff expected defaultFile.Html + defaultFile |> toHtml expected [] - let toHtml ([]expected: string) (actual: MarkdownResult) = - createDiff expected actual.Html + let containsHtml ([]expected: string) (actual: MarkdownResult) = + + let prettyExpected = prettyHtml expected + let prettyActual = prettyHtml actual.Html + + if not <| prettyActual.Contains prettyExpected then + let msg = $"""Expected html to contain: +{prettyExpected} +But was not found in: +{prettyActual} +""" + raise (XunitException(msg)) + + + [] + let convertsToContainingHtml ([]expected: string) (actual: Lazy) = + let actual = actual.Value + + let defaultFile = actual.MarkdownResults |> Seq.head + defaultFile |> containsHtml expected diff --git a/tests/authoring/Framework/MarkdownDocumentAssertions.fs b/tests/authoring/Framework/MarkdownDocumentAssertions.fs new file mode 100644 index 00000000..c5d3c3a7 --- /dev/null +++ b/tests/authoring/Framework/MarkdownDocumentAssertions.fs @@ -0,0 +1,25 @@ +// Licensed to Elasticsearch B.V under one or more agreements. +// 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 + +namespace authoring + +open System.Diagnostics +open Markdig.Syntax +open Xunit.Sdk + +module MarkdownDocumentAssertions = + + [] + let parses<'element when 'element :> MarkdownObject> (actual: MarkdownResult) = + let unsupportedBlocks = actual.Document.Descendants<'element>() |> Array.ofSeq + if unsupportedBlocks.Length = 0 then + raise (XunitException($"Could not find {typedefof<'element>.Name} in fully parsed document")) + unsupportedBlocks; + + [] + let parsesMinimal<'element when 'element :> MarkdownObject> (actual: MarkdownResult) = + let unsupportedBlocks = actual.MinimalParse.Descendants<'element>() |> Array.ofSeq + if unsupportedBlocks.Length = 0 then + raise (XunitException($"Could not find {typedefof<'element>.Name} in minimally parsed document")) + unsupportedBlocks; diff --git a/tests/authoring/Framework/TestValues.fs b/tests/authoring/Framework/TestValues.fs index 2bffd76c..6fbd89da 100644 --- a/tests/authoring/Framework/TestValues.fs +++ b/tests/authoring/Framework/TestValues.fs @@ -61,6 +61,7 @@ type TestLoggerFactory () = type MarkdownResult = { File: MarkdownFile + MinimalParse: MarkdownDocument Document: MarkdownDocument Html: string Context: MarkdownTestContext @@ -89,8 +90,9 @@ and MarkdownTestContext = |> Seq.map (fun (f: MarkdownFile) -> task { // technically we do this work twice since generate all also does it let! document = f.ParseFullAsync(ctx) + let! minimal = f.MinimalParseAsync(ctx) let html = f.CreateHtml(document) - return { File = f; Document = document; Html = html; Context = this } + return { File = f; Document = document; MinimalParse = minimal; Html = html; Context = this } }) // this is not great code, refactor or depend on FSharp.Control.TaskSeq // for now this runs without issue diff --git a/tests/authoring/Inline/InlineAnchors.fs b/tests/authoring/Inline/InlineAnchors.fs index 73f9552b..e068f4f5 100644 --- a/tests/authoring/Inline/InlineAnchors.fs +++ b/tests/authoring/Inline/InlineAnchors.fs @@ -4,8 +4,13 @@ module ``inline elements``.``anchors DEPRECATED`` +open Elastic.Markdown.Myst.InlineParsers +open Markdig.Syntax +open Swensen.Unquote +open System.Linq open Xunit open authoring +open authoring.MarkdownDocumentAssertions type ``inline anchor in the middle`` () = @@ -22,3 +27,83 @@ this is *regular* text and this $$$is-an-inline-anchor$$$ and this continues to """ [] let ``has no errors`` () = markdown |> hasNoErrors + +type ``inline anchors embedded in definition lists`` () = + + static let markdown = Setup.Generate [ + Index """# Testing nested inline anchors + +$$$search-type$$$ + +`search_type` +: (Optional, string) How distributed term frequencies are calculated for relevance scoring. + + ::::{dropdown} Valid values for `search_type` + `query_then_fetch` + : (Default) Distributed term frequencies are calculated locally for each shard running the search. We recommend this option for faster searches with potentially less accurate scoring. + + $$$dfs-query-then-fetch$$$ + + `dfs_query_then_fetch` + : Distributed term frequencies are calculated globally, using information gathered from all shards running the search. + + :::: +""" + Markdown "file.md" """ + [Link to first](index.md#search-type) + [Link to second](index.md#dfs-query-then-fetch) + """ + ] + + [] + let ``emits nested inline anchor`` () = + markdown |> convertsToContainingHtml """""" + + [] + let ``emits definition list block anchor`` () = + markdown |> convertsToContainingHtml """""" + + [] + let ``has no errors`` () = markdown |> hasNoErrors + + [] + let ``minimal parse sees two inline anchors`` () = + let inlineAnchors = markdown |> converts "index.md" |> parsesMinimal + test <@ inlineAnchors.Length = 2 @> + + + +type ``inline anchors embedded in indented code`` () = + + static let markdown = Setup.Generate [ + Index """# Testing nested inline anchors + +$$$search-type$$$ + + indented codeblock + + $$$dfs-query-then-fetch$$$ + + block +""" + Markdown "file.md" """ + [Link to first](index.md#search-type) + [Link to second](index.md#dfs-query-then-fetch) + """ + ] + + [] + let ``emits nested inline anchor`` () = + markdown |> convertsToContainingHtml """""" + + [] + let ``emits definition list block anchor`` () = + markdown |> convertsToContainingHtml """""" + + [] + let ``has no errors`` () = markdown |> hasNoErrors + + [] + let ``minimal parse sees two inline anchors`` () = + let inlineAnchors = markdown |> converts "index.md" |> parsesMinimal + test <@ inlineAnchors.Length = 2 @> diff --git a/tests/authoring/authoring.fsproj b/tests/authoring/authoring.fsproj index fcfe76be..663ca37f 100644 --- a/tests/authoring/authoring.fsproj +++ b/tests/authoring/authoring.fsproj @@ -29,6 +29,7 @@ +