-
Notifications
You must be signed in to change notification settings - Fork 373
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
TemplateStringPart
abstraction and aTemplateStringParseHandler
interface. - Adds
FillConfig.templateStringParseHandler
with a default implementation. - Refactors
ExcelWriteFillExecutor
to 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
unconventionalFill
test 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
,templateStringParseHandler
may be null. Ensureinit()
is invoked on theFillConfig
instance prior to retrieving the handler.
currentTemplateStringParseHandler = fillConfig.getTemplateStringParseHandler();
stringValue = stringValue.replaceAll(ESCAPE_FILL_PREFIX, FILL_PREFIX); | ||
stringValue = stringValue.replaceAll(ESCAPE_FILL_SUFFIX, FILL_SUFFIX); |
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); |
Copilot uses AI. Check for mistakes.
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); |
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); |
Copilot uses AI. Check for mistakes.
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
readTemplateData
inExcelWriteFillExecutor
is called, theprepareData
method is invoked to obtain the prepared datapreparedData
(string). And thepreparedData
will 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).TemplateStringParseHandler
interface. My initial plan was to place this custom setting entry in the parameters of thewithTemplate
method ofExcelWriteBuilder
. However, eventually, following the principle of minimal modification, I set it in theFillConfig
configuration. 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.