Skip to content

feat: registered EscapeHexCellWriteHandler in DefaultWriteHandlerLoader for XLSX - #1044

Open
delei wants to merge 8 commits into
apache:mainfrom
delei:default-handler-path1
Open

feat: registered EscapeHexCellWriteHandler in DefaultWriteHandlerLoader for XLSX#1044
delei wants to merge 8 commits into
apache:mainfrom
delei:default-handler-path1

Conversation

@delei

@delei delei commented Aug 26, 2026

Copy link
Copy Markdown
Member

Purpose of the pull request

Closed: #850

What's changed?

  • Moved EscapeHexCellWriteHandler to org.apache.fesod.sheet.write.handler.impl package
  • Registered EscapeHexCellWriteHandler in DefaultWriteHandlerLoader for XLSX format.

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.

delei added 4 commits August 26, 2026 13:19
- Moved EscapeHexCellWriteHandler to handler.impl package
- Registered EscapeHexCellWriteHandler in DefaultWriteHandlerLoader for XLSX format
- Updated XlsxEscapeUtils documentation to link to EscapeHexCellWriteHandler
- Improved handling of XLSX character escapes in cell writing process
- Add EscapeHexCellWriteHandlerRoundTripTest to verify hex escapes on write are read back literally
- Remove redundant HexEscapeRoundTripTest which tested similar functionality
- Import EscapeHexCellWriteHandler where necessary in write handler tests
- Ensure round trip correctness for cells containing hexadecimal escape patterns in data fields
@delei delei changed the title refactor: registered EscapeHexCellWriteHandler in DefaultWriteHandlerLoader for XLSX feat: registered EscapeHexCellWriteHandler in DefaultWriteHandlerLoader for XLSX Aug 26, 2026
@nkuprins

Copy link
Copy Markdown
Contributor

You should also probably update the comment in EscapeHexCellWriteHandler on 39-43: "This handler is not registered by default..."

@nkuprins

Copy link
Copy Markdown
Contributor

Not sure if you considered this already, but the handler isn't idempotent, so this PR affects the users who already register the handler themselves. Therefore, maybe it'd be good to make sure the handler can only take effect once per value.

@delei

delei commented Aug 26, 2026

Copy link
Copy Markdown
Member Author

Not sure if you considered this already, but the handler isn't idempotent, so this PR affects the users who already register the handler themselves. Therefore, maybe it'd be good to make sure the handler can only take effect once per value.

A very good point. Thank you for pointing out the issue.

I have corrected this. This handler really should ensure that you only execute it once.

Set<String> alreadyExistedHandlerSet = new HashSet<>();
List<WriteHandler> cleanUpHandlerList = new ArrayList<>();
for (Map.Entry<Integer, List<WriteHandler>> entry : orderExcelWriteHandlerMap.entrySet()) {
for (WriteHandler handler : entry.getValue()) {
if (handler instanceof NotRepeatExecutor) {
String uniqueValue = ((NotRepeatExecutor) handler).uniqueValue();
if (alreadyExistedHandlerSet.contains(uniqueValue)) {
continue;
}
alreadyExistedHandlerSet.add(uniqueValue);
}
cleanUpHandlerList.add(handler);
}
}

@delei
delei requested review from alaahong and psxjoy August 26, 2026 13:43
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.

[Bug] 兼容类似于:_x005F_x 数据

2 participants