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

Ignore formula tag that isn't within a cell #128

Merged
merged 1 commit into from
Jan 12, 2018
Merged

Ignore formula tag that isn't within a cell #128

merged 1 commit into from
Jan 12, 2018

Conversation

cmuchinsky
Copy link

@cmuchinsky cmuchinsky commented Jan 11, 2018

This pull request fixes an issue where a formula tag that shows up outside of a cell is incorrectly being attached to the previous cell.

@ms1111
Copy link
Contributor

ms1111 commented Jan 12, 2018

Oh wow - I can only imagine the horrors that went into producing such a broken file :)

I'm just a user of the library, but the code change looks good to me and includes a unit test and sample file. 👍

Can you tell me where in the sample file to find the errant <f>? I'm not seeing it in sheet1.xml. The last row doesn't seem to have any formula tags.

    <row r="34" spans="2:3" ht="25" customHeight="1" x14ac:dyDescent="0.2">
      <c r="B34" s="5" t="s">
        <v>24</v>
      </c>
      <c r="C34" s="4">
        <v>100</v>
      </c>
    </row>

@cmuchinsky
Copy link
Author

Thanks for the second set of eyes @ms1111, the test file is actually an older version of the Manage My Money template from Microsoft. I thought the errant formula tag was after the cell in the row but perhaps not? I never converted it to an xml representation to see clearly where it is, but IIRC it might have been from the Chart Data formula.

@monitorjbl
Copy link
Owner

Yeah, I don't know how all these wonky worksheets have wound up in the wild. The PR looks good though and thanks for including a test file that demonstrates the issue!

@monitorjbl monitorjbl merged commit 3a06f17 into monitorjbl:master Jan 12, 2018
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.

3 participants