You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@fesod.apache.org by "32154678925 (via GitHub)" <gi...@apache.org> on 2026/01/18 06:48:18 UTC
[PR] fix: fix error in using relativeHeadRowIndex for writing tables only [fesod]
32154678925 opened a new pull request, #810:
URL: https://github.com/apache/fesod/pull/810
## Purpose of the pull request
Fixes(#794)
When you do not write directly in the `sheet`, but only write data in the `table `and set the `head`, the offset value set with `relativeHeadRowIndex()` will make an error, and the blank line with the specified offset will be empty again between the head and the data. If the `head `is not set, the offset value set with `relativeHeadRowIndex()` will not take effect
## What's changed?
Change the judgment of whether to add offset in the `add` method in the `ExcelWriteAddExecutor` class from `writeSheetHolder` to `currentWriteHolder`
**Before**:
`WriteSheetHolder writeSheetHolder = writeContext.writeSheetHolder();
int newRowIndex = writeSheetHolder.getNewRowIndexAndStartDoWrite();
if (writeSheetHolder.isNew()
&& !writeSheetHolder.getExcelWriteHeadProperty().hasHead()) {
newRowIndex += writeContext.currentWriteHolder().relativeHeadRowIndex();
}`
**After**:
`WriteSheetHolder writeSheetHolder = writeContext.writeSheetHolder();
WriteHolder currentWriteHolder = writeContext.currentWriteHolder();
int newRowIndex = writeSheetHolder.getNewRowIndexAndStartDoWrite();
if (currentWriteHolder.isNew()) {
if(currentWriteHolder instanceof AbstractWriteHolder){
AbstractWriteHolder writeHolder = (AbstractWriteHolder) currentWriteHolder;
if(!writeHolder.getExcelWriteHeadProperty().hasHead()){
newRowIndex += currentWriteHolder.relativeHeadRowIndex();
}
}
}`
## Checklist
- [x] I have read the [Contributor Guide](https://fesod.apache.org/community/contribution/).
- [ ] I have written the necessary doc or comment.
- [x] I have added the necessary unit tests and all cases have passed.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@fesod.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@fesod.apache.org
For additional commands, e-mail: commits-help@fesod.apache.org
Re: [PR] fix: fix error in using relativeHeadRowIndex for writing tables only [fesod]
Posted by "bengbengbalabalabeng (via GitHub)" <gi...@apache.org>.
bengbengbalabalabeng commented on code in PR #810:
URL: https://github.com/apache/fesod/pull/810#discussion_r3464135054
##########
fesod-sheet/src/test/java/org/apache/fesod/sheet/writesheet/WriteTableTest.java:
##########
@@ -0,0 +1,138 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.fesod.sheet.writesheet;
+
+import java.io.File;
+import java.util.*;
Review Comment:
Avoid using `import *` (34 lines have the same issue)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@fesod.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@fesod.apache.org
For additional commands, e-mail: commits-help@fesod.apache.org
Re: [PR] fix: fix error in using relativeHeadRowIndex for writing tables only [fesod]
Posted by "bengbengbalabalabeng (via GitHub)" <gi...@apache.org>.
bengbengbalabalabeng commented on code in PR #810:
URL: https://github.com/apache/fesod/pull/810#discussion_r3412915862
##########
fesod-sheet/src/test/java/org/apache/fesod/sheet/writesheet/WriteTableTest.java:
##########
@@ -0,0 +1,247 @@
+package org.apache.fesod.sheet.writesheet;
Review Comment:
Please supplement the copyright header. [license-header.txt](https://github.com/apache/fesod/blob/main/tools/spotless/license-header.txt)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@fesod.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@fesod.apache.org
For additional commands, e-mail: commits-help@fesod.apache.org
Re: [PR] fix: fix error in using relativeHeadRowIndex for writing tables only [fesod]
Posted by "32154678925 (via GitHub)" <gi...@apache.org>.
32154678925 commented on code in PR #810:
URL: https://github.com/apache/fesod/pull/810#discussion_r3464469377
##########
fesod-sheet/src/test/java/org/apache/fesod/sheet/writesheet/WriteTableTest.java:
##########
@@ -0,0 +1,138 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.fesod.sheet.writesheet;
+
+import java.io.File;
+import java.util.*;
Review Comment:
> Avoid using `import *` (34 lines have the same issue)
我已经修改了引入规则，还有其他需要注意的么？或者本项目有什么规则之类的说明？或许我可以去看看。
I have modified the import rules. Is there anything else I should pay attention to? Or what are the rules of this project? Maybe I can go and have a look.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@fesod.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@fesod.apache.org
For additional commands, e-mail: commits-help@fesod.apache.org
Re: [PR] fix: fix error in using relativeHeadRowIndex for writing tables only [fesod]
Posted by "psxjoy (via GitHub)" <gi...@apache.org>.
psxjoy merged PR #810:
URL: https://github.com/apache/fesod/pull/810
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@fesod.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@fesod.apache.org
For additional commands, e-mail: commits-help@fesod.apache.org
Re: [PR] fix: fix error in using relativeHeadRowIndex for writing tables only [fesod]
Posted by "32154678925 (via GitHub)" <gi...@apache.org>.
32154678925 commented on PR #810:
URL: https://github.com/apache/fesod/pull/810#issuecomment-4641786557
> 这些测试不包含断言，因此不验证修复。请用检查实际输出的断言来增强它们。（同时确保涵盖XLSX和XLS格式）
谢谢，我修改了测试代码，并且成功跑通测试，且测试了未修改bug的代码，测试是跑不同的。我的代码经验不多，还有什么需要修改的么？
Thank you. I modified the test code and successfully ran through the test. I also tested the code without modifying the bug. The test runs differently. I don't have much code experience. Is there anything I need to change?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@fesod.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@fesod.apache.org
For additional commands, e-mail: commits-help@fesod.apache.org
Re: [PR] fix: fix error in using relativeHeadRowIndex for writing tables only [fesod]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #810:
URL: https://github.com/apache/fesod/pull/810#discussion_r2835922354
##########
fesod-sheet/src/main/java/org/apache/fesod/sheet/write/executor/ExcelWriteAddExecutor.java:
##########
@@ -63,10 +64,15 @@ public void add(Collection<?> data) {
data = new ArrayList<>();
}
WriteSheetHolder writeSheetHolder = writeContext.writeSheetHolder();
+ WriteHolder currentWriteHolder = writeContext.currentWriteHolder();
int newRowIndex = writeSheetHolder.getNewRowIndexAndStartDoWrite();
- if (writeSheetHolder.isNew()
- && !writeSheetHolder.getExcelWriteHeadProperty().hasHead()) {
- newRowIndex += writeContext.currentWriteHolder().relativeHeadRowIndex();
+ if (currentWriteHolder.isNew()) {
+ if (currentWriteHolder instanceof AbstractWriteHolder) {
+ AbstractWriteHolder writeHolder = (AbstractWriteHolder) currentWriteHolder;
+ if (!writeHolder.getExcelWriteHeadProperty().hasHead()) {
+ newRowIndex += currentWriteHolder.relativeHeadRowIndex();
+ }
Review Comment:
The instanceof check for AbstractWriteHolder is unnecessary. All implementations of WriteHolder (WriteSheetHolder, WriteTableHolder, WriteWorkbookHolder) extend AbstractWriteHolder. The rest of the codebase directly casts currentWriteHolder() to AbstractWriteHolder without instanceof checks (see WriteHandlerUtils.java lines 79, 124, 160, 168, 176, 184). For consistency with the existing codebase patterns, the cast should be done directly without the instanceof check.
```suggestion
AbstractWriteHolder writeHolder = (AbstractWriteHolder) currentWriteHolder;
if (!writeHolder.getExcelWriteHeadProperty().hasHead()) {
newRowIndex += currentWriteHolder.relativeHeadRowIndex();
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@fesod.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@fesod.apache.org
For additional commands, e-mail: commits-help@fesod.apache.org
Re: [PR] fix: fix error in using relativeHeadRowIndex for writing tables only [fesod]
Posted by "bengbengbalabalabeng (via GitHub)" <gi...@apache.org>.
bengbengbalabalabeng commented on PR #810:
URL: https://github.com/apache/fesod/pull/810#issuecomment-4508158131
These tests contain no assertions and therefore do not validate the fix. Please enhance them with assertions that check the actual output. (Also ensure that both XLSX and XLS formats are covered)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@fesod.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@fesod.apache.org
For additional commands, e-mail: commits-help@fesod.apache.org
Re: [PR] fix: fix error in using relativeHeadRowIndex for writing tables only [fesod]
Posted by "32154678925 (via GitHub)" <gi...@apache.org>.
32154678925 commented on PR #810:
URL: https://github.com/apache/fesod/pull/810#issuecomment-4780315025
> Suggest covering multi-sheet table writing scenarios.
我添加了版权声明，移除了随机数，添加了多组偏移量，它们现在能够正常工作，还有什么需要修改的么？
I added a copyright notice, removed random numbers, and added multiple sets of offsets. They now work properly. Is there anything else I need to modify?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@fesod.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@fesod.apache.org
For additional commands, e-mail: commits-help@fesod.apache.org
Re: [PR] fix: fix error in using relativeHeadRowIndex for writing tables only [fesod]
Posted by "bengbengbalabalabeng (via GitHub)" <gi...@apache.org>.
bengbengbalabalabeng commented on PR #810:
URL: https://github.com/apache/fesod/pull/810#issuecomment-4707201397
Suggest covering multi-sheet table writing scenarios.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@fesod.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@fesod.apache.org
For additional commands, e-mail: commits-help@fesod.apache.org
Re: [PR] fix: fix error in using relativeHeadRowIndex for writing tables only [fesod]
Posted by "bengbengbalabalabeng (via GitHub)" <gi...@apache.org>.
bengbengbalabalabeng commented on code in PR #810:
URL: https://github.com/apache/fesod/pull/810#discussion_r3412919675
##########
fesod-sheet/src/test/java/org/apache/fesod/sheet/writesheet/WriteTableTest.java:
##########
@@ -0,0 +1,247 @@
+package org.apache.fesod.sheet.writesheet;
+
+import java.io.File;
+import java.lang.reflect.Field;
+import java.util.*;
+import org.apache.fesod.sheet.ExcelWriter;
+import org.apache.fesod.sheet.FesodSheet;
+import org.apache.fesod.sheet.annotation.ExcelProperty;
+import org.apache.fesod.sheet.support.ExcelTypeEnum;
+import org.apache.fesod.sheet.util.TestFileUtil;
+import org.apache.fesod.sheet.write.builder.ExcelWriterBuilder;
+import org.apache.fesod.sheet.write.builder.ExcelWriterSheetBuilder;
+import org.apache.fesod.sheet.write.metadata.WriteSheet;
+import org.apache.fesod.sheet.write.metadata.WriteTable;
+import org.apache.poi.ss.usermodel.*;
+import org.junit.jupiter.api.Assertions;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.EnumSource;
+
+public class WriteTableTest {
+
+ private static final String HEADER_STRING;
+
+ static {
+ try {
+ Field field = WriteSheetData.class.getDeclaredField("string");
+ ExcelProperty annotation = field.getAnnotation(ExcelProperty.class);
+ HEADER_STRING = annotation.value()[0];
+ } catch (Exception e) {
+ throw new RuntimeException("Failed to resolve @ExcelProperty header from WriteSheetData.string", e);
+ }
+ }
+
+ @ParameterizedTest
+ @EnumSource(
+ value = ExcelTypeEnum.class,
+ names = {"XLS", "XLSX"})
+ public void testWriteTableWithTitle(ExcelTypeEnum excelType) throws Exception {
+ Random random = new Random();
Review Comment:
Random numbers may lead to non-deterministic, it is recommended to use @MethodSource to provide N sets of representative boundary values for testing.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@fesod.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@fesod.apache.org
For additional commands, e-mail: commits-help@fesod.apache.org
Re: [PR] fix: fix error in using relativeHeadRowIndex for writing tables only [fesod]
Posted by "bengbengbalabalabeng (via GitHub)" <gi...@apache.org>.
bengbengbalabalabeng commented on code in PR #810:
URL: https://github.com/apache/fesod/pull/810#discussion_r3473439029
##########
fesod-sheet/src/test/java/org/apache/fesod/sheet/writesheet/WriteTableTest.java:
##########
@@ -0,0 +1,138 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.fesod.sheet.writesheet;
+
+import java.io.File;
+import java.util.*;
Review Comment:
The unit tests for the `fesod-sheet` module have recently undergone systematic refactoring, and the original `TestFileUtil` has been completely removed. It is recommended to refer to the following latest test case examples:
`org.apache.fesod.sheet.sheet.WriteSheetTest`
`org.apache.fesod.sheet.sheet.HiddenSheetsTest`
......
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@fesod.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@fesod.apache.org
For additional commands, e-mail: commits-help@fesod.apache.org
Re: [PR] fix: fix error in using relativeHeadRowIndex for writing tables only [fesod]
Posted by "32154678925 (via GitHub)" <gi...@apache.org>.
32154678925 commented on code in PR #810:
URL: https://github.com/apache/fesod/pull/810#discussion_r3480986092
##########
fesod-sheet/src/test/java/org/apache/fesod/sheet/writesheet/WriteTableTest.java:
##########
@@ -0,0 +1,138 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.fesod.sheet.writesheet;
+
+import java.io.File;
+import java.util.*;
Review Comment:
I remove import TestFileUtil
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@fesod.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@fesod.apache.org
For additional commands, e-mail: commits-help@fesod.apache.org
Re: [PR] fix: fix error in using relativeHeadRowIndex for writing tables only [fesod]
Posted by "bengbengbalabalabeng (via GitHub)" <gi...@apache.org>.
bengbengbalabalabeng commented on code in PR #810:
URL: https://github.com/apache/fesod/pull/810#discussion_r3464135054
##########
fesod-sheet/src/test/java/org/apache/fesod/sheet/writesheet/WriteTableTest.java:
##########
@@ -0,0 +1,138 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.fesod.sheet.writesheet;
+
+import java.io.File;
+import java.util.*;
Review Comment:
Avoid using `import *`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@fesod.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@fesod.apache.org
For additional commands, e-mail: commits-help@fesod.apache.org