- Notifications
You must be signed in to change notification settings - Fork1.9k
Fix misinterpreted ranges from <definedName>#636
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
@guyonroche, is anything holding this back? It happened to us again today, and we'd rather not go back to forking exceljs :) |
@guyonroche, sorry to be a ping-in-the-ass 😼, but have you had a chance to take a look at this? |
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.
I trust that tests are green ;)
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.
looks good
Hi!
I ran into a file that contained a bunch of formulas and external references in
<definedName>
s, which caused this crash:Turns out it's caused by what looks likea rather naive range extraction algorithm that doesn't take into account that the defined names could contain double quoted strings or formulas.
I'm unable to share the original file in its full glory, but I made a new one that reproduces one variant of the error.
I'm really not sure that this is the right fix -- it would probably be better to implement proper tokenization/parsing in
extractRanges
so that the full syntax is understood. Then again, that would be a lot of work, and to be honest I'm have no idea if those ranges are even used for anything important during the parsing?