Skip to content

Conversation

@alaahong
Copy link
Member

@alaahong alaahong commented Aug 7, 2025

Purpose of the pull request

  1. add SXSSFCell checking, skip other types
  2. recreate escapeHex method for performance tuning
image image image image image
  1. create relative unit test
image

Related: #364

What's changed?

fastexcel/src/main/java/cn/idev/excel/write/handler/EscapeHexCellWriteHandler.java
fastexcel-test/src/test/java/cn/idev/excel/test/demo/write/EscapeHexCellWriteHandlerTest.java

Checklist

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

1. add SXSSFCell checking, skip other cell
2. recreate escapeHex method for performance tuning
3. create relative unit test

close: #364
@psxjoy psxjoy requested a review from Copilot August 7, 2025 17:35
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR optimizes the EscapeHexCellWriteHandler to improve performance and fix type checking issues that affected XLS and CSV file formats. The changes focus on preventing POI from automatically decoding hex-encoded strings in x[0-9A-Fa-f]{4} format by escaping them appropriately.

  • Performance optimization through custom hex validation using lookup tables instead of regex
  • Type checking fix to only process SXSSFCell instances (XLSX format)
  • Comprehensive test coverage for various file formats and edge cases

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 6 comments.

File Description
EscapeHexCellWriteHandler.java Replaces regex-based hex processing with optimized manual parsing and adds SXSSFCell type checking
EscapeHexCellWriteHandlerTest.java Adds comprehensive unit tests covering multiple file formats, edge cases, and validation scenarios

@alaahong
Copy link
Member Author

alaahong commented Aug 8, 2025

As I understand, the expected result is not additional prefix "_x005F" and display String well, so no change on given test, resolve all.

Copy link
Member

@psxjoy psxjoy left a comment

Choose a reason for hiding this comment

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

LGTM

@psxjoy psxjoy merged commit b1cc842 into apache:main Aug 9, 2025
5 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.

2 participants