Next gen - #518
Conversation
There was a problem hiding this comment.
Pull request overview
This PR updates BookGen to a “next gen” baseline by moving projects to .NET 11, refactoring the embedded HTTP server factory, and introducing a new folder preview workflow that serves rendered Markdown with live updates via a file-system observer.
Changes:
- Upgrade projects and CI workflows to target net11.0 and updated dependency lockfiles.
- Add folder preview command + preview HTML template + preview route plumbing.
- Introduce
IFileSystemObserver+CreateObserver(...)onIReadOnlyFileSystemand implement it in VFS file systems.
Reviewed changes
Copilot reviewed 39 out of 40 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| Test/Bookgen.Tests/EmbeddedTestFolder.cs | Adds CreateObserver stub to satisfy updated IReadOnlyFileSystem contract in tests. |
| Test/Bookgen.Tests/Bookgen.Tests.csproj | Targets net11.0 for test project. |
| Source/BookGen/Properties/launchSettings.json | Adds a folder preview launch profile. |
| Source/BookGen/packages.lock.json | Updates lockfile to net11.0 + new dependency graph. |
| Source/BookGen/Commands/GuiCommand.cs | Updates references from ServerFactory to HttpServerFactory. |
| Source/BookGen/Commands/folder/ServeCommand.cs | Updates references from ServerFactory to HttpServerFactory. |
| Source/BookGen/Commands/Folder/PreviewCommand.cs | New CLI command to run preview server with folder lock. |
| Source/BookGen/BookGen.csproj | Targets net11.0 and adjusts Microsoft.Extensions package references. |
| Source/BookGen.Vfs/ReadOnlyFileSystem.cs | Implements CreateObserver(...) with scope validation. |
| Source/BookGen.Vfs/MultiReadScopeFileSystem.cs | Implements CreateObserver(...) with single-scope restriction. |
| Source/BookGen.Vfs/IReadOnlyFileSystem.cs | Adds CreateObserver(ILogger, string) to the VFS abstraction. |
| Source/BookGen.Vfs/IFileSystemObserver.cs | New observer interface for filesystem change events. |
| Source/BookGen.Vfs/FileSystemObserver.cs | New FileSystemWatcher wrapper that emits typed change events. |
| Source/BookGen.Vfs/FileSystemChange.cs | New FileSystemChangeEventArgs DTO. |
| Source/BookGen.Vfs/BookGen.Vfs.csproj | Targets net11.0 for VFS project. |
| Source/BookGen.Shellprog/packages.lock.json | Updates lockfile to net11.0 + new dependency graph. |
| Source/BookGen.Shellprog/BookGen.Shellprog.csproj | Targets net11.0 and adjusts Microsoft.Extensions package references. |
| Source/BookGen.Shell.Shared/packages.lock.json | Updates lockfile to net11.0 + new dependency graph. |
| Source/BookGen.Shell.Shared/BookGen.Shell.Shared.csproj | Targets net11.0 and adjusts package references. |
| Source/BookGen.Lib/packages.lock.json | Updates lockfile to net11.0 + new dependency graph. |
| Source/BookGen.Lib/Http/QRCodeTemplate.html | Updates template placeholder casing for {{Links}}. |
| Source/Bookgen.Lib/Http/PreviewRoutes.cs | New preview route provider with live file-list updates. |
| Source/BookGen.Lib/Http/PageFactory.cs | Adds file listing HTML generator; aligns template placeholders ({{Code}}, {{Links}}). |
| Source/BookGen.Lib/Http/MimeTypes.cs | Removes unused usings. |
| Source/Bookgen.Lib/Http/IRouteProvider.cs | New route-provider interface for registering grouped routes. |
| Source/Bookgen.Lib/Http/HttpServerFactory.cs | Renames/extends server factory; adds preview server creation. |
| Source/BookGen.Lib/Http/HttpServer.cs | Adds localhost-only binding option and disposable tracking for route providers. |
| Source/BookGen.Lib/Http/ErrorPageTemplate.html | Fixes placeholder casing and MDN link placeholder. |
| Source/Bookgen.Lib/DisposableTracker.cs | New helper to dispose tracked IDisposable instances. |
| Source/BookGen.Lib/BundledAssets.cs | Adds TemplatePreview asset constant. |
| Source/BookGen.Lib/BookGen.Lib.csproj | Targets net11.0 for lib project. |
| Source/BookGen.Contents/BookGen.Contents.csproj | Targets net11.0 for contents project. |
| Source/BookGen.Cli/packages.lock.json | Updates lockfile to net11.0 + new dependency graph. |
| Source/BookGen.Cli/BookGen.Cli.csproj | Targets net11.0 and adjusts package references. |
| Docs/changelog.md | Notes .NET 11 base and new preview command. |
| Docs/bookgen.lock | Adds a lock file artifact. |
| Directory.Packages.props | Updates Microsoft.Extensions + Syndication versions to 11 preview; removes Abstractions entries. |
| Assets/BundledAssets/Preview.html | Adds bundled preview HTML template. |
| .github/workflows/dotnet.yml | Updates CI to .NET 11 preview. |
| .github/workflows/codeql.yml | Updates CodeQL build to .NET 11 preview. |
Comments suppressed due to low confidence (5)
Source/Bookgen.Lib/Http/HttpServerFactory.cs:20
- These new/modified files are under
Source/Bookgen.Lib/...(lowercaseg), but the project is rooted atSource/BookGen.Lib/BookGen.Lib.csproj. On case-sensitive filesystems (Linux CI), SDK-style projects only compile files under the csproj directory, so HttpServerFactory/PreviewRoutes/IRouteProvider/DisposableTracker likely won’t be included and references to them will fail to build. Move them underSource/BookGen.Lib/...(or explicitly include them from the csproj).
Source/Bookgen.Lib/Http/HttpServerFactory.cs:76 - CreateServerForPreview defines PreviewPort=8181 but still calls ChoosePort() with the default HostingPort (8081). This can cause preview to collide with the directory hosting server port and makes PreviewPort unused.
Source/Bookgen.Lib/Http/PreviewRoutes.cs:176 - CanServe reads _allowedFiles without the lock used by OnFileChange. This can race with updates from the watcher and intermittently fail/throw under load.
Source/BookGen.Cli/BookGen.Cli.csproj:16 - BookGen.Cli code uses Microsoft.Extensions.DependencyInjection and Microsoft.Extensions.Logging, but the project no longer references those packages. Unless these assemblies are provided via a framework reference, this will fail to compile.
<ItemGroup>
<PackageReference Include="Roslynator.Analyzers">
<PrivateAssets>all</PrivateAssets>
<IncludeAssets>runtime; build; native; contentfiles; analyzers; buildtransitive</IncludeAssets>
</PackageReference>
Source/BookGen.Shell.Shared/BookGen.Shell.Shared.csproj:16
- BookGen.Shell.Shared contains multiple ILogger usages/implementations, but the csproj no longer references Microsoft.Extensions.Logging (or Logging.Abstractions). This will likely fail to compile.
<PackageReference Include="Roslynator.Analyzers">
<PrivateAssets>all</PrivateAssets>
<IncludeAssets>runtime; build; native; contentfiles; analyzers; buildtransitive</IncludeAssets>
</PackageReference>
<PackageReference Include="Webmaster442.WindowsTerminal" />
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 41 out of 42 changed files in this pull request and generated 3 comments.
Comments suppressed due to low confidence (2)
Source/BookGen.Lib/Http/HttpServerFactory.cs:76
- CreateServerForPreview defines PreviewPort but doesn't use it: it calls ChoosePort() which defaults to HostingPort. Running preview alongside directory hosting can collide on the same port range. Pass PreviewPort into ChoosePort to keep the two server types separated by default.
Source/BookGen.Vfs/BookGen.Vfs.csproj:4 - BookGen.Vfs now depends on Microsoft.Extensions.Logging types (ILogger) in public API (IReadOnlyFileSystem.CreateObserver / FileSystemObserver), but the project file has no PackageReference for Microsoft.Extensions.Logging. Add the package reference so the project compiles independently.
<TargetFramework>net11.0</TargetFramework>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 64 out of 65 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (13)
Source/BookGen.Lib/Http/HttpServerFactory.cs:77
- CreateServerForPreview defines PreviewPort but still calls ChoosePort() without passing PreviewPort, so preview may bind to the hosting port range instead of the dedicated preview port.
Test/Bookgen.Tests/EmbeddedTestFolder.cs:29 - EmbeddedTestFileSystem.CreateObserver currently throws NotImplementedException. With IReadOnlyFileSystem now requiring CreateObserver and PreviewRoutes using it, this will cause runtime failures if any tests exercise preview functionality with this file system.
Source/BookGen.Lib/Rendering/Markdown/Renderers/SyntaxRenderPlugins/CsvRenderPlugin.cs:46 - Switch cases should use the corrected constant names after renaming (CommaSeparated/TabSeparated/SemicolonSeparated).
return parsedLanguageMoniker switch
{
ComaSeperated => ',',
TabSeperated => '\t',
SemicolonSeperated => ';',
Source/BookGen.Lib/Http/QRCodeTemplate.html:165
- The page title has a typo: "Liks" -> "Links".
<h1>Liks for this server</h1>
{{Links}}
Source/BookGen.Lib/Http/ErrorPageTemplate.html:163
- User-facing text typo: "occured" -> "occurred".
An error occured during serving your request. <br>
Test/Bookgen.Tests/Lib/UT_MarkdownConverter_Specials.cs:267
- Test name says "Csv" but the test cases include TSV and SSV as well. Renaming the test would better reflect what it covers.
Source/BookGen.Lib/Rendering/Markdown/Renderers/SyntaxRenderPlugins/CsvRenderPlugin.cs:19 - Identifier typos reduce readability: "ComaSeperated" / "Seperated" should be "CommaSeparated" / "Separated".
This issue also appears on line 42 of the same file.
private const string ComaSeperated = "table-csv";
private const string TabSeperated = "table-tsv";
private const string SemicolonSeperated = "table-ssv";
public override string[] LanguageMonikers { get; }
Source/BookGen/Tooldownloaders/PandocTooldownloader.cs:28
- ToolInfo currently allocates a new ToolInfo instance on every access. Since ToolInfo is immutable, expose it as a cached get-only property to avoid repeated allocations (ToolsCommand calls ToolInfo multiple times per menu render).
Source/BookGen/Tooldownloaders/GlowDownloader.cs:28 - ToolInfo currently allocates a new ToolInfo instance on every access. Since ToolInfo is immutable, expose it as a cached get-only property to avoid repeated allocations.
Source/BookGen/Tooldownloaders/MicrosoftEditToolDownloader.cs:28 - ToolInfo currently allocates a new ToolInfo instance on every access. Since ToolInfo is immutable, expose it as a cached get-only property to avoid repeated allocations.
Source/BookGen/Tooldownloaders/GithubDownloader.cs:28 - ToolInfo currently allocates a new ToolInfo instance on every access. Since ToolInfo is immutable, expose it as a cached get-only property to avoid repeated allocations. Also, the user-facing tool name should be "GitHub CLI".
Source/BookGen/Tooldownloaders/CopyPartyDownloader.cs:28 - ToolInfo currently allocates a new ToolInfo instance on every access. Since ToolInfo is immutable, expose it as a cached get-only property to avoid repeated allocations.
Source/BookGen/Tooldownloaders/ChromaDownloader.cs:28 - ToolInfo currently allocates a new ToolInfo instance on every access. Since ToolInfo is immutable, expose it as a cached get-only property to avoid repeated allocations.
No description provided.