Skip to content

refactor: simplify hex escape pattern scanning - #1003

Merged
delei merged 8 commits into
apache:mainfrom
nkuprins:refactor/cleanup-escape-hex-write-handler
Aug 16, 2026
Merged

refactor: simplify hex escape pattern scanning#1003
delei merged 8 commits into
apache:mainfrom
nkuprins:refactor/cleanup-escape-hex-write-handler

Conversation

@nkuprins

@nkuprins nkuprins commented Aug 10, 2026

Copy link
Copy Markdown
Contributor

Related: #1000

Purpose of the pull request

The second of the two PRs. This one does the refactor/cleanup. No behaviour change.

What's changed?

  • The match loop is written once instead of twice. The scan used to live in two methods: escapeHex located the
    first valid pattern, then processWithPatterns located every later one. Both carried the same match-handling code - bounds check, closing-_ test, hex validation, append, so any change to the rule had to be made in two places. It is now a single loop.
  • The no-match path allocates nothing. The StringBuilder is created on the first match rather than up front, so a cell containing no _xHHHH_ - costs one indexOf scan and no allocation.
  • The offsets have names. Each iteration derives hexStart, suffixIndex and patternEnd from PREFIX_LENGTH and HEX_DIGIT_COUNT.
  • isValidHexFast is now isHexDigits, and the lookup table has a note on why it is not Character.digit(c, 16) - that method also accepts non-ASCII digits such as U+0663, which OOXML never encodes.

Checklist

  • I have read the Contributor Guide.
  • I have written the necessary doc or comment.
  • I have added the necessary unit tests and all cases have passed.

@nkuprins nkuprins changed the title refactor: cleanup escape hex write handler refactor: simplify hex escape pattern scanning Aug 10, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Refactors EscapeHexCellWriteHandler’s _xHHHH_ pattern scanning to remove duplicated logic while preserving existing behavior, improving readability and avoiding allocations on the no-match path (as described in #1000’s refactor/cleanup step).

Changes:

  • Consolidates pattern detection and replacement into a single scanning loop (removing the prior two-method split).
  • Lazily allocates the StringBuilder only after the first valid match is found.
  • Introduces named offsets/constants (PREFIX_LENGTH, HEX_DIGIT_COUNT) and renames hex validation helper to isHexDigits.

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

nkuprins and others added 2 commits August 16, 2026 09:26
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

@delei delei 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.

LGTM

@delei
delei merged commit 0add567 into apache:main Aug 16, 2026
9 checks passed
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.

4 participants