Skip to content

Improve C# comments filtering#95

Open
Vladyslav-Kuksiuk wants to merge 5 commits into
improve-go-filteringfrom
improve-csharp-filtering
Open

Improve C# comments filtering#95
Vladyslav-Kuksiuk wants to merge 5 commits into
improve-go-filteringfrom
improve-csharp-filtering

Conversation

@Vladyslav-Kuksiuk

Copy link
Copy Markdown
Collaborator

This PR improves the C# comments filtering.

Resolves this issue.

@Vladyslav-Kuksiuk Vladyslav-Kuksiuk self-assigned this Jul 6, 2026
@Vladyslav-Kuksiuk Vladyslav-Kuksiuk marked this pull request as ready for review July 6, 2026 13:56
@Oleg-Melnik

Oleg-Melnik commented Jul 6, 2026

Copy link
Copy Markdown
Collaborator

Claude Review

Three non-blocking findings:

  1. C# 11 raw string literals (""" / $""") are unhandled. A multi-line """ block degrades into empty-string + open-string parsing, so //-like text inside a raw string gets stripped as a comment — silent content corruption in embedded snippets. Probably a follow-up issue rather than a blocker for this PR.
  2. /*/ self-closes. consumeComment doesn't advance past the /* opener, so consumeActiveBlock's strings.Index search finds the overlapping */ at offset 1 and treats /*/ as a complete comment, leaking the actual comment body as code. Inherited from the identical pattern in the JS and marker filters, so a shared fix would cover all three.
  3. Duplication. consumeActiveBlock, consumeCodeByte, writeEscapedByte, and the Filter loop skeleton are now byte-identical third copies of the JavaScript filter's versions. A shared line-scanner core would keep the three lexers from drifting.

@Oleg-Melnik Oleg-Melnik left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

@Vladyslav-Kuksiuk LGTM with Claude comments to be addressed before merging.

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.

3 participants