close
Skip to content

Added resource-based localization for column attributes#967

Merged
michelebastione merged 2 commits into
mini-software:masterfrom
michelebastione:localization-support
May 24, 2026
Merged

Added resource-based localization for column attributes#967
michelebastione merged 2 commits into
mini-software:masterfrom
michelebastione:localization-support

Conversation

@michelebastione
Copy link
Copy Markdown
Collaborator

Introduced resource-based localization for column names by adding ResourceType support and ResourceManager resolution to MiniExcelColumnAttribute and MiniExcelColumnNameAttribute, added unit tests covering localization for some cultures, and updated README to document the feature usage.

Resolves #478

Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a 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 introduces resource-based localization for MiniExcelColumnAttribute and MiniExcelColumnNameAttribute, allowing column names to be resolved dynamically from .NET resource files. The changes include updates to the core attributes, logic adjustments in ColumnMappingsProvider to support localized name resolution, and comprehensive test coverage for multiple cultures. Review feedback highlights an issue where GetColumnName returns an empty string instead of null, which would break the fallback logic in the mapping provider. Additionally, it is recommended to refactor the duplicated ResourceManager initialization logic into a shared base class or helper method to improve maintainability.

Comment thread src/MiniExcel.Core/Attributes/MiniExcelColumnNameAttribute.cs Outdated
Comment thread src/MiniExcel.Core/Attributes/MiniExcelColumnAttribute.cs
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a 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 introduces resource-based localization support for MiniExcel attributes, allowing column names to be resolved from .resx files. The review identifies several issues, including a breaking change caused by renaming the public 'ExcelColumnName' property and a bug in the 'Init' method that prevents 'IndexName' from being updated correctly. Other feedback points to flawed 'ResourceManager' resolution logic, inconsistent return values that break naming fallbacks, and potential compatibility concerns regarding the use of the C# 13 'field' keyword.

Comment thread src/MiniExcel.Core/Attributes/MiniExcelColumnAttribute.cs Outdated
Comment thread src/MiniExcel.Core/Attributes/MiniExcelColumnAttribute.cs
Comment thread src/MiniExcel.Core/Attributes/MiniExcelColumnNameAttribute.cs
Comment thread src/MiniExcel.Core/Attributes/MiniExcelColumnNameAttribute.cs Outdated
Comment thread src/MiniExcel.Core/Attributes/MiniExcelColumnAttribute.cs Outdated
@michelebastione michelebastione merged commit f9aa916 into mini-software:master May 24, 2026
3 checks passed
@michelebastione michelebastione deleted the localization-support branch May 24, 2026 22:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Allow text Localization

1 participant