Skip to content
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

Fix Add method when table name is null. #410 #411

Closed
wants to merge 1 commit into from

Conversation

vmrocha
Copy link

@vmrocha vmrocha commented Jul 18, 2017

What's this PR do?

Fixes issue #410

Where should the reviewer start?

From the new unit test.

How should this be manually tested?

By trying to edit the file using ClosedXML.

Any background context you want to provide?

The file was initially created with ClosedXML and edited using Google Spreadsheet.

Screenshots (if appropriate)

Questions:

  • Is there a blog post? No

  • Does the knowledge base need an update? No

  • Does this add new (C#) dependencies? No

  • C# Code Review: @csreviewer

  • Test Automation Review: @csreviewer

@@ -278,13 +278,16 @@
<EmbeddedResource Include="Resource\Examples\Ranges\AddingRowToTables.xlsx" />
<EmbeddedResource Include="Resource\Examples\Misc\RightToLeft.xlsx" />
<EmbeddedResource Include="Resource\Examples\PivotTables\PivotTables.xlsx" />
<None Include="packages.config" />
<None Include="packages.config">
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Undo this change

@@ -24,7 +24,7 @@ IEnumerator IEnumerable.GetEnumerator()

public void Add(IXLTable table)
{
_tables.Add(table.Name, table);
_tables.Add(table.Name ?? String.Empty, table);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will lead to duplicate keys if the file contains multiple nameless tables.

@igitur
Copy link
Member

igitur commented Jul 18, 2017

Fixed in #412

@igitur igitur closed this Jul 18, 2017
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.

None yet

2 participants