- Notifications
You must be signed in to change notification settings - Fork427
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 ourterms of service andprivacy 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?
Uh oh!
There was an error while loading.Please reload this page.
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 | AddedunconventionalFill example test demonstrating custom placeholders. |
| TemplateStringPart.java | New immutable value type representing segments of a template string. |
| FillConfig.java | ExposedtemplateStringParseHandler in configuration and set default ininit(). |
| 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 aroundTemplateStringPart. |
| 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); |
CopilotAIJul 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] UsingreplaceAll compiles the regex every call; consider usingString.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); |
CopilotAIJul 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.
Theorder field is never set when parts are created, so sorting byorder has no effect. Consider assigning a sequence value toorder in the parser or remove unused ordering logic.
| publicstaticTemplateStringParttext(Stringtext) { | |
| if (text ==null) { | |
| thrownewIllegalArgumentException("The text parameter cannot be empty when creating a text part"); | |
| } | |
| returnnewTemplateStringPart(TemplateStringPartType.TEXT,text | |
| ,null,null,null); | |
| } | |
| publicstaticTemplateStringPartcommonVariable(StringvariableName) { | |
| if (variableName ==null) { | |
| thrownewIllegalArgumentException("The variableName parameter cannot be null " + | |
| "when creating a variable part"); | |
| } | |
| returnnewTemplateStringPart(TemplateStringPartType.COMMON_VARIABLE,null | |
| ,variableName,null,null); | |
| } | |
| publicstaticTemplateStringPartcollectionVariable(StringcollectionName,StringvariableName) { | |
| if (variableName ==null) { | |
| thrownewIllegalArgumentException("The variableName parameter cannot be null " + | |
| "when creating a collection variable part"); | |
| } | |
| returnnewTemplateStringPart(TemplateStringPartType.COLLECTION_VARIABLE,null | |
| ,variableName,collectionName,null); | |
| } | |
| publicstaticTemplateStringPartbareCollectionVariable(StringvariableName) { | |
| returncollectionVariable(null,variableName); | |
| publicstaticTemplateStringParttext(Stringtext,Integerorder) { | |
| if (text ==null) { | |
| thrownewIllegalArgumentException("The text parameter cannot be empty when creating a text part"); | |
| } | |
| returnnewTemplateStringPart(TemplateStringPartType.TEXT,text | |
| ,null,null,order); | |
| } | |
| publicstaticTemplateStringPartcommonVariable(StringvariableName,Integerorder) { | |
| if (variableName ==null) { | |
| thrownewIllegalArgumentException("The variableName parameter cannot be null " + | |
| "when creating a variable part"); | |
| } | |
| returnnewTemplateStringPart(TemplateStringPartType.COMMON_VARIABLE,null | |
| ,variableName,null,order); | |
| } | |
| publicstaticTemplateStringPartcollectionVariable(StringcollectionName,StringvariableName,Integerorder) { | |
| if (variableName ==null) { | |
| thrownewIllegalArgumentException("The variableName parameter cannot be null " + | |
| "when creating a collection variable part"); | |
| } | |
| returnnewTemplateStringPart(TemplateStringPartType.COLLECTION_VARIABLE,null | |
| ,variableName,collectionName,order); | |
| } | |
| publicstaticTemplateStringPartbareCollectionVariable(StringvariableName,Integerorder) { | |
| returncollectionVariable(null,variableName,order); |
Copilot uses AI. Check for mistakes.
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.