Skip to content

Harden our Check for Common-Leading-Whitespace in Doc-Comments - #829

Merged
InsertCreativityHere merged 3 commits into
icerpc:mainfrom
InsertCreativityHere:fix-doc-comment-leading-space-stripping
Sep 16, 2026
Merged

InsertCreativityHere merged 3 commits into
icerpc:mainfrom
InsertCreativityHere:fix-doc-comment-leading-space-stripping

Conversation

@InsertCreativityHere

@InsertCreativityHere InsertCreativityHere commented Sep 14, 2026 •

Copy link
Copy Markdown
Member

This PR is a code-hardening to handle doc-comments where a user has different kinds of whitespace at the starts of their doc-comment lines. It fixes #800.


It is common for users to have whitespace between a /// and the actual comment's text:

/// See how there's a space after '///' here?.
///     - This line could be indented as a bullet point.
///And this line has no leading whitespace at all.

We have code to detect this and strip it away from the comment's actual content.
The exact check is to determine the "common leading whitespace" and remove it; i.e. find the maximum amount of whitespace such that every line starts with at least that amount of whitespace.

However, this check isn't very exact right now. We currently only count the number of whitespace characters. So if I have a multiline comment, one starts with 2 spaces, and one starts with 2 tabs, we'd count those as the same amount of whitespace, which is wrong. If you have 2 spaces and 2 non-ASCII whitespace characters, this could ever cause a panic, since we're counting over bytes instead of unicode characters.

This PR changes this so we track the whitespace as an actual str, not just a count of characters. We determine the largest substring such that:

  • It is only made of whitespace characters
  • Every single line of a comment starts with it.

And like before, we strip it from every single line.

What's Changed

Nothing - this only affects doc-comments where an inconsistent kind of whitespace is used to pad the start of each line.
In practice, almost all users will use a consistent kind of whitespace padding.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟡 Changes recommended

Regression tests are needed for mixed ASCII, tab, and multibyte whitespace cases.

Get a fresh assessment by requesting another Copilot review.

Pull request overview

Hardens doc-comment indentation stripping against mixed and multibyte whitespace.

Changes:

  • Computes the exact common whitespace prefix.
  • Safely strips prefixes at UTF-8 boundaries.
File summaries
File Description
slicec/src/parsers/comments/grammar.rs Implements prefix-based whitespace sanitization.
Review details
  • Files reviewed: 1/1 changed files
  • Comments generated: 1
  • Review effort level: Balanced

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment on lines +106 to +109
common_leading_whitespace = Some(match common_leading_whitespace {
Some(current_whitespace) => get_common_prefix(current_whitespace, text),
None => get_leading_whitespace(text),
});

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I think this is minor and finicky enough to not warrant a test, if any other reviewers disagree, let me know and I will add a test using a mix of ASCII and non-ASCII whitespace at the start of doc-comment lines.

@externl externl left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Looks good!

@InsertCreativityHere
InsertCreativityHere merged commit a4a6610 into icerpc:main Sep 16, 2026
7 checks passed
InsertCreativityHere added a commit that referenced this pull request Sep 16, 2026
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.

Doc-Comment Indent Stripping Splits a UTF-8 Codepoint

4 participants