设置图片嵌入到单元格 - #994
Conversation
📝 WalkthroughWalkthroughThe PR adds ChangesIn-cell image embedding
Sequence Diagram(s)sequenceDiagram
participant OpenXmlWriter
participant OpenXmlPictureImplement
participant OpenXmlPlaceInCellImplement
participant ExcelPackage
OpenXmlWriter->>OpenXmlPictureImplement: collect and partition image records
OpenXmlPictureImplement->>OpenXmlPlaceInCellImplement: add PlaceInCell images
OpenXmlPlaceInCellImplement->>ExcelPackage: write media and rich-data XML
ExcelPackage-->>OpenXmlWriter: updated worksheet package
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (6)
src/MiniExcel.OpenXml/Picture/OpenXmlPlaceInCellImplement.cs (1)
507-573: 🗄️ Data Integrity & Integration | 🔵 Trivial | 💤 Low valueConsider updating the row
spansattribute.
UpsertImageCellcan add a cell to an existing row at a column outside the row's currentspansrange. The code does not updatespans. Excel treatsspansas a hint and recomputes it, so files still open. Some stricter OOXML consumers and validators report an inconsistency.Either widen
spanswhen you insert a cell, or remove the attribute from rows you modify.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/MiniExcel.OpenXml/Picture/OpenXmlPlaceInCellImplement.cs` around lines 507 - 573, Update UpsertImageCell and its row insertion flow so a modified row’s spans attribute remains consistent with the inserted cell’s column; either widen the existing spans range to include the new column or remove spans from the affected row. Preserve the current cell creation and update behavior.src/MiniExcel.OpenXml/Picture/OpenXmlPictureImplement.cs (1)
25-34: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRemove the unused
OpenXmlReaderinstance.Line 31 creates a reader, but the code only calls the static method
OpenXmlReader.GetWorkbookRelsAsyncwithexcelArchive.EntryCollection. The instance is never used. It also introduces ambiguous ownership ofexcelArchive, because both the reader anddisposableExcelArchivecan dispose the same archive.If the reader instance is not required, drop it and keep only the static call.
♻️ Proposed cleanup
var excelArchive = await OpenXmlZip.CreateAsync(excelStream, leaveOpen: true, cancellationToken: cancellationToken) .ConfigureAwait(false); await using var disposableExcelArchive = excelArchive.ConfigureAwait(false); - using var reader = OpenXmlReader.CreateFromArchive(excelArchive, null); var rels = await OpenXmlReader.GetWorkbookRelsAsync(excelArchive.EntryCollection, cancellationToken).ConfigureAwait(false); sheetEntries = rels?.ToList() ?? [];🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/MiniExcel.OpenXml/Picture/OpenXmlPictureImplement.cs` around lines 25 - 34, Remove the unused OpenXmlReader instance created in the sheet-list initialization block, leaving OpenXmlReader.GetWorkbookRelsAsync as the only reader-related operation and retaining disposableExcelArchive as the archive owner.src/MiniExcel.OpenXml/Writer/OpenXmlWriter.cs (1)
90-111: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider reusing
AddFilesToZipAsyncwith a filter.This loop duplicates the entry-writing logic of
AddFilesToZipAsync. The only difference is the!f.IsImagepredicate. Two copies of the same write path can drift, for example if compression level or cancellation handling changes.Add an optional predicate parameter to
AddFilesToZipAsyncand call it from both branches.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/MiniExcel.OpenXml/Writer/OpenXmlWriter.cs` around lines 90 - 111, Update AddFilesToZipAsync to accept an optional file predicate and apply it while selecting _files, preserving the existing behavior when no predicate is provided. Replace the inline non-image entry-writing loop in the EmbedImagesInCell branch of OpenXmlWriter with AddFilesToZipAsync using !f.IsImage, while keeping the shared-string, workbook, content-type, and image generation calls unchanged.tests/MiniExcel.Tests.Common/Localization.Designer.cs (1)
3-7: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winVerify this locale change is intentional; it is unrelated to the PlaceInCell feature.
This file only translates auto-generated XML-doc comments into Chinese. This file has
GeneratedCodeAttributemarking it as produced byStronglyTypedResourceBuilderfrom a.resxfile. Comments here usually change only when someone regenerates the designer file using a tool running under a different UI culture. This diff has no connection to the PlaceInCell embedding feature in this PR.Confirm this is an intended, tracked localization effort. If it is an accidental side effect of the local toolchain, revert it to keep the file in English, consistent with the rest of the codebase, and avoid repeated unrelated diffs on future regenerations.
Also applies to: 16-21, 36-36, 50-51, 64-64, 73-73, 82-82, 91-91
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/MiniExcel.Tests.Common/Localization.Designer.cs` around lines 3 - 7, Revert the unrelated Chinese localization changes in the generated Localization.Designer.cs comments, restoring the English tool-generated text used elsewhere in the codebase; leave the generated resource code and PlaceInCell changes untouched.tests/MiniExcel.OpenXml.Tests/MiniExcelPlaceInCellTests.cs (1)
195-249: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd a negative-path test for
EmbedImagesInCellprerequisites.Both tests here set
FastMode = trueandEnableConvertByteArray = true.OpenXmlConfiguration.EmbedImagesInCellis documented as requiringEnableConvertByteArray = trueandFastMode = true. No test exercisesEmbedImagesInCell = truecombined withFastMode = falseorEnableConvertByteArray = false. Add a test asserting the documented behavior (exception or fallback) when these prerequisites are not met, so the contract stays enforced as the writer implementation evolves.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/MiniExcel.OpenXml.Tests/MiniExcelPlaceInCellTests.cs` around lines 195 - 249, Add a negative-path test near EmbedImagesInCell_ExportConfig_WritesPlaceInCellNotDrawing that sets EmbedImagesInCell = true while disabling FastMode or EnableConvertByteArray. Assert the documented exception or fallback behavior, using the existing _excelExporter.Export flow and OpenXmlConfiguration setup, and cover both missing-prerequisite combinations if the contract distinguishes them.README.md (1)
1503-1514: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDocument the
EmbedImagesInCellconfiguration option in all README variants.Each README documents only the manual
AddPicture(ImgType = XlsxImgType.PlaceInCell)path. None of them documentsOpenXmlConfiguration.EmbedImagesInCell, which is the config-driven approach highlighted in the PR objectives (FastMode = true, EnableConvertByteArray = true, EmbedImagesInCell = true) and covered by the newEmbedImagesInCell_ExportConfig_WritesPlaceInCellNotDrawingandEmbedImagesInCell_False_KeepsFloatingDrawingtests. Users exportingbyte[]columns will not discover this feature without reading source code.
README.md#L1503-L1514: add a short example showingOpenXmlConfiguration { FastMode = true, EnableConvertByteArray = true, EmbedImagesInCell = true }used with an exporter, and state its prerequisites.README.zh-CN.md#L1669-L1680: add the same example and prerequisites in Chinese, matching the surrounding translation style.README_V2.md#L1793-L1807: add the same example using theMiniExcel.Exporters.GetOpenXmlExporter()fluent style already used in this file.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@README.md` around lines 1503 - 1514, Document the config-driven EmbedImagesInCell option in README.md lines 1503-1514 with an OpenXmlConfiguration example using FastMode, EnableConvertByteArray, and EmbedImagesInCell, plus its prerequisites; add the equivalent Chinese example and prerequisites in README.zh-CN.md lines 1669-1680, matching local translation style; and add the same configuration through the existing MiniExcel.Exporters.GetOpenXmlExporter() fluent style in README_V2.md lines 1793-1807. Mention that this supports embedding exported byte[] images in cells and requires the applicable OpenXML/Excel prerequisites.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/MiniExcel.OpenXml/Picture/OpenXmlPlaceInCellImplement.cs`:
- Around line 217-231: In EnsureWorkbookRichDataRelationships, load or create
the XML document with LoadOrCreateXml before calling GetOrCreateEntry for
WorkbookRelsPath. Then resolve the entry and save the populated document as
before, preserving the existing relationship updates and root validation.
- Around line 619-630: Update the no-change guard in ExpandDimension to also
verify that the existing start coordinates already cover the incoming maxCol and
maxRow; allow execution to continue when the image extends before the range
start, so the start-adjustment logic updates the dimension ref while preserving
the current early return for truly unchanged ranges.
In `@src/MiniExcel.OpenXml/Writer/OpenXmlWriter.cs`:
- Around line 51-54: Extend the configuration validation near the existing
EmbedImagesInCell/FastMode guard in OpenXmlWriter to also reject
EmbedImagesInCell when EnableConvertByteArray is false. Keep the existing
FastMode validation and use an InvalidOperationException with a clear
requirement message for the byte-array conversion setting.
---
Nitpick comments:
In `@README.md`:
- Around line 1503-1514: Document the config-driven EmbedImagesInCell option in
README.md lines 1503-1514 with an OpenXmlConfiguration example using FastMode,
EnableConvertByteArray, and EmbedImagesInCell, plus its prerequisites; add the
equivalent Chinese example and prerequisites in README.zh-CN.md lines 1669-1680,
matching local translation style; and add the same configuration through the
existing MiniExcel.Exporters.GetOpenXmlExporter() fluent style in README_V2.md
lines 1793-1807. Mention that this supports embedding exported byte[] images in
cells and requires the applicable OpenXML/Excel prerequisites.
In `@src/MiniExcel.OpenXml/Picture/OpenXmlPictureImplement.cs`:
- Around line 25-34: Remove the unused OpenXmlReader instance created in the
sheet-list initialization block, leaving OpenXmlReader.GetWorkbookRelsAsync as
the only reader-related operation and retaining disposableExcelArchive as the
archive owner.
In `@src/MiniExcel.OpenXml/Picture/OpenXmlPlaceInCellImplement.cs`:
- Around line 507-573: Update UpsertImageCell and its row insertion flow so a
modified row’s spans attribute remains consistent with the inserted cell’s
column; either widen the existing spans range to include the new column or
remove spans from the affected row. Preserve the current cell creation and
update behavior.
In `@src/MiniExcel.OpenXml/Writer/OpenXmlWriter.cs`:
- Around line 90-111: Update AddFilesToZipAsync to accept an optional file
predicate and apply it while selecting _files, preserving the existing behavior
when no predicate is provided. Replace the inline non-image entry-writing loop
in the EmbedImagesInCell branch of OpenXmlWriter with AddFilesToZipAsync using
!f.IsImage, while keeping the shared-string, workbook, content-type, and image
generation calls unchanged.
In `@tests/MiniExcel.OpenXml.Tests/MiniExcelPlaceInCellTests.cs`:
- Around line 195-249: Add a negative-path test near
EmbedImagesInCell_ExportConfig_WritesPlaceInCellNotDrawing that sets
EmbedImagesInCell = true while disabling FastMode or EnableConvertByteArray.
Assert the documented exception or fallback behavior, using the existing
_excelExporter.Export flow and OpenXmlConfiguration setup, and cover both
missing-prerequisite combinations if the contract distinguishes them.
In `@tests/MiniExcel.Tests.Common/Localization.Designer.cs`:
- Around line 3-7: Revert the unrelated Chinese localization changes in the
generated Localization.Designer.cs comments, restoring the English
tool-generated text used elsewhere in the codebase; leave the generated resource
code and PlaceInCell changes untouched.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: c24d6bff-a2a9-4cb5-81cb-a6536aa6b41c
📒 Files selected for processing (13)
README.mdREADME.zh-CN.mdREADME_V2.mdsrc/MiniExcel.Core/Enums/XlsxImgType.cssrc/MiniExcel.OpenXml/OpenXmlConfiguration.cssrc/MiniExcel.OpenXml/Picture/OpenXmlPicture.cssrc/MiniExcel.OpenXml/Picture/OpenXmlPictureImplement.cssrc/MiniExcel.OpenXml/Picture/OpenXmlPlaceInCellImplement.cssrc/MiniExcel.OpenXml/Reader/OpenXmlReader.cssrc/MiniExcel.OpenXml/Writer/OpenXmlWriter.XmlGeneration.cssrc/MiniExcel.OpenXml/Writer/OpenXmlWriter.cstests/MiniExcel.OpenXml.Tests/MiniExcelPlaceInCellTests.cstests/MiniExcel.Tests.Common/Localization.Designer.cs
| private static void EnsureWorkbookRichDataRelationships(ZipArchive archive) | ||
| { | ||
| var entry = GetOrCreateEntry(archive, WorkbookRelsPath); | ||
| var doc = LoadOrCreateXml(archive, WorkbookRelsPath, CreateRelationshipsXml); | ||
| var root = doc.DocumentElement | ||
| ?? throw new InvalidOperationException("workbook.xml.rels is missing a root element."); | ||
|
|
||
| EnsureRelationship(root, RelSheetMetadata, "metadata.xml"); | ||
| EnsureRelationship(root, RelRdRichValue, "richData/rdrichvalue.xml"); | ||
| EnsureRelationship(root, RelRdRichValueStructure, "richData/rdrichvaluestructure.xml"); | ||
| EnsureRelationship(root, RelRdRichValueTypes, "richData/rdRichValueTypes.xml"); | ||
| EnsureRelationship(root, RelRichValueRel, "richData/richValueRel.xml"); | ||
|
|
||
| SaveXml(doc, entry); | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Reorder GetOrCreateEntry and LoadOrCreateXml.
Line 219 can create an empty xl/_rels/workbook.xml.rels entry. Line 220 then calls LoadOrCreateXml for the same path. Because the entry now exists, LoadOrCreateXml no longer uses the CreateRelationshipsXml factory. It falls through to LoadXml, which returns a bare new XmlDocument() for an empty stream (line 758-759). That document has no root element, so line 221 throws "workbook.xml.rels is missing a root element.".
A valid xlsx always contains this part, so the path is not reachable today. The ordering still defeats the factory fallback and makes the helper fragile.
Load the document first, then resolve the entry.
🛠️ Proposed fix
private static void EnsureWorkbookRichDataRelationships(ZipArchive archive)
{
- var entry = GetOrCreateEntry(archive, WorkbookRelsPath);
var doc = LoadOrCreateXml(archive, WorkbookRelsPath, CreateRelationshipsXml);
+ var entry = GetOrCreateEntry(archive, WorkbookRelsPath);
var root = doc.DocumentElement
?? throw new InvalidOperationException("workbook.xml.rels is missing a root element.");📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| private static void EnsureWorkbookRichDataRelationships(ZipArchive archive) | |
| { | |
| var entry = GetOrCreateEntry(archive, WorkbookRelsPath); | |
| var doc = LoadOrCreateXml(archive, WorkbookRelsPath, CreateRelationshipsXml); | |
| var root = doc.DocumentElement | |
| ?? throw new InvalidOperationException("workbook.xml.rels is missing a root element."); | |
| EnsureRelationship(root, RelSheetMetadata, "metadata.xml"); | |
| EnsureRelationship(root, RelRdRichValue, "richData/rdrichvalue.xml"); | |
| EnsureRelationship(root, RelRdRichValueStructure, "richData/rdrichvaluestructure.xml"); | |
| EnsureRelationship(root, RelRdRichValueTypes, "richData/rdRichValueTypes.xml"); | |
| EnsureRelationship(root, RelRichValueRel, "richData/richValueRel.xml"); | |
| SaveXml(doc, entry); | |
| } | |
| private static void EnsureWorkbookRichDataRelationships(ZipArchive archive) | |
| { | |
| var doc = LoadOrCreateXml(archive, WorkbookRelsPath, CreateRelationshipsXml); | |
| var entry = GetOrCreateEntry(archive, WorkbookRelsPath); | |
| var root = doc.DocumentElement | |
| ?? throw new InvalidOperationException("workbook.xml.rels is missing a root element."); | |
| EnsureRelationship(root, RelSheetMetadata, "metadata.xml"); | |
| EnsureRelationship(root, RelRdRichValue, "richData/rdrichvalue.xml"); | |
| EnsureRelationship(root, RelRdRichValueStructure, "richData/rdrichvaluestructure.xml"); | |
| EnsureRelationship(root, RelRdRichValueTypes, "richData/rdRichValueTypes.xml"); | |
| EnsureRelationship(root, RelRichValueRel, "richData/richValueRel.xml"); | |
| SaveXml(doc, entry); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/MiniExcel.OpenXml/Picture/OpenXmlPlaceInCellImplement.cs` around lines
217 - 231, In EnsureWorkbookRichDataRelationships, load or create the XML
document with LoadOrCreateXml before calling GetOrCreateEntry for
WorkbookRelsPath. Then resolve the entry and save the populated document as
before, preserving the existing relationship updates and root validation.
| var newEndCol = Math.Max(endCol, maxCol); | ||
| var newEndRow = Math.Max(endRow, maxRow); | ||
| if (newEndCol == endCol && newEndRow == endRow) | ||
| return; | ||
|
|
||
| var newStart = CellReferenceConverter.GetCellFromCoordinates(Math.Min(startCol, maxCol), Math.Min(startRow, maxRow)); | ||
| // Keep original start if image is within/after the existing range start | ||
| if (startCol <= maxCol && startRow <= maxRow) | ||
| newStart = start; | ||
|
|
||
| var newEnd = CellReferenceConverter.GetCellFromCoordinates(newEndCol, newEndRow); | ||
| dimension.SetAttribute("ref", newStart == newEnd ? newStart : $"{newStart}:{newEnd}"); |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
ExpandDimension never expands the range start.
Line 621 returns early when the end of the range does not grow. The start-adjustment logic at lines 624-627 is therefore unreachable whenever maxCol <= endCol and maxRow <= endRow.
Example: the existing dimension/@ref is C3:D4 and an image is placed at A1. newEndCol == endCol and newEndRow == endRow, so the method returns and the ref stays C3:D4. The ref no longer covers the populated cell A1.
This matters for round-tripping. OpenXmlReader.TryGetMaxRowColumnIndexAsync stops parsing as soon as it reads the dimension element and derives max row and column from it. A ref that omits written cells produces short reads.
Include the start in the no-change guard.
🐛 Proposed fix
var newEndCol = Math.Max(endCol, maxCol);
var newEndRow = Math.Max(endRow, maxRow);
- if (newEndCol == endCol && newEndRow == endRow)
+ var newStartCol = Math.Min(startCol, maxCol);
+ var newStartRow = Math.Min(startRow, maxRow);
+ if (newEndCol == endCol && newEndRow == endRow
+ && newStartCol == startCol && newStartRow == startRow)
return;
- var newStart = CellReferenceConverter.GetCellFromCoordinates(Math.Min(startCol, maxCol), Math.Min(startRow, maxRow));
- // Keep original start if image is within/after the existing range start
- if (startCol <= maxCol && startRow <= maxRow)
- newStart = start;
-
+ var newStart = newStartCol == startCol && newStartRow == startRow
+ ? start
+ : CellReferenceConverter.GetCellFromCoordinates(newStartCol, newStartRow);
var newEnd = CellReferenceConverter.GetCellFromCoordinates(newEndCol, newEndRow);
dimension.SetAttribute("ref", newStart == newEnd ? newStart : $"{newStart}:{newEnd}");📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| var newEndCol = Math.Max(endCol, maxCol); | |
| var newEndRow = Math.Max(endRow, maxRow); | |
| if (newEndCol == endCol && newEndRow == endRow) | |
| return; | |
| var newStart = CellReferenceConverter.GetCellFromCoordinates(Math.Min(startCol, maxCol), Math.Min(startRow, maxRow)); | |
| // Keep original start if image is within/after the existing range start | |
| if (startCol <= maxCol && startRow <= maxRow) | |
| newStart = start; | |
| var newEnd = CellReferenceConverter.GetCellFromCoordinates(newEndCol, newEndRow); | |
| dimension.SetAttribute("ref", newStart == newEnd ? newStart : $"{newStart}:{newEnd}"); | |
| var newEndCol = Math.Max(endCol, maxCol); | |
| var newEndRow = Math.Max(endRow, maxRow); | |
| var newStartCol = Math.Min(startCol, maxCol); | |
| var newStartRow = Math.Min(startRow, maxRow); | |
| if (newEndCol == endCol && newEndRow == endRow | |
| && newStartCol == startCol && newStartRow == startRow) | |
| return; | |
| var newStart = newStartCol == startCol && newStartRow == startRow | |
| ? start | |
| : CellReferenceConverter.GetCellFromCoordinates(newStartCol, newStartRow); | |
| var newEnd = CellReferenceConverter.GetCellFromCoordinates(newEndCol, newEndRow); | |
| dimension.SetAttribute("ref", newStart == newEnd ? newStart : $"{newStart}:{newEnd}"); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/MiniExcel.OpenXml/Picture/OpenXmlPlaceInCellImplement.cs` around lines
619 - 630, Update the no-change guard in ExpandDimension to also verify that the
existing start coordinates already cover the incoming maxCol and maxRow; allow
execution to continue when the image extends before the range start, so the
start-adjustment logic updates the dimension ref while preserving the current
early return for truly unchanged ranges.
| // Place-in-cell rewriting needs ZipArchiveMode.Update (seekable entry rewrite). | ||
| if (conf is { EmbedImagesInCell: true, FastMode: false }) | ||
| throw new InvalidOperationException("EmbedImagesInCell requires FastMode to be enabled"); | ||
|
|
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Also validate EnableConvertByteArray for EmbedImagesInCell.
The XML doc on OpenXmlConfiguration.EmbedImagesInCell states that the option requires EnableConvertByteArray = true. This check enforces only FastMode.
If a caller sets EmbedImagesInCell = true and EnableConvertByteArray = false, the byte[] branch in GetCellValue (OpenXmlWriter.XmlGeneration.cs line 226) is skipped. Execution falls through to the final value.ToString() return, which writes the literal text System.Byte[] into the cell. The feature fails silently and produces incorrect output.
Add the matching guard so the misconfiguration fails fast.
🛡️ Proposed fix
// Place-in-cell rewriting needs ZipArchiveMode.Update (seekable entry rewrite).
if (conf is { EmbedImagesInCell: true, FastMode: false })
throw new InvalidOperationException("EmbedImagesInCell requires FastMode to be enabled");
+
+ if (conf is { EmbedImagesInCell: true, EnableConvertByteArray: false })
+ throw new InvalidOperationException("EmbedImagesInCell requires EnableConvertByteArray to be enabled");📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Place-in-cell rewriting needs ZipArchiveMode.Update (seekable entry rewrite). | |
| if (conf is { EmbedImagesInCell: true, FastMode: false }) | |
| throw new InvalidOperationException("EmbedImagesInCell requires FastMode to be enabled"); | |
| // Place-in-cell rewriting needs ZipArchiveMode.Update (seekable entry rewrite). | |
| if (conf is { EmbedImagesInCell: true, FastMode: false }) | |
| throw new InvalidOperationException("EmbedImagesInCell requires FastMode to be enabled"); | |
| if (conf is { EmbedImagesInCell: true, EnableConvertByteArray: false }) | |
| throw new InvalidOperationException("EmbedImagesInCell requires EnableConvertByteArray to be enabled"); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/MiniExcel.OpenXml/Writer/OpenXmlWriter.cs` around lines 51 - 54, Extend
the configuration validation near the existing EmbedImagesInCell/FastMode guard
in OpenXmlWriter to also reject EmbedImagesInCell when EnableConvertByteArray is
false. Keep the existing FastMode validation and use an
InvalidOperationException with a clear requirement message for the byte-array
conversion setting.
2 participants
Summary by CodeRabbit
New Features
Documentation
Bug Fixes