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

Problem with broken links #533

Closed
estani opened this issue Mar 25, 2020 · 1 comment · Fixed by #543
Closed

Problem with broken links #533

estani opened this issue Mar 25, 2020 · 1 comment · Fixed by #543

Comments

@estani
Copy link

estani commented Mar 25, 2020

If broken links are found (references to non existing sheets, DB, etc) a set of problems arise, e.g.

 > worksheet.sheet(1).cell(1,1)
'assuming this works'
 > worksheet.sheet(1).a1
NoMethodError (undefined method `split' for nil:NilClass)

The main culprit is:
File: ../../.rbenv/versions/2.5.7/lib/ruby/gems/2.5.0/gems/roo-2.8.3/lib/roo/excelx/workbook.rb
31: def defined_names
32: doc.xpath('//definedName').each_with_object({}) do |defined_name, hash|
33: # "Sheet1!$C$5"
34: sheet, coordinates = defined_name.text.split('!$', 2)
35: col, row = coordinates.split('$')
36: name = defined_name['name']
37: hash[name] = Label.new(name, sheet, row, col)
38: end
39: end

defined_name.text turns out to be "Datenblatt!#REF!" which can't be properly split at !$

The main question is why !$ is used instead of simply ! which seems more proper. In any case a more robust solution (which I'm using now) would be:

File: ../../.rbenv/versions/2.5.7/lib/ruby/gems/2.5.0/gems/roo-2.8.3/lib/roo/excelx/workbook.rb
31: def defined_names
32: doc.xpath('//definedName').each_with_object({}) do |defined_name, hash|
33: # "Sheet1!$C$5"
34: sheet, coordinates = defined_name.text.split('!$', 2)
35: next unless coordinates
36:
37: col, row = coordinates.split('$')
38: name = defined_name['name']
39: hash[name] = Label.new(name, sheet, row, col)
40: end
41: end

(a pull request costs me much more than writing this, and I have no Idea why !$ is important, as I don't know the format)

@coorasse
Copy link
Contributor

coorasse commented Oct 6, 2020

Formatted lib/roo/excelx/workbook.rb:

current version

def defined_names
  doc.xpath('//definedName').each_with_object({}) do |defined_name, hash|
    # "Sheet1!$C$5"
    sheet, coordinates = defined_name.text.split('!$', 2)
    col, row = coordinates.split('$')
    name = defined_name['name']
    hash[name] = Label.new(name, sheet, row, col)
  end
end

patched version

def defined_names
  doc.xpath('//definedName').each_with_object({}) do |defined_name, hash|
    # "Sheet1!$C$5"
    sheet, coordinates = defined_name.text.split('!$', 2)
    next unless coordinates
    col, row = coordinates.split('$')
    name = defined_name['name']
    hash[name] = Label.new(name, sheet, row, col)
  end
end

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants