-
-
Notifications
You must be signed in to change notification settings - Fork 399
docs: add horizontal fill workaround #912
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
docs: add horizontal fill workaround #912
Conversation
Summary of ChangesHello @fuguiKz, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request addresses a common user need by enhancing the MiniExcel documentation with practical guidance on handling horizontal data filling. It introduces new sections in both English and Chinese README files, offering a primary workaround involving data transposition and an alternative approach using scalar placeholders, thereby improving the clarity and utility of the documentation for users facing this specific challenge. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
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.
Code Review
This pull request adds documentation for a workaround to achieve horizontal data filling, which is a current limitation of MiniExcel templates. The changes are added to both the English and Chinese README files.
My review focuses on improving the provided C# code example. The current example is repetitive and not easily maintainable. I've suggested an alternative using reflection that is more robust and demonstrates a better coding practice for transposing data. This makes the example more useful as a copy-paste solution for users with different data structures.
README.md
Outdated
| var table = new DataTable(); | ||
| table.Columns.Add("A"); | ||
| for (var i = 0; i < employees.Length; i++) | ||
| table.Columns.Add($"B{i + 1}"); | ||
|
|
||
| table.Rows.Add(new object[] { "Name" }.Concat(employees.Select(e => (object)e.Name)).ToArray()); | ||
| table.Rows.Add(new object[] { "Department" }.Concat(employees.Select(e => (object)e.Department)).ToArray()); | ||
| table.Rows.Add(new object[] { "City" }.Concat(employees.Select(e => (object)e.City)).ToArray()); | ||
| table.Rows.Add(new object[] { "Country" }.Concat(employees.Select(e => (object)e.Country)).ToArray()); |
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 current example for transposing data is a bit repetitive and not very robust. If properties of the employees objects change, this code would need significant updates.
A more generic approach using reflection would be more resilient to changes and demonstrates a better practice. It automatically discovers the properties and creates a row for each, making the code cleaner and easier to maintain.
| var table = new DataTable(); | |
| table.Columns.Add("A"); | |
| for (var i = 0; i < employees.Length; i++) | |
| table.Columns.Add($"B{i + 1}"); | |
| table.Rows.Add(new object[] { "Name" }.Concat(employees.Select(e => (object)e.Name)).ToArray()); | |
| table.Rows.Add(new object[] { "Department" }.Concat(employees.Select(e => (object)e.Department)).ToArray()); | |
| table.Rows.Add(new object[] { "City" }.Concat(employees.Select(e => (object)e.City)).ToArray()); | |
| table.Rows.Add(new object[] { "Country" }.Concat(employees.Select(e => (object)e.Country)).ToArray()); | |
| var table = new DataTable(); | |
| table.Columns.Add("Header"); | |
| for (var i = 0; i < employees.Length; i++) | |
| table.Columns.Add(); // Column names don't matter with printHeader: false | |
| if (employees.Any()) | |
| { | |
| var properties = employees.First().GetType().GetProperties(); | |
| foreach (var prop in properties) | |
| { | |
| var rowValues = new List<object> { prop.Name }; | |
| rowValues.AddRange(employees.Select(e => prop.GetValue(e))); | |
| table.Rows.Add(rowValues.ToArray()); | |
| } | |
| } |
README.zh-CN.md
Outdated
| var table = new DataTable(); | ||
| table.Columns.Add("A"); | ||
| for (var i = 0; i < employees.Length; i++) | ||
| table.Columns.Add($"B{i + 1}"); | ||
|
|
||
| table.Rows.Add(new object[] { "Name" }.Concat(employees.Select(e => (object)e.Name)).ToArray()); | ||
| table.Rows.Add(new object[] { "Department" }.Concat(employees.Select(e => (object)e.Department)).ToArray()); | ||
| table.Rows.Add(new object[] { "City" }.Concat(employees.Select(e => (object)e.City)).ToArray()); | ||
| table.Rows.Add(new object[] { "Country" }.Concat(employees.Select(e => (object)e.Country)).ToArray()); |
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 current example for transposing data is a bit repetitive and not very robust. If properties of the employees objects change, this code would need significant updates.
A more generic approach using reflection would be more resilient to changes and demonstrates a better practice. It automatically discovers the properties and creates a row for each, making the code cleaner and easier to maintain. This change should be applied to the Chinese documentation as well to maintain consistency with the English version.
| var table = new DataTable(); | |
| table.Columns.Add("A"); | |
| for (var i = 0; i < employees.Length; i++) | |
| table.Columns.Add($"B{i + 1}"); | |
| table.Rows.Add(new object[] { "Name" }.Concat(employees.Select(e => (object)e.Name)).ToArray()); | |
| table.Rows.Add(new object[] { "Department" }.Concat(employees.Select(e => (object)e.Department)).ToArray()); | |
| table.Rows.Add(new object[] { "City" }.Concat(employees.Select(e => (object)e.City)).ToArray()); | |
| table.Rows.Add(new object[] { "Country" }.Concat(employees.Select(e => (object)e.Country)).ToArray()); | |
| var table = new DataTable(); | |
| table.Columns.Add("Header"); | |
| for (var i = 0; i < employees.Length; i++) | |
| table.Columns.Add(); // Column names don't matter with printHeader: false | |
| if (employees.Any()) | |
| { | |
| var properties = employees.First().GetType().GetProperties(); | |
| foreach (var prop in properties) | |
| { | |
| var rowValues = new List<object> { prop.Name }; | |
| rowValues.AddRange(employees.Select(e => prop.GetValue(e))); | |
| table.Rows.Add(rowValues.ToArray()); | |
| } | |
| } |
|
@fuguiKz Thank you for your contribution! Could you please move your workaround to the FAQ section of the readme and also add it to the README-V2 document? |
|
Thanks @michelebastione! I moved the workaround into the FAQ section of README.md and also added the same FAQ entry to README-V2.md.\n\n(I kept README.zh-CN.md in sync as well—happy to revert that part if you prefer English-only changes.) |
|
Looks good, thank you! |
Issue #619 asks about filling data horizontally (left-to-right) with MiniExcel templates.
MiniExcel template collection rendering currently expands vertically (top-to-bottom), so this PR adds a small doc section (EN + zh-CN) that:
MiniExcel.SaveAs(..., printHeader: false)Refs #619.