-
Notifications
You must be signed in to change notification settings - Fork 31
Add ability to set canonical base url #776
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR adds support for specifying a canonical base URL throughout the documentation site. Key changes include:
- Introducing a new nullable property CanonicalBaseUrl in BuildContext and updating related view models.
- Adding a new CLI parameter for canonicalBaseUrl and wiring it through the documentation generation process.
- Incorporating the canonical URL tag in the rendered HTML head section.
Reviewed Changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated no comments.
Show a summary per file
File | Description |
---|---|
src/Elastic.Markdown/BuildContext.cs | Adds a new property to hold the canonical base URL |
src/Elastic.Markdown/Slices/_ViewModels.cs | Updates view models to include the canonical base URL property |
src/docs-builder/Cli/Commands.cs | Introduces the canonicalBaseUrl CLI parameter and propagates it |
src/Elastic.Markdown/Slices/HtmlWriter.cs | Incorporates trimming for the canonical base URL before usage |
src/Elastic.Markdown/Slices/Index.cshtml | Passes canonicalBaseUrl to the view |
src/Elastic.Markdown/Slices/Layout/_Head.cshtml | Renders the canonical link tag based on the canonical base URL |
Comments suppressed due to low confidence (1)
src/Elastic.Markdown/Slices/Layout/_Head.cshtml:13
- Consider using a URL join method or ensuring proper slash handling when concatenating Model.CanonicalBaseUrl and Model.CurrentDocument.Url to avoid malformed canonical URLs.
<link rel="canonical" href="@(Model.CanonicalBaseUrl + Model.CurrentDocument.Url)">
src/Elastic.Markdown/BuildContext.cs
Outdated
public bool AllowIndexing { get; init; } | ||
|
||
// This property is used for the canonical URL | ||
public string? CanonicalBaseUrl { get; init; } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Uri
instead of string
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
|
||
@if (Model.CanonicalBaseUrl is not null) | ||
{ | ||
<link rel="canonical" href="@(Model.CanonicalBaseUrl + Model.CurrentDocument.Url)"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
<link rel="canonical" href="@(Model.CanonicalBaseUrl + Model.CurrentDocument.Url)"> | |
<link rel="canonical" href="@(new Uri(Model.CanonicalBaseUri, Model.CurrentDocument.Url))"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
src/docs-builder/Cli/Commands.cs
Outdated
Force = force ?? false, | ||
AllowIndexing = allowIndexing ?? false | ||
AllowIndexing = allowIndexing ?? false, | ||
CanonicalBaseUrl = canonicalBaseUrl |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Uri.TryCreate(...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No description provided.