Skip to content

API docs: dedent examples + scrollbar#54

Merged
PrzemyslawKlys merged 1 commit intoweb/apidocs-creffrom
web/apidocs-dedent
Feb 5, 2026
Merged

API docs: dedent examples + scrollbar#54
PrzemyslawKlys merged 1 commit intoweb/apidocs-creffrom
web/apidocs-dedent

Conversation

@PrzemyslawKlys
Copy link
Member

  • Dedent XML code examples to fix misaligned code blocks.
  • Style API sidebar scrollbars for consistency.

Copilot AI review requested due to automatic review settings February 5, 2026 08:41
@claude
Copy link

claude bot commented Feb 5, 2026

Pull Request Review

This PR makes two focused improvements to the API documentation system: dedenting XML code examples and styling sidebar scrollbars. Overall, the changes are well-implemented, but I have a few suggestions for consideration.


What's Good

  1. Clean separation of concerns: The CSS changes are isolated to scrollbar styling, and the C# changes focus solely on code formatting.

  2. Proper dedent implementation: The Dedent method correctly:

    • Finds minimum indentation across non-empty lines
    • Handles edge cases (empty strings, no common indent)
    • Preserves relative indentation within code blocks
    • Uses StringBuilder for efficient string building
  3. Cross-browser scrollbar support: The CSS includes both WebKit-specific selectors and the standard scrollbar-width/scrollbar-color properties for Firefox compatibility.

  4. Non-intrusive styling: The scrollbar colors use subtle transparency that won't clash with the existing dark theme.


🔍 Potential Issues & Suggestions

1. Tab Character Handling (Minor Bug)

Location: PowerForge.Web/Services/WebApiDocsGenerator.cs:2108

The current dedent logic only counts space characters:

while (indent < line.Length && line[indent] == ' ') indent++;

Issue: If XML code examples use tabs or mixed spaces/tabs, the dedent calculation will be incorrect.

Suggestion: Consider handling tabs, or at minimum, document this limitation:

// Count leading whitespace (spaces only; tabs not supported)
while (indent < line.Length && line[indent] == ' ') indent++;

Or handle tabs more robustly:

while (indent < line.Length && char.IsWhiteSpace(line[indent])) indent++;

Note: If you use char.IsWhiteSpace, be aware it includes newlines, so you'd need to explicitly check for space and tab only.


2. Empty Line Handling Inconsistency (Code Clarity)

Location: PowerForge.Web/Services/WebApiDocsGenerator.cs:2117-2118

if (string.IsNullOrWhiteSpace(line))
    sb.Append("");

Issue: Appending an empty string is a no-op. The intent appears to be preserving empty lines, but this is already handled by the \n appended on line 2115.

Suggestion: Either remove the redundant append or add a clarifying comment:

if (string.IsNullOrWhiteSpace(line))
    continue; // Empty lines are preserved via newlines

Or if you intentionally want to preserve whitespace-only lines as truly empty:

if (string.IsNullOrWhiteSpace(line))
    sb.Append(string.Empty); // Normalize whitespace-only lines to empty

3. Test Coverage Gap

Observation: There's no unit test for the new Dedent method.

Suggestion: Add test cases covering:

  • Standard indented code blocks
  • Mixed indentation levels
  • Edge cases: empty strings, single-line code, no indentation
  • Code with empty lines in the middle

Example test structure:

[Theory]
[InlineData("    line1\n    line2", "line1\nline2")]
[InlineData("  line1\n    line2", "line1\n  line2")]
[InlineData("line1\nline2", "line1\nline2")]
public void Dedent_RemovesCommonLeadingWhitespace(string input, string expected)
{
    // Test implementation
}

4. CSS Specificity (Minor Consideration)

Location: PowerForge.Web/Assets/ApiDocs/fallback.css:137-141

The scrollbar styles target both .api-sidebar and .sidebar-nav.

Question: Are these separate elements, or is .sidebar-nav a child of .api-sidebar? If they're the same element or parent-child, you might be able to simplify the selectors.


