Visitar URL original
XmlHelper - Replace with char.IsHighSurrogate + char.IsLowSurrogate by snakefoot · Pull Request #6268 · NLog/NLog · GitHub
Skip to content

XmlHelper - Replace with char.IsHighSurrogate + char.IsLowSurrogate - #6268

Merged
snakefoot merged 1 commit into
NLog:devfrom
snakefoot:XmlConvertSurrogate
Sep 3, 2026
Merged

XmlHelper - Replace with char.IsHighSurrogate + char.IsLowSurrogate#6268
snakefoot merged 1 commit into
NLog:devfrom
snakefoot:XmlConvertSurrogate

Conversation

@snakefoot

@snakefoot snakefoot commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

No description provided.

@coderabbitai

coderabbitai Bot commented Sep 3, 2026 •
edited
Loading

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Team

Run ID: 11eb6465-e84a-4bfb-9a52-6a2d1e2270d0

📥 Commits

Reviewing files that changed from the base of the PR and between 8a481eb and 800a49e.

📒 Files selected for processing (1)
  • src/NLog/Internal/XmlHelper.cs

Included review availability: Your plan provides up to 8 included reviews per hour; 6 remain after this review.

Walkthrough

XmlHelper now uses built-in char APIs for surrogate checks. It removes duplicate public surrogate methods and unused range constants. Surrogate-pair validation keeps its existing public signature.

Changes

Surrogate classification cleanup

Layer / File(s) Summary
Update surrogate validation
src/NLog/Internal/XmlHelper.cs		 XmlHelper uses char.IsHighSurrogate and char.IsLowSurrogate. It removes duplicate public helpers and unused range constants. XmlConvertIsXmlSurrogatePair keeps its signature and validates arguments with the char APIs.

Estimated code review effort: 2 (Simple) | ~10 minutes

Merge Risk: ⚪ Minimal · up to 800a4

XML surrogate classification now uses the platform character APIs without changing surrogate-pair validation behavior. No current merge-blocking risk is identified.

Poem

A rabbit checks each surrogate pair,
With tidy char calls in the air.
Old helpers hop away,
Constants leave today,
And XmlHelper keeps the code fair.

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (1 warning, 1 inconclusive)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 3 functions across 1 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
Description check ❓ Inconclusive No pull request description was provided, so the relationship between the changes and their purpose cannot be confirmed from the description. Add a concise description that explains the surrogate-classification refactoring, the removed public methods, and any compatibility impact.
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly summarizes the main change: replacing custom surrogate classification methods with char.IsHighSurrogate and char.IsLowSurrogate.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
  • Fix all pre-merge checks with AI
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@snakefoot
snakefoot force-pushed the XmlConvertSurrogate branch 2 times, most recently from 865dbc9 to a593e6a Compare September 3, 2026 16:30
@sonarqubecloud

sonarqubecloud Bot commented Sep 3, 2026

Copy link
Copy Markdown

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
100.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

@snakefoot
snakefoot merged commit ad963a9 into NLog:dev Sep 3, 2026
6 checks passed
@snakefoot snakefoot added this to the 6.2.1 milestone Sep 6, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

refactoring size/S

Projects

None yet

Milestone

6.2.1

Development

Successfully merging this pull request may close these issues.

1 participant

@snakefoot