-
Notifications
You must be signed in to change notification settings - Fork 441
feat: Add support for custom parsing placeholder syntax for template filling #398
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this 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 adds a flexible placeholder parsing layer to the existing template fill feature, allowing users to provide custom parsing rules without altering core logic.
- Introduces
TemplateStringPartabstraction and aTemplateStringParseHandlerinterface. - Adds
FillConfig.templateStringParseHandlerwith a default implementation. - Refactors
ExcelWriteFillExecutorto delegate placeholder parsing and rebuilds the core fill loop around parsed parts.
Reviewed Changes
Copilot reviewed 7 out of 8 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| FillTest.java | Added unconventionalFill example test demonstrating custom placeholders. |
| TemplateStringPart.java | New immutable value type representing segments of a template string. |
| FillConfig.java | Exposed templateStringParseHandler in configuration and set default in init(). |
| DefaultTemplateStringParseHandler.java | Provided default parsing logic for {} and escaped braces. |
| TemplateStringParseHandler.java | Defined interface for pluggable parsing strategy. |
| ExcelWriteFillExecutor.java | Removed inline parsing, wired in handler, and rebuilt fill logic around TemplateStringPart. |
| TemplateStringPartType.java | Enum for part kinds: TEXT, COMMON_VARIABLE, COLLECTION_VARIABLE. |
Comments suppressed due to low confidence (3)
fastexcel-test/src/test/java/cn/idev/excel/test/demo/fill/FillTest.java:274
- The
unconventionalFilltest writes output but does not verify any cell values or file contents. Consider adding assertions that read back the generated sheets and confirm that placeholders were replaced correctly.
excelWriter.fill(getListMap.apply(10), EasyExcel.writerSheet("list").build());
fastexcel-core/src/main/java/cn/idev/excel/write/executor/ExcelWriteFillExecutor.java:495
- Missing import for
CollectionUtils, which will cause a compilation error. Please add the appropriate import (e.g.,import cn.idev.excel.util.CollectionUtils;).
if (CollectionUtils.isEmpty(templateStringParts)) {
fastexcel-core/src/main/java/cn/idev/excel/write/executor/ExcelWriteFillExecutor.java:117
- If
fillConfig.init()has not been called beforefill,templateStringParseHandlermay be null. Ensureinit()is invoked on theFillConfiginstance prior to retrieving the handler.
currentTemplateStringParseHandler = fillConfig.getTemplateStringParseHandler();
| stringValue = stringValue.replaceAll(ESCAPE_FILL_PREFIX, FILL_PREFIX); | ||
| stringValue = stringValue.replaceAll(ESCAPE_FILL_SUFFIX, FILL_SUFFIX); |
Copilot
AI
Jul 1, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nitpick] Using replaceAll compiles the regex every call; consider using String.replace for literal replacements to improve performance.
| stringValue = stringValue.replaceAll(ESCAPE_FILL_PREFIX, FILL_PREFIX); | |
| stringValue = stringValue.replaceAll(ESCAPE_FILL_SUFFIX, FILL_SUFFIX); | |
| stringValue = stringValue.replace(ESCAPE_FILL_PREFIX, FILL_PREFIX); | |
| stringValue = stringValue.replace(ESCAPE_FILL_SUFFIX, FILL_SUFFIX); |
| public static TemplateStringPart text(String text) { | ||
| if (text == null) { | ||
| throw new IllegalArgumentException("The text parameter cannot be empty when creating a text part"); | ||
| } | ||
| return new TemplateStringPart(TemplateStringPartType.TEXT, text | ||
| , null, null, null); | ||
| } | ||
|
|
||
| public static TemplateStringPart commonVariable(String variableName) { | ||
| if (variableName == null) { | ||
| throw new IllegalArgumentException("The variableName parameter cannot be null " + | ||
| "when creating a variable part"); | ||
| } | ||
| return new TemplateStringPart(TemplateStringPartType.COMMON_VARIABLE, null | ||
| , variableName, null, null); | ||
| } | ||
|
|
||
| public static TemplateStringPart collectionVariable(String collectionName, String variableName) { | ||
| if (variableName == null) { | ||
| throw new IllegalArgumentException("The variableName parameter cannot be null " + | ||
| "when creating a collection variable part"); | ||
| } | ||
| return new TemplateStringPart(TemplateStringPartType.COLLECTION_VARIABLE, null | ||
| , variableName, collectionName, null); | ||
| } | ||
|
|
||
| public static TemplateStringPart bareCollectionVariable(String variableName) { | ||
| return collectionVariable(null, variableName); |
Copilot
AI
Jul 1, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The order field is never set when parts are created, so sorting by order has no effect. Consider assigning a sequence value to order in the parser or remove unused ordering logic.
| public static TemplateStringPart text(String text) { | |
| if (text == null) { | |
| throw new IllegalArgumentException("The text parameter cannot be empty when creating a text part"); | |
| } | |
| return new TemplateStringPart(TemplateStringPartType.TEXT, text | |
| , null, null, null); | |
| } | |
| public static TemplateStringPart commonVariable(String variableName) { | |
| if (variableName == null) { | |
| throw new IllegalArgumentException("The variableName parameter cannot be null " + | |
| "when creating a variable part"); | |
| } | |
| return new TemplateStringPart(TemplateStringPartType.COMMON_VARIABLE, null | |
| , variableName, null, null); | |
| } | |
| public static TemplateStringPart collectionVariable(String collectionName, String variableName) { | |
| if (variableName == null) { | |
| throw new IllegalArgumentException("The variableName parameter cannot be null " + | |
| "when creating a collection variable part"); | |
| } | |
| return new TemplateStringPart(TemplateStringPartType.COLLECTION_VARIABLE, null | |
| , variableName, collectionName, null); | |
| } | |
| public static TemplateStringPart bareCollectionVariable(String variableName) { | |
| return collectionVariable(null, variableName); | |
| public static TemplateStringPart text(String text, Integer order) { | |
| if (text == null) { | |
| throw new IllegalArgumentException("The text parameter cannot be empty when creating a text part"); | |
| } | |
| return new TemplateStringPart(TemplateStringPartType.TEXT, text | |
| , null, null, order); | |
| } | |
| public static TemplateStringPart commonVariable(String variableName, Integer order) { | |
| if (variableName == null) { | |
| throw new IllegalArgumentException("The variableName parameter cannot be null " + | |
| "when creating a variable part"); | |
| } | |
| return new TemplateStringPart(TemplateStringPartType.COMMON_VARIABLE, null | |
| , variableName, null, order); | |
| } | |
| public static TemplateStringPart collectionVariable(String collectionName, String variableName, Integer order) { | |
| if (variableName == null) { | |
| throw new IllegalArgumentException("The variableName parameter cannot be null " + | |
| "when creating a collection variable part"); | |
| } | |
| return new TemplateStringPart(TemplateStringPartType.COLLECTION_VARIABLE, null | |
| , variableName, collectionName, order); | |
| } | |
| public static TemplateStringPart bareCollectionVariable(String variableName, Integer order) { | |
| return collectionVariable(null, variableName, order); |
|
Hi, @Hui-FatGod We should resolve conflicts first, and please modify the commit message to better comply with the commit guidelines. |
After I submitted this PR, the fastexcel main branch went through some directory and code style refactoring. To keep things clean, I’ve re-added the original enhancement for custom parser template filling to the latest version of the main branch. Right now, there are no conflicts. |
…ing string placeholder syntax
|
Thanks for your contribution. Can you try to merge latest code then resolve the conflicts? And meanwhile, can you try to add strong unit test with assertions? The sample file ( unconventional.xlsx ) is enough and good for verification, but I can not see the behavior since change executed from ut. |
Background
Fastexcel is a very excellent excel operation tool java library. One of the advanced features included, template filling, is also stable and trustworthy. However, in the actual application of some migration scenarios, the template filling function of fastexcel may appear inadequate. This limitation is mainly due to the fixed template placeholder rules of fastexcel, which cannot adapt to the template placeholder syntax when migrating from other libraries (which may be some good commercial libraries like Aspose, or internal enterprise/organization framework libraries, etc.). I think this flaw can be fatal, especially in the migration scenario I just described. For example, the underlying Excel operation of a large business system is migrated from Aspose to Fastexcel. The original system contains a large number of (the order of magnitude may reach thousands, even tens of thousands) of original templates. At this time, it is obviously unrealistic to change the placeholder syntax of all the templates, and the cost is too high.
Implementation
To address the aforementioned issues, I made some modifications to the fill in the write package. My approach was to keep changes to the original code logic minimal. Specifically, I introduced an abstraction layer between the template cell string value and
AnalysisCell, represented asCollection<TemplateStringPart>. In this implementation, simple string values are first parsed into mutil parts(currently categorized into three types: plain text type, normal variable type, and collection variable type). This parsing behavior can be customized by fastexcel users. The interface for this functionality is named
TemplateStringParseHandler. Additionally, I encapsulated fastexcel's original parsing logic within aDefaultTemplateStringParseHandler.notes
Other than that, I've made a slight logical change to the
'{}'placeholder syntax parsing. I made this change as the second commit in this PR, mainly to solve the original fastexcel parsing'{}'placeholder syntax,'{foo\}'or'}foo{'this situation will throw an index out of bounds exception, even though this syntax will not appear in the actual application, but I still think it should be considered.questions
readTemplateDatainExcelWriteFillExecutoris called, theprepareDatamethod is invoked to obtain the prepared datapreparedData(string). And thepreparedDatawill be filled into the cell first. But it seems that this' preparedData 'does not contain the last string of theAnalysisCell. What is the specific reason for this? To adapt to this phenomenon, when preparing this data, I also discarded the last textTemplateStringPart(multiple text parts will be merged).TemplateStringParseHandlerinterface. My initial plan was to place this custom setting entry in the parameters of thewithTemplatemethod ofExcelWriteBuilder. However, eventually, following the principle of minimal modification, I set it in theFillConfigconfiguration. When comparing these two schemes, I think both are acceptable. After all, in the vast majority of cases, a template will definitely have only one grammar. If there is a better solution, please ignore this question.