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

.labels #276

Closed
rogsmith opened this issue Nov 30, 2015 · 6 comments
Closed

.labels #276

rogsmith opened this issue Nov 30, 2015 · 6 comments

Comments

@rogsmith
Copy link

NoMethodError: undefined method split' for nil:NilClass lib/roo/excelx/workbook.rb:35:inblock in defined_names'

@stevendaniels
Copy link
Contributor

I'd really need to see a copy of your excel file in order to understand why this isn't working properly.Can you create a gist for this issue (sample gist)? Here are some instructions for creating such a gist.

  1. Create a gist with code that creates the error.
  2. Clone the gist repo locally, add a stripped down version of the offending spreadsheet to the gist repo, and push the gist's changes master.
  3. Paste the gist url here.

@stevendaniels
Copy link
Contributor

@rogsmith Any chance I could see a sample of the file that caused the error you encountered?

@stevendaniels
Copy link
Contributor

Please reopen if the issue occurs again.

@md5
Copy link

md5 commented Dec 28, 2017

I just ran into a sheet exhibiting this issue. The definedName in question looked like 'Sheet1'!#REF!, so the split on !$ only returns a single item, leaving coordinates set to nil and causing the exception mentioned above when nil#split is called.

@md5
Copy link

md5 commented Dec 28, 2017

Here's the simplest fix I can see for this:

diff --git a/lib/roo/excelx/workbook.rb b/lib/roo/excelx/workbook.rb
index 7ef841f..bb79e02 100644
--- a/lib/roo/excelx/workbook.rb
+++ b/lib/roo/excelx/workbook.rb
@@ -32,10 +32,12 @@ module Roo
         Hash[doc.xpath('//definedName').map do |defined_name|
           # "Sheet1!$C$5"
           sheet, coordinates = defined_name.text.split('!$', 2)
+          next if coordinates.nil?
+
           col, row = coordinates.split('$')
           name = defined_name['name']
           [name, Label.new(name, sheet, row, col)]
-        end]
+        end.compact]
       end
 
       def base_date

If you want to actually represent invalid labels in the hash returned by defined_names, then the Label class would need to be enhanced to allow it to represent these invalid labels. The parsing would also need to be changed to split on ! instead of !$ and to deal with the reference after the ! consistently. To be honest, it's probably not a bad idea to use common code for parsing that reference anyways instead of ad hoc code in the defined_names method.

@md5
Copy link

md5 commented Jan 24, 2018

@stevendaniels Do you want me to open a PR with my suggested change?

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

No branches or pull requests

3 participants