Repository navigation
Pre-Flatten Comment Messages in the Parser Rules - #828
InsertCreativityHere merged 3 commits into
Conversation
InsertCreativityHere
added
easy
| } | ||
| } | ||
|
|
||
| Message = MessageComponent+; |
There was a problem hiding this comment.
At a grammar level, these two constructs should be equivalent:
(MessageComponent+)? (match 1 more components, but also accept 0 components)
MessageComponent* (match 0 or more components)
But in addition to being a simpler rule, it also simplifies the types.
The 1st expression gives Option<Vec<...>>, whereas the 2nd gives just Vec<...>.
There was a problem hiding this comment.
🟡 Changes recommended
Blank lines can panic, and empty inline messages now produce different AST output.
Get a fresh assessment by requesting another Copilot review.
Pull request overview
Simplifies doc-comment parsing by making Message directly nullable and removing nested option flattening.
Changes:
- Uses empty component vectors for blank message lines.
- Updates message construction and whitespace sanitization accordingly.
File summaries
| File | Description |
|---|---|
grammar.rs |
Adapts message sanitization to flattened lines. |
grammar.lalrpop |
Simplifies nullable message grammar rules. |
Review details
- Files reviewed: 2/2 changed files
- Comments generated: 3
- Review effort level: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
There was a problem hiding this comment.
🟢 Approval recommended
The refactor preserves existing parsing behavior; only minor stale comment wording remains.
Review details
Suppressed comments (1)
slicec/src/parsers/comments/grammar.rs:125
- The comment implies newline insertion only happens when text is present, while empty lines also receive a newline. Reword it to describe both operations accurately.
if let Some(MessageComponent::Text(text)) = line.first_mut() {
text.replace_range(..common_leading_whitespace, "");
}
line.push(MessageComponent::Text("\n".to_owned()));
- Files reviewed: 2/2 changed files
- Comments generated: 1
- Review effort level: Balanced
| // To check the start of the line, we check the first message component. | ||
| // It's safe to unwrap because we know the line was non-empty in this block. | ||
| match message.first().unwrap() { | ||
| match message { |
(cherry picked from commit b6e4900)
This PR makes a slight simplification to the grammar rules used to parse doc-comment messages, which I noticed while working on #800.
What's Changed
None - This PR is just an internal simplification of the grammar rules used to parse doc-comments with no side-effects.