🎯 Performance & Security

  • Performance: The Dedent implementation is O(n) where n is total characters, which is optimal. StringBuilder usage is appropriate.
  • Security: No injection risks or security concerns. The method operates on already-parsed XML content.
  • Memory: Acceptable memory usage for typical documentation strings.

📋 Summary

Approve with suggestions: The core functionality is sound and the changes are beneficial. The main improvement would be adding tests and clarifying the tab handling behavior.

Recommended actions before merge:

  1. ✅ Consider tab character handling in Dedent
  2. ✅ Add unit tests for the Dedent method
  3. ✅ Clean up the empty line handling logic for clarity

Rating: 8/10 — Good implementation with room for minor improvements in edge case handling and test coverage.


🔗 References

  • Dedent implementation: PowerForge.Web/Services/WebApiDocsGenerator.cs:2099-2123
  • Usage: PowerForge.Web/Services/WebApiDocsGenerator.cs:2063
  • CSS changes: PowerForge.Web/Assets/ApiDocs/fallback.css:137-141

Reviewed by Claude Sonnet 4.5

Copy link

Copilot AI left a 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 improves the API documentation rendering by dedenting XML code examples to prevent misaligned code blocks and adds custom scrollbar styling to API sidebars for better visual consistency.

Changes:

  • Added a new Dedent method to remove common leading whitespace from XML code examples
  • Applied the dedenting logic to code blocks in API documentation
  • Styled scrollbars for .api-sidebar and .sidebar-nav with webkit-specific and standard CSS properties

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

File Description
PowerForge.Web/Services/WebApiDocsGenerator.cs Implements dedenting logic for XML code examples to fix alignment issues
PowerForge.Web/Assets/ApiDocs/fallback.css Adds consistent scrollbar styling for API navigation sidebars

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

var line = lines[i];
if (string.IsNullOrWhiteSpace(line)) continue;
var indent = 0;
while (indent < line.Length && line[indent] == ' ') indent++;
Copy link

Copilot AI Feb 5, 2026

Choose a reason for hiding this comment

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

The Dedent method only counts space characters for indentation, but does not handle tab characters. If XML code examples use tabs for indentation, those lines will be treated as having zero indentation, which will cause incorrect dedenting behavior. Consider also counting tab characters or normalizing tabs to spaces before processing.

Suggested change
while (indent < line.Length && line[indent] == ' ') indent++;
while (indent < line.Length && (line[indent] == ' ' || line[indent] == '\t')) indent++;

Copilot uses AI. Check for mistakes.
Comment on lines +2117 to +2119
if (string.IsNullOrWhiteSpace(line))
sb.Append("");
else
Copy link

Copilot AI Feb 5, 2026

Choose a reason for hiding this comment

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

Appending an empty string has no effect. This line can be removed since the StringBuilder will already contain an empty string representation for blank lines (just the newline character from line 2115).

Suggested change
if (string.IsNullOrWhiteSpace(line))
sb.Append("");
else
if (!string.IsNullOrWhiteSpace(line))

Copilot uses AI. Check for mistakes.

private static string Dedent(string code)
{
var lines = code.Split('\n');
Copy link

Copilot AI Feb 5, 2026

Choose a reason for hiding this comment

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

When splitting by '\n', any remaining '\r' characters from Windows line endings (\r\n) will be included in each line element. While this doesn't break the dedenting logic, it results in inconsistent line endings in the output. Consider splitting by '\n' and then trimming '\r' from each line, or using Split(new[] {"\r\n", "\n"}, StringSplitOptions.None) to handle both line ending styles consistently.

Suggested change
var lines = code.Split('\n');
var lines = code.Split(new[] { "\r\n", "\n" }, StringSplitOptions.None);

Copilot uses AI. Check for mistakes.
@PrzemyslawKlys PrzemyslawKlys merged commit 9a9d5c5 into web/apidocs-cref Feb 5, 2026
7 checks passed
@PrzemyslawKlys PrzemyslawKlys deleted the web/apidocs-dedent branch February 5, 2026 09:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants