From 9110c929fbf89abe8b1ddb53a85ccf93a785ff8a Mon Sep 17 00:00:00 2001 From: Steven Daniels Date: Mon, 1 Jun 2015 12:54:32 +0800 Subject: [PATCH 01/14] Use Shared class to pass shared data to Excelx Sheets. --- lib/roo/excelx.rb | 31 +++++++++++++------------------ lib/roo/excelx/shared.rb | 32 ++++++++++++++++++++++++++++++++ 2 files changed, 45 insertions(+), 18 deletions(-) create mode 100644 lib/roo/excelx/shared.rb diff --git a/lib/roo/excelx.rb b/lib/roo/excelx.rb index d4e00ae3..0823927c 100644 --- a/lib/roo/excelx.rb +++ b/lib/roo/excelx.rb @@ -2,9 +2,13 @@ require 'zip/filesystem' require 'roo/link' require 'roo/utils' +require 'forwardable' module Roo class Excelx < Roo::Base + extend Forwardable + + require 'roo/excelx/shared' require 'roo/excelx/workbook' require 'roo/excelx/shared_strings' require 'roo/excelx/styles' @@ -13,7 +17,9 @@ class Excelx < Roo::Base require 'roo/excelx/relationships' require 'roo/excelx/comments' require 'roo/excelx/sheet_doc' - + + delegate [:styles, :workbook, :shared_strings, :rels_files, :sheet_files, :comments_files] => :@shared + module Format EXCEPTIONAL_FORMATS = { 'h:mm am/pm' => :date, @@ -94,9 +100,9 @@ def initialize(filename_or_stream, options = {}) end @tmpdir = make_tmpdir(basename, options[:tmpdir_root]) + @shared = Shared.new(@tmpdir) @filename = local_filename(filename_or_stream, @tmpdir, packed) - @comments_files = [] - @rels_files = [] + process_zipfile(@filename || filename_or_stream) @sheet_names = workbook.sheets.map do |sheet| @@ -106,7 +112,7 @@ def initialize(filename_or_stream, options = {}) end.compact @sheets = [] @sheets_by_name = Hash[@sheet_names.map.with_index do |sheet_name, n| - @sheets[n] = Sheet.new(sheet_name, @rels_files[n], @sheet_files[n], @comments_files[n], styles, shared_strings, workbook, sheet_options) + @sheets[n] = Sheet.new(sheet_name, rels_files[n], sheet_files[n], comments_files[n], styles, shared_strings, workbook, sheet_options) [sheet_name, @sheets[n]] end] @@ -399,6 +405,7 @@ def extract_sheets_in_order(entries, sheet_ids, sheets, tmpdir) name = sheets[id] entry = entries.find { |e| e.name =~ /#{name}$/ } path = "#{tmpdir}/roo_sheet#{i + 1}" + sheet_files << path @sheet_files << path entry.extract(path) end @@ -458,31 +465,19 @@ def process_zipfile_entries(entries) # sheet's comment file is in the sheet1.xml.rels file. SEE # ECMA-376 12.3.3 in "Ecma Office Open XML Part 1". nr = Regexp.last_match[1].to_i - @comments_files[nr - 1] = "#{@tmpdir}/roo_comments#{nr}" + comments_files[nr - 1] = "#{@tmpdir}/roo_comments#{nr}" when /sheet([0-9]+).xml.rels$/ # FIXME: Roo seems to use sheet[\d].xml.rels for hyperlinks only, but # it also stores the location for sharedStrings, comments, # drawings, etc. nr = Regexp.last_match[1].to_i - @rels_files[nr - 1] = "#{@tmpdir}/roo_rels#{nr}" + rels_files[nr - 1] = "#{@tmpdir}/roo_rels#{nr}" end entry.extract(path) if path end end - def styles - @styles ||= Styles.new(File.join(@tmpdir, 'roo_styles.xml')) - end - - def shared_strings - @shared_strings ||= SharedStrings.new(File.join(@tmpdir, 'roo_sharedStrings.xml')) - end - - def workbook - @workbook ||= Workbook.new(File.join(@tmpdir, 'roo_workbook.xml')) - end - def safe_send(object, method, *args) object.send(method, *args) if object && object.respond_to?(method) end diff --git a/lib/roo/excelx/shared.rb b/lib/roo/excelx/shared.rb new file mode 100644 index 00000000..3677fa25 --- /dev/null +++ b/lib/roo/excelx/shared.rb @@ -0,0 +1,32 @@ +module Roo + class Excelx + # Public: Shared class for allowing sheets to share data. This should + # reduce memory usage and reduce the number of objects being passed + # to various inititializers. + class Shared + attr_accessor :comments_files, :sheet_files, :rels_files + def initialize(dir) + @dir = dir + @comments_files = [] + @sheet_files = [] + @rels_files = [] + end + + def styles + @styles ||= Styles.new(File.join(@dir, 'roo_styles.xml')) + end + + def shared_strings + @shared_strings ||= SharedStrings.new(File.join(@dir, 'roo_sharedStrings.xml')) + end + + def workbook + @workbook ||= Workbook.new(File.join(@dir, 'roo_workbook.xml')) + end + + def base_date + workbook.base_date + end + end + end +end From 4b41a2e490ba01c99ff8b6d103a8e8076dd55bea Mon Sep 17 00:00:00 2001 From: Steven Daniels Date: Mon, 1 Jun 2015 16:55:46 +0800 Subject: [PATCH 02/14] Excelx::Sheet and Excelx::SheetDoc use shared data This changes the initializers for Roo::Excelx::Sheet and Roo::Excelx::SheetDoc, so a minor version bump will be required. --- lib/roo/excelx.rb | 3 ++- lib/roo/excelx/sheet.rb | 17 +++++++++++------ lib/roo/excelx/sheet_doc.rb | 18 ++++++++++-------- 3 files changed, 23 insertions(+), 15 deletions(-) diff --git a/lib/roo/excelx.rb b/lib/roo/excelx.rb index 0823927c..9d84f26b 100644 --- a/lib/roo/excelx.rb +++ b/lib/roo/excelx.rb @@ -112,7 +112,8 @@ def initialize(filename_or_stream, options = {}) end.compact @sheets = [] @sheets_by_name = Hash[@sheet_names.map.with_index do |sheet_name, n| - @sheets[n] = Sheet.new(sheet_name, rels_files[n], sheet_files[n], comments_files[n], styles, shared_strings, workbook, sheet_options) + # @sheets[n] = Sheet.new(sheet_name, rels_files[n], sheet_files[n], comments_files[n], styles, shared_strings, workbook, sheet_options) + @sheets[n] = Sheet.new(sheet_name, @shared, n, sheet_options) [sheet_name, @sheets[n]] end] diff --git a/lib/roo/excelx/sheet.rb b/lib/roo/excelx/sheet.rb index be78983c..b691d288 100644 --- a/lib/roo/excelx/sheet.rb +++ b/lib/roo/excelx/sheet.rb @@ -1,12 +1,17 @@ +require 'forwardable' module Roo class Excelx class Sheet - def initialize(name, rels_path, sheet_path, comments_path, styles, shared_strings, workbook, options = {}) + extend Forwardable + + delegate [:styles, :workbook, :shared_strings, :rels_files, :sheet_files, :comments_files] => :@shared + + def initialize(name, shared, sheet_index, options = {}) @name = name - @rels = Relationships.new(rels_path) - @comments = Comments.new(comments_path) - @styles = styles - @sheet = SheetDoc.new(sheet_path, @rels, @styles, shared_strings, workbook, options) + @shared = shared + @rels = Relationships.new(rels_files[sheet_index]) + @comments = Comments.new(comments_files[sheet_index]) + @sheet = SheetDoc.new(sheet_files[sheet_index], @rels, shared, options) end def cells @@ -65,7 +70,7 @@ def last_column def excelx_format(key) cell = cells[key] - @styles.style_format(cell.style).to_s if cell + styles.style_format(cell.style).to_s if cell end def hyperlinks diff --git a/lib/roo/excelx/sheet_doc.rb b/lib/roo/excelx/sheet_doc.rb index 092e8b7f..6aa2c727 100644 --- a/lib/roo/excelx/sheet_doc.rb +++ b/lib/roo/excelx/sheet_doc.rb @@ -1,15 +1,17 @@ +require 'forwardable' require 'roo/excelx/extractor' module Roo class Excelx class SheetDoc < Excelx::Extractor - def initialize(path, relationships, styles, shared_strings, workbook, options = {}) + extend Forwardable + delegate [:styles, :workbook, :shared_strings, :base_date] => :@shared + + def initialize(path, relationships, shared, options = {}) super(path) + @shared = shared @options = options @relationships = relationships - @styles = styles - @shared_strings = shared_strings - @workbook = workbook end def cells(relationships) @@ -62,7 +64,7 @@ def cell_from_xml(cell_xml, hyperlink) when 'inlineStr' :inlinestr else - format = @styles.style_format(style) + format = styles.style_format(style) Excelx::Format.to_type(format) end formula = nil @@ -72,7 +74,7 @@ def cell_from_xml(cell_xml, hyperlink) when 'is' cell.children.each do |inline_str| if inline_str.name == 't' - return Excelx::Cell.new(inline_str.content, :string, formula, :string, inline_str.content, style, hyperlink, @workbook.base_date, Excelx::Cell::Coordinate.new(row, column)) + return Excelx::Cell.new(inline_str.content, :string, formula, :string, inline_str.content, style, hyperlink, base_date, Excelx::Cell::Coordinate.new(row, column)) end end when 'f' @@ -92,7 +94,7 @@ def cell_from_xml(cell_xml, hyperlink) when :shared value_type = :string excelx_type = :string - @shared_strings[cell.content.to_i] + shared_strings[cell.content.to_i] when :boolean (cell.content.to_i == 1 ? 'TRUE' : 'FALSE') when :date, :time, :datetime @@ -106,7 +108,7 @@ def cell_from_xml(cell_xml, hyperlink) value_type = :float cell.content end - return Excelx::Cell.new(value, value_type, formula, excelx_type, cell.content, style, hyperlink, @workbook.base_date, Excelx::Cell::Coordinate.new(row, column)) + return Excelx::Cell.new(value, value_type, formula, excelx_type, cell.content, style, hyperlink, base_date, Excelx::Cell::Coordinate.new(row, column)) end end Excelx::Cell.new(nil, nil, nil, nil, nil, nil, nil, nil, Excelx::Cell::Coordinate.new(row, column)) From 9aca2ca76496aae74f8a7297cc4ec8f2a5e54813 Mon Sep 17 00:00:00 2001 From: Steven Daniels Date: Fri, 26 Jun 2015 21:06:31 +0800 Subject: [PATCH 03/14] Fix invalid new lines. Fixes #223. --- lib/roo/excelx/shared_strings.rb | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/lib/roo/excelx/shared_strings.rb b/lib/roo/excelx/shared_strings.rb index c2fd5ebe..4abf3804 100644 --- a/lib/roo/excelx/shared_strings.rb +++ b/lib/roo/excelx/shared_strings.rb @@ -13,9 +13,19 @@ def to_a private + def fix_invalid_shared_strings(doc) + invalid = { '_x000D_' => "\n" } + xml = doc.to_s + + if xml[/#{invalid.keys.join('|')}/] + @doc = ::Nokogiri::XML(xml.gsub(/#{invalid.keys.join('|')}/, invalid)) + end + end + def extract_shared_strings return [] unless doc_exists? + fix_invalid_shared_strings(doc) # read the shared strings xml document doc.xpath('/sst/si').map do |si| shared_string = '' From 2246a59b282c38b4ee08faa7c5c51378d8ceb801 Mon Sep 17 00:00:00 2001 From: Steven Daniels Date: Thu, 9 Jul 2015 07:08:26 +0800 Subject: [PATCH 04/14] Linting Roo::Base --- lib/roo/base.rb | 149 ++++++++++++++++++++++++------------------------ 1 file changed, 75 insertions(+), 74 deletions(-) diff --git a/lib/roo/base.rb b/lib/roo/base.rb index 7bec4719..6a7029e0 100644 --- a/lib/roo/base.rb +++ b/lib/roo/base.rb @@ -91,7 +91,7 @@ def collect_last_row_col_for_sheet(sheet) first_column = [first_column, key.last.to_i].min last_column = [last_column, key.last.to_i].max end if @cell[sheet] - {first_row: first_row, first_column: first_column, last_row: last_row, last_column: last_column} + { first_row: first_row, first_column: first_column, last_row: last_row, last_column: last_column } end %w(first_row last_row first_column last_column).each do |key| @@ -117,22 +117,23 @@ def to_yaml(prefix = {}, from_row = nil, from_column = nil, to_row = nil, to_col result = "--- \n" from_row.upto(to_row) do |row| from_column.upto(to_column) do |col| - unless empty?(row, col, sheet) - result << "cell_#{row}_#{col}: \n" - prefix.each do|k, v| - result << " #{k}: #{v} \n" - end - result << " row: #{row} \n" - result << " col: #{col} \n" - result << " celltype: #{celltype(row, col, sheet)} \n" - value = cell(row, col, sheet) - if celltype(row, col, sheet) == :time - value = integer_to_timestring(value) - end - result << " value: #{value} \n" + next if empty?(row, col, sheet) + + result << "cell_#{row}_#{col}: \n" + prefix.each do|k, v| + result << " #{k}: #{v} \n" + end + result << " row: #{row} \n" + result << " col: #{col} \n" + result << " celltype: #{celltype(row, col, sheet)} \n" + value = cell(row, col, sheet) + if celltype(row, col, sheet) == :time + value = integer_to_timestring(value) end + result << " value: #{value} \n" end end + result end @@ -170,7 +171,7 @@ def to_matrix(from_row = nil, from_column = nil, to_row = nil, to_column = nil, end def inspect - "<##{ self.class }:#{ self.object_id.to_s(8) } #{ self.instance_variables.join(' ') }>" + "<##{self.class}:#{object_id.to_s(8)} #{instance_variables.join(' ')}>" end # find a row either by row number or a condition @@ -217,7 +218,7 @@ def set(row, col, value, sheet = default_sheet) #:nodoc: row, col = normalize(row, col) cell_type = cell_type_by_value(value) set_value(row, col, value, sheet) - set_type(row, col, cell_type , sheet) + set_type(row, col, cell_type, sheet) end def cell_type_by_value(value) @@ -256,13 +257,13 @@ def info sheets.each do|sheet| self.default_sheet = sheet result << 'Sheet ' + n.to_s + ":\n" - unless first_row - result << ' - empty -' - else + if first_row result << " First row: #{first_row}\n" result << " Last row: #{last_row}\n" result << " First column: #{::Roo::Utils.number_to_letter(first_column)}\n" result << " Last column: #{::Roo::Utils.number_to_letter(last_column)}" + else + result << ' - empty -' end result << "\n" if sheet != sheets.last n += 1 @@ -282,12 +283,12 @@ def to_xml # sonst gibt es Fehler bei leeren Blaettern first_row.upto(last_row) do |row| first_column.upto(last_column) do |col| - unless empty?(row, col) - x.cell(cell(row, col), + next if empty?(row, col) + + x.cell(cell(row, col), row: row, column: col, type: celltype(row, col)) - end end end end @@ -318,7 +319,7 @@ def method_missing(m, *args) # access different worksheets by calling spreadsheet.sheet(1) # or spreadsheet.sheet('SHEETNAME') def sheet(index, name = false) - self.default_sheet = String === index ? index : sheets[index] + self.default_sheet = index.is_a?(::String) ? index : sheets[index] name ? [default_sheet, self] : self end @@ -352,25 +353,23 @@ def each_with_pagename # control characters and white spaces around columns def each(options = {}) - if block_given? - if options.empty? - 1.upto(last_row) do |line| - yield row(line) - end - else - clean_sheet_if_need(options) - search_or_set_header(options) - headers = @headers || - Hash[(first_column..last_column).map do |col| - [cell(@header_line, col), col] - end] - - @header_line.upto(last_row) do |line| - yield(Hash[headers.map { |k, v| [k, cell(line, v)] }]) - end + return to_enum(:each, options) unless block_given? + + if options.empty? + 1.upto(last_row) do |line| + yield row(line) end else - to_enum(:each, options) + clean_sheet_if_need(options) + search_or_set_header(options) + headers = @headers || + Hash[(first_column..last_column).map do |col| + [cell(@header_line, col), col] + end] + + @header_line.upto(last_row) do |line| + yield(Hash[headers.map { |k, v| [k, cell(line, v)] }]) + end end end @@ -413,19 +412,20 @@ def file_type_check(filename, exts, name, warning_level, packed = nil) filename = filename[0..qs_begin - 1] end exts = Array(exts) - if !exts.include?(File.extname(filename).downcase) - case warning_level - when :error - warn file_type_warning_message(filename, exts) - fail TypeError, "#{filename} is not #{name} file" - when :warning - warn "are you sure, this is #{name} spreadsheet file?" - warn file_type_warning_message(filename, exts) - when :ignore - # ignore - else - fail "#{warning_level} illegal state of file_warning" - end + + return if exts.include?(File.extname(filename).downcase) + + case warning_level + when :error + warn file_type_warning_message(filename, exts) + fail TypeError, "#{filename} is not #{name} file" + when :warning + warn "are you sure, this is #{name} spreadsheet file?" + warn file_type_warning_message(filename, exts) + when :ignore + # ignore + else + fail "#{warning_level} illegal state of file_warning" end end @@ -476,9 +476,9 @@ def local_filename(filename, tmpdir, packed) return if is_stream?(filename) filename = download_uri(filename, tmpdir) if uri?(filename) filename = unzip(filename, tmpdir) if packed == :zip - unless File.file?(filename) - fail IOError, "file #{filename} does not exist" - end + + fail IOError, "file #{filename} does not exist" unless File.file?(filename) + filename end @@ -537,10 +537,11 @@ def reinitialize def make_tmpdir(prefix = nil, root = nil, &block) prefix = if prefix - TEMP_PREFIX + prefix - else - TEMP_PREFIX - end + TEMP_PREFIX + prefix + else + TEMP_PREFIX + end + ::Dir.mktmpdir(prefix, root || ENV['ROO_TMP'], &block).tap do |result| block_given? || track_tmpdir!(result) end @@ -588,9 +589,9 @@ def normalize(row, col) fail ArgumentError end end - if col.is_a?(::String) - col = ::Roo::Utils.letter_to_number(col) - end + + col = ::Roo::Utils.letter_to_number(col) if col.is_a?(::String) + [row, col] end @@ -641,7 +642,7 @@ def validate_sheet!(sheet) fail RangeError, "sheet index #{sheet} not found" end when String - unless sheets.include? sheet + unless sheets.include?(sheet) fail RangeError, "sheet '#{sheet}' not found" end else @@ -670,14 +671,14 @@ def process_zipfile_packed(zip, tmpdir, path = '') # parameter is nil the output goes to STDOUT def write_csv_content(file = nil, sheet = nil, separator = ',') file ||= STDOUT - if first_row(sheet) # sheet is not empty - 1.upto(last_row(sheet)) do |row| - 1.upto(last_column(sheet)) do |col| - file.print(separator) if col > 1 - file.print cell_to_csv(row, col, sheet) - end - file.print("\n") - end # sheet not empty + return unless first_row(sheet) # The sheet is empty + + 1.upto(last_row(sheet)) do |row| + 1.upto(last_column(sheet)) do |col| + file.print(separator) if col > 1 + file.print cell_to_csv(row, col, sheet) + end + file.print("\n") end end @@ -729,9 +730,9 @@ def cell_to_csv(row, col, sheet) # converts an integer value to a time string like '02:05:06' def integer_to_timestring(content) h = (content / 3600.0).floor - content = content - h * 3600 + content -= h * 3600 m = (content / 60.0).floor - content = content - m * 60 + content -= m * 60 s = content sprintf('%02d:%02d:%02d', h, m, s) end From 99bd965e68d7703409d0de1cda1938f5d1b17a33 Mon Sep 17 00:00:00 2001 From: Steven Daniels Date: Thu, 9 Jul 2015 07:12:26 +0800 Subject: [PATCH 05/14] Linting and minor refactoring Roo::Base --- lib/roo/base.rb | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/lib/roo/base.rb b/lib/roo/base.rb index 6a7029e0..eff444c3 100644 --- a/lib/roo/base.rb +++ b/lib/roo/base.rb @@ -226,7 +226,7 @@ def cell_type_by_value(value) when Fixnum then :float when String, Float then :string else - raise ArgumentError, "Type for #{value} not set" + fail ArgumentError, "Type for #{value} not set" end end @@ -408,7 +408,7 @@ def file_type_check(filename, exts, name, warning_level, packed = nil) filename = File.basename(filename, File.extname(filename)) end - if uri?(filename) && qs_begin = filename.rindex('?') + if uri?(filename) && (qs_begin = filename.rindex('?')) filename = filename[0..qs_begin - 1] end exts = Array(exts) @@ -536,11 +536,7 @@ def reinitialize end def make_tmpdir(prefix = nil, root = nil, &block) - prefix = if prefix - TEMP_PREFIX + prefix - else - TEMP_PREFIX - end + prefix = "#{TEMP_PREFIX}#{prefix}" ::Dir.mktmpdir(prefix, root || ENV['ROO_TMP'], &block).tap do |result| block_given? || track_tmpdir!(result) From 9cb6b6898cf00f7332572b2ed8869df17ba45b2b Mon Sep 17 00:00:00 2001 From: Pablo Herrero Date: Fri, 10 Jul 2015 23:21:05 -0300 Subject: [PATCH 06/14] Convert generic spreadsheet tests to rspec --- spec/helpers.rb | 5 + spec/lib/roo/base_spec.rb | 212 +++++++++++++++++++++++++++ spec/spec_helper.rb | 7 +- test/test_generic_spreadsheet.rb | 237 ------------------------------- test/test_helper.rb | 4 - 5 files changed, 223 insertions(+), 242 deletions(-) create mode 100644 spec/helpers.rb delete mode 100644 test/test_generic_spreadsheet.rb diff --git a/spec/helpers.rb b/spec/helpers.rb new file mode 100644 index 00000000..f1406595 --- /dev/null +++ b/spec/helpers.rb @@ -0,0 +1,5 @@ +module Helpers + def yaml_entry(row,col,type,value) + "cell_#{row}_#{col}: \n row: #{row} \n col: #{col} \n celltype: #{type} \n value: #{value} \n" + end +end diff --git a/spec/lib/roo/base_spec.rb b/spec/lib/roo/base_spec.rb index 4e51aaf7..5c9b7183 100644 --- a/spec/lib/roo/base_spec.rb +++ b/spec/lib/roo/base_spec.rb @@ -1,4 +1,216 @@ require 'spec_helper' describe Roo::Base do + let(:klass) do + Class.new(Roo::Base) do + def initialize(filename, data = {}) + super(filename) + @data ||= data + end + + def read_cells(sheet = default_sheet) + return if @cells_read[sheet] + type_map = { String => :string, Date => :date, Numeric => :float } + + @cell[sheet] = @data + @cell_type[sheet] = Hash[@data.map { |k, v| [k, type_map.find {|type,_| v.is_a?(type) }.last ] }] + @first_row[sheet] = @data.map { |k, _| k[0] }.min + @last_row[sheet] = @data.map { |k, _| k[0] }.max + @first_column[sheet] = @data.map { |k, _| k[1] }.min + @last_column[sheet] = @data.map { |k, _| k[1] }.max + @cells_read[sheet] = true + end + + def cell(row, col, sheet = nil) + sheet ||= default_sheet + read_cells(sheet) + @cell[sheet][[row, col]] + end + + def celltype(row, col, sheet = nil) + sheet ||= default_sheet + read_cells(sheet) + @cell_type[sheet][[row, col]] + end + + def sheets + ['my_sheet', 'blank sheet'] + end + end + end + + let(:spreadsheet_data) do + { + [5, 1] => Date.civil(1961, 11, 21), + + [8, 3] => 'thisisc8', + [8, 7] => 'thisisg8', + + [12, 1] => 41.0, + [12, 2] => 42.0, + [12, 3] => 43.0, + [12, 4] => 44.0, + [12, 5] => 45.0, + + [15, 3] => 43.0, + [15, 4] => 44.0, + [15, 5] => 45.0, + + [16, 2] => '"Hello world!"', + [16, 3] => 'forty-three', + [16, 4] => 'forty-four', + [16, 5] => 'forty-five' + } + end + + let(:spreadsheet) { klass.new('some_file', spreadsheet_data) } + + describe '#uri?' do + it 'should return true when passed a filename starting with http(s)://' do + expect(spreadsheet.send(:uri?, 'http://example.com/')).to be_truthy + expect(spreadsheet.send(:uri?, 'https://example.com/')).to be_truthy + end + + it 'should return false when passed a filename which does not start with http(s)://' do + expect(spreadsheet.send(:uri?, 'example.com')).to be_falsy + end + + it 'should return false when passed non-String object such as Tempfile' do + expect(spreadsheet.send(:uri?, Tempfile.new('test'))).to be_falsy + end + end + + describe '#set' do + it 'should not update cell when setting an invalid type' do + spreadsheet.set(1, 1, 1) + expect { spreadsheet.set(1, 1, :invalid_type) }.to raise_error(ArgumentError) + expect(spreadsheet.cell(1, 1)).to eq(1) + expect(spreadsheet.celltype(1, 1)).to eq(:float) + end + end + + describe '#first_row' do + it 'should return the first row' do + expect(spreadsheet.first_row).to eq(5) + end + end + + describe '#last_row' do + it 'should return the last row' do + expect(spreadsheet.last_row).to eq(16) + end + end + + describe '#first_column' do + it 'should return the first column' do + expect(spreadsheet.first_column).to eq(1) + end + end + + describe '#first_column_as_letter' do + it 'should return the first column as a letter' do + expect(spreadsheet.first_column_as_letter).to eq('A') + end + end + + describe '#last_column' do + it 'should return the last column' do + expect(spreadsheet.last_column).to eq(7) + end + end + + describe '#last_column_as_letter' do + it 'should return the last column as a letter' do + expect(spreadsheet.last_column_as_letter).to eq('G') + end + end + + describe '#row' do + it 'should return the specified row' do + expect(spreadsheet.row(12)).to eq([41.0, 42.0, 43.0, 44.0, 45.0, nil, nil]) + expect(spreadsheet.row(16)).to eq([nil, '"Hello world!"', 'forty-three', 'forty-four', 'forty-five', nil, nil]) + end + end + + describe '#empty?' do + it 'should return true when empty' do + expect(spreadsheet.empty?(1, 1)).to be_truthy + expect(spreadsheet.empty?(8, 3)).to be_falsy + expect(spreadsheet.empty?('A', 11)).to be_truthy + expect(spreadsheet.empty?('A', 12)).to be_falsy + end + end + + describe '#reload' do + it 'should return reinitialize the spreadsheet' do + spreadsheet.reload + expect(spreadsheet.instance_variable_get(:@cell).empty?).to be_truthy + end + end + + describe '#each' do + it 'should return an enumerator with all the rows' do + each = spreadsheet.each + expect(each).to be_a(Enumerator) + expect(each.to_a.last).to eq([nil, '"Hello world!"', 'forty-three', 'forty-four', 'forty-five', nil, nil]) + end + end + + describe '#to_yaml' do + it 'should convert the spreadsheet to yaml' do + expect(spreadsheet.to_yaml({}, 5, 1, 5, 1)).to eq("--- \n" + yaml_entry(5, 1, 'date', '1961-11-21')) + expect(spreadsheet.to_yaml({}, 8, 3, 8, 3)).to eq("--- \n" + yaml_entry(8, 3, 'string', 'thisisc8')) + + expect(spreadsheet.to_yaml({}, 12, 3, 12, 3)).to eq("--- \n" + yaml_entry(12, 3, 'float', 43.0)) + + expect(spreadsheet.to_yaml({}, 12, 3, 12)).to eq( + "--- \n" + yaml_entry(12, 3, 'float', 43.0) + + yaml_entry(12, 4, 'float', 44.0) + + yaml_entry(12, 5, 'float', 45.0)) + + expect(spreadsheet.to_yaml({}, 12, 3)).to eq( + "--- \n" + yaml_entry(12, 3, 'float', 43.0) + + yaml_entry(12, 4, 'float', 44.0) + + yaml_entry(12, 5, 'float', 45.0) + + yaml_entry(15, 3, 'float', 43.0) + + yaml_entry(15, 4, 'float', 44.0) + + yaml_entry(15, 5, 'float', 45.0) + + yaml_entry(16, 3, 'string', 'forty-three') + + yaml_entry(16, 4, 'string', 'forty-four') + + yaml_entry(16, 5, 'string', 'forty-five')) + end + end + + let(:expected_csv) do + < Date.civil(1961, 11, 21).to_s, - - [8, 3] => 'thisisc8', - [8, 7] => 'thisisg8', - - [12, 1] => 41.0, - [12, 2] => 42.0, - [12, 3] => 43.0, - [12, 4] => 44.0, - [12, 5] => 45.0, - - [15, 3] => 43.0, - [15, 4] => 44.0, - [15, 5] => 45.0, - - [16, 2] => '"Hello world!"', - [16, 3] => 'dreiundvierzig', - [16, 4] => 'vierundvierzig', - [16, 5] => 'fuenfundvierzig' - } - end - - def set_sheet_types(workbook) - workbook.instance_variable_get(:@cell_type)[workbook.default_sheet] = { - [5, 1] => :date, - - [8, 3] => :string, - [8, 7] => :string, - - [12, 1] => :float, - [12, 2] => :float, - [12, 3] => :float, - [12, 4] => :float, - [12, 5] => :float, - - [15, 3] => :float, - [15, 4] => :float, - [15, 5] => :float, - - [16, 2] => :string, - [16, 3] => :string, - [16, 4] => :string, - [16, 5] => :string - } - end - - def set_first_row(workbook) - row_hash = workbook.instance_variable_get(:@first_row) - row_hash[workbook.default_sheet] = workbook.instance_variable_get(:@cell)[workbook.default_sheet].map { |k, _v| k[0] }.min - end - - def set_last_row(workbook) - row_hash = workbook.instance_variable_get(:@last_row) - row_hash[workbook.default_sheet] = workbook.instance_variable_get(:@cell)[workbook.default_sheet].map { |k, _v| k[0] }.max - end - - def set_first_col(workbook) - col_hash = workbook.instance_variable_get(:@first_column) - col_hash[workbook.default_sheet] = workbook.instance_variable_get(:@cell)[workbook.default_sheet].map { |k, _v| k[1] }.min - end - - def set_last_col(workbook) - col_hash = workbook.instance_variable_get(:@last_column) - col_hash[workbook.default_sheet] = workbook.instance_variable_get(:@cell)[workbook.default_sheet].map { |k, _v| k[1] }.max - end - - def set_cells_read(workbook) - read_hash = workbook.instance_variable_get(:@cells_read) - read_hash[workbook.default_sheet] = true - end - - def expected_csv - < Date: Thu, 23 Jul 2015 11:12:43 +0800 Subject: [PATCH 07/14] Added specs for fixing _x000D_ issue --- spec/lib/roo/excelx_spec.rb | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/spec/lib/roo/excelx_spec.rb b/spec/lib/roo/excelx_spec.rb index eb6135af..0eb6dee6 100644 --- a/spec/lib/roo/excelx_spec.rb +++ b/spec/lib/roo/excelx_spec.rb @@ -454,4 +454,11 @@ end end end + + describe '_x000D_' do + let(:path) { 'test/files/x000D.xlsx' } + it 'does not contain _x000D_' do + expect(subject.cell(2, 9)).not_to include('_x000D_') + end + end end From 2a1a350c5f4c6d1523ef0db1fd7a64c94ab6b962 Mon Sep 17 00:00:00 2001 From: Steven Daniels Date: Thu, 23 Jul 2015 11:16:33 +0800 Subject: [PATCH 08/14] Added specs for fixing _x000D_ issue --- test/files/x000D.xlsx | Bin 0 -> 9947 bytes 1 file changed, 0 insertions(+), 0 deletions(-) create mode 100644 test/files/x000D.xlsx diff --git a/test/files/x000D.xlsx b/test/files/x000D.xlsx new file mode 100644 index 0000000000000000000000000000000000000000..921fc65b764391bd2c2b65b552d59097d3378262 GIT binary patch literal 9947 zcmeHtWmFwW*X|*>Yw+Mskb{Ka?(Po3-3fNkAi+JjI|K;9T@&1b1Pc({-Tj_qGH+&> z`R=;Ezx(!CtNV0U?Wek{o?Uy_-lZT7355lK0l)zO08&72?hu;`1ONa94FF&Q<{`9& z?QES*Y@PL0-0e-AbQ#@jtVwcaAZW4xPr&v6d;D+ffl`$|+fHV*7WHj@kyf=&qr$4H zP$;^Q3>c@7GCZW%kv8ry2tJFFN(#m16j+QQm4kEZlq*~1m3`P`_y;UNskdYhi6EPA zbeNf8bsf19(edN`9iE3`Q<`P;u^Ae`w7`Rr~JU(I!~vGPaV5)-jYIxNe5-+DEeH*HvL|h z0ZPPXPL1SL*+9iWJyOc3>nun+}g&)~3w6NJ@)tc~J}!g~guvTpP? zddRPS`Jq`GpY(^fx6f`P=JQwT!@h~vd*Zzv{yJ{vdX)eGczA#WDEv(aYgC!Z z&%nfz0jn4yU>HK*(Zt$`iSfsoB&Ywsy7#|E=fY!0zE(Itk`d0{v{5t z$mCo68gEb=kpctHWQQdjEC{xQ@0#bTvW`vG&W&l^%P>(cTp$CJd%$H)*c7pNWUeSh zXh72c=}Fo+dfyG15o89{td9KYL+wikGU<>}iSH*oi=j>eJ3D@indZfpEyFy^g#k;l z#+L>ipHWrx!Nc}XuKV%DC!T^!8o^x02f#tNSu_1XcUL<{D?>Xws~;BU7xkfjP#K7VXolHZN2Z0u{yd#h+;E8Ww#uzC_v* zK#7SW^r4J(+8`U8Phnnl7P{%D{V;c|ov^mp*Bck{z70Iu|KyB6t?9WQxP$=A8B{PC zz?}IjTf!B2!CHkoA5-tYmd*t)3#A$-1&5}KCTwAb>0?pKORKMMYc1_<5;VQtA_pN+Th~>o)YvH)v#vLYNiLyiWjoqhoD1}RFIv+;aj&iH#g7GWgeTNHKOeo5fteV$| z+v5V&1zy3U&&#uMQNY1P`ewl!g}@{-u4H~}v3&@fvYFttmuU%=9Ctp5106J6EPygn zeLR5Z8)E{(p$L%8W6(jEeX7L^& ztnWs+)P+zGfkq^xx$x}pB%(Pb3DLm2Vg@*xVhGMRhJf!PI)|al45Xy=(HotX2!>n- zHW|`_Yl;X^>AtJtp{qTyH7>?!PY(*QkixHH_1k0hD`LWm^zeheo(L;ASy{nZZdxVg zu6otB)H!gPu4iapFOit8tCyL+YkrR#F)kRsimLq%gOktPXO{TWEQsD1JblOa4WE;k z?wj>(`)7kXY znEYe)jT-{x8uOmrY!RHfMZ0v~D4TwH>iehknd$miq7E(@wO)mH)6cuOJt|`omgGJj zu$a2bem)241#y2y`T0M?xEaYcly@M-q|NYULmY5BRRjJl8>7_c6TJXS+L0xh*id8@ zf4bddJMr(sUJ(f4+=kX_uBEr)C$Hv~z<#)uS~ zmwF2bl+4t*)vI2p_jXIF=O>T3);DDnmer-^)!%%m?I<3|JYPQ4!yQVyf3OnhIdZCT zIe*&MLY2RJi3=Jooj*-;x{6;7+vA{aDL0W%t6@D0=7#OywB0SP`g*0GYFWNs`gTth z9WvnI8p{Ztbk(>)Y1xNx=RT}{rz(cvz;=j&5+%_!I9bl>p zflJAM&puA(CMM2Kzl_(<8{mITS3p!pXeTq4=mq4xup21uYao)aG6z|`>IS6Y^(@R> zV)O;p9jJ=XqJ3jZfhkJN^NGeMfvZM1<`svaS%x;jk1w~sVZk%-L>r7V zwx&SJ$w`IuL~A@n{1~W6afjRG{b}Z7Bz$o+-};PXh&yxNRT#l&X^ExHu|8}ylU&Wg z^Hnc(e=c4n&HLR*FJd0s+_qtfE>E1?;_I{?P-@<|Z~KOh3;*KYLeG){a0Fx0U>eSPW!mL+yeu{<>uP zBZ2w2*hlS&6*u06bPpFZc2>D<0?Q}~iQqRLf45^<9irBxP06u#klblS;<`TAlFhilRlJAiSP)*bgASlX^k-bgPRc0mog)(2ZlAtP_DqQ~?j(6V8)d zO|w{SiVnTt<6A>Qh`H1i;2FHf{fazOo5!k+>9-~%kxx?F(3v5v>#~Fx{?BAB@kz}p zyP&w*OV|9e(D@0YLH)u=($Gv=dXt{b{p)K83lbqoT0+`cJIV4=i5THj^kM~YK@Fo? z9~PRc(@!`61=tSrwsqrWYqv;%8$J!o*R;o_7hkRR!p$Iz+Zd1cRR|^JDv`lW5a=JlQUe-W+9w<4L?JpzK7_((O z&w4EYYKaG-;5pSQBk03kdawtMC`Kc@yI7zb5xqBQz+P(7IR#|-A<^TLvrG5l-b8ml zBSeP&tOZygsXy#yymB;<91t4AVN+x{xPuaw9K34rKHu9hlONqNi;CLk=WTrG`r;2O ze$iIfOxXF15LUEPUEk;U28TFjF~jp}d&z9gP4m;k@x1tm&wVu^+*7ux&CeTAK3>&7(0Vn8_HYZ`mFW@!yq)lGoiATxbe zO~_93+>;b>yuY$;@E4UelMANcXQL$blud2& zM}&I8CSiXDp=!YY34U;zU7i)*xSuPUW(i~1#k3*5uDdI(e7CP6djzSLSfbSBs+aM@ z_S&v_bQMrr?$vN+a+d1Q_v%hHrhJ)>T@G}$ajA4It)U`Qt#on*4m`tpbNuJ)3jwrH z9lKBT^!!;Hv60O%2YV%M=55cOjpxOFW5&Oih!tq1fw-E4=lYr?((OqAdvqMhk-6-xUDD zYAD`6NI`z{(1V$l@quJenxP$zbk05(Q6puWN2K5PUx6Q~4Cdh?+HS&-YG`r$-SOL3&+AK4!PpY&UT{RP)n;5f{ zWR=U5&I&sUObcAJv$xcNXaZC?<|%9-VO)70zHL;$3GyRy3_V*vjrKgKVLx=0=c`TX zps7Pm^`koRDL3pZx7Di@8@st_^o!_8oZat!&i>V5!nkR>IkHqk z{f#f4rB_c?4qc~F&+F_1&8&vxFb`O{xQT~#9OM`>3agY+xm#Hy%9c^(J>77gv>>bBDi`EQQeA3FzQX0bDL~1(EE5rKyn+RrrGI*>gT@k7Xkc$u z3mn`;{=;xNIlEh%{B%>RRfcRQn9;x{3nk%zt!Yk4F+OkGgpwjUx=}*M43qlfYG!YS zZfr(_!zyXL!+v00Hbzv(%bDBfma5sEl*xFY*5r}J=_8Zer*nuK`Rwre8P8v0QdKpu znRN17e`V5hSVxyZ>Bo;bRCDuA=JUY@S--0<4-gPT>!xlND%BT{8KKMW$eh-~)uJsO z_g!sdIj+HszKPCLt=4)zOpZKu$i`SIIKtAznsittOcHU2Jm;U&ip?Ws1gUj;|1K^| zs0hHIItQyxiUyrWWGB`3a=7*8er+S_18IWlcOKwoSXu^e@Sz#x1D2uO@@Y&hmv;u7 zRDL(4b|7S~0REIOO`2w07s`1-nIwns_StTuSJW=!v+9A#0ZpNar!p&LuQ_E0NG4ivLKb40R5S3bb_IglM5qA_E$RlJy6M>D8)#k?+OslJc~&ll=q)yy=~)WpIYy7li1osMi*ei1Nj7O(*T+<$m|zx}=NBweRjHMF1u?(v7L zFJ8#k)k6S0qp6SM6hogZoEW+w*dU&%a@D5ZUhiM|Hq(D)ZSi-liHc0<37^?Y^|@%; z)O_t#ma3VS7O^71F5Oa+I`sU0XJF3`$6+G1L{pV}zDV;{_v|S3>O-51zVk)dBAp2t z8Qb0S1^jV2v0U#4)teIj@iNPz%CGaSET^?|;n%gwll|;_uDQ-UjN&&XwubUa&JS<1 zr*=GwZBLIaKH#sArq8Md4KLd5)JAQLtID$92q+eQt-7R3_Iy6z#+KQr{h<3kkw{Ox zz{694yN1nD6f>Co#yCpLrwV65nT^U=o1LyCp}<=$b93#v_eA&R-G)aMyRAS_#3T%2k7VqXtx}dpOHos(U<3tRd}%~dq5Y=5Ks!vzz}*YeJG6>>q5V4G&RgHJM8~!WO@64-2c2IJ z<_=WQwwd_}s=VPe9`e808GS{C*pc7V*5mRWz=ObyBq0_J7~*__5n15@ZzR#|@*SOu z@F+9{K50Wm?^P%IhhB-r^e$Mx0CK$}AjCr_Ey+@_dc2H4hp%q{`Dp*?7#FG<11{?oi9jbj4JBH8yMS*1rZKfLGcy{vXr>Pf2?1j7Z)zt{>`p23Jmx|(@40J{r!IQ!T(q#T%lL4F^MzeM-^;pIm1r) z%DB%VaY;WgAYiUJU;TIh{U<8KFH}w6U#QrBHvIQ0;R^l#K~4CDy7mjzeVKv^{9>AK z{Jdj-HUs|)RI;g|gNZ?gr^BxQHpl8iV?-rE4WHI_VNfj{&Z*T#Qc%7=SHrP%o1j6A zEcWU91Q)C>p;(2_Hj{F6YsN8+$jcS68S>bt{HX#1N+mkM?*{tbSYvufaryC|Zn}}r z!qjSOwWN4RzHhIw4WE~c^;`!@bN7hp?@;a9F;wntci+x5zBaGk+Mc{=R6O-h!;d^5 z!WtTUmR0s*TX@oDyCBNN(#EJsBb=+v^wcigR<}*xy)jXHc7;xhKGFhK%xrA^usg#y z$@y*J3(NOvuTjrXxv1{k%dJjCeah5Lt?c#}iO!Qx^<3E*oac#LvFjRHbyW$t`tk~T z_V(3nd+%H8_Jb5_-g691&02S12AJVL;}}1A_*bJ%<-_$d6TH?I1+O@%|Co90!LcQ0 z6GvqeXXhV5CZ`Vf}!NrpAbv`gtFM5R8%e5UG$(Zy-M+qT&E3~?*T{+N9nhe1SQ zMS^eJDNQ$h$%R4#Do`K`^Y*@!_PQ5$khaKA=jf1!nr%6tWt%k>n4!+g;n+@%-*_SA zk}N6rr8x{@GS#n{TiIW!1XQu1p*#1Ov!`wbD^L_1a>}5?Zw0NychzQ%EJy2iRhBWz z?8B~f$POBW1W!)mKpZ?QWU^e zcB>M2-BFxIksRA||8lJ7!J1BuCd+uPFX;&}0=M-pjvl@3*>M0pL-L@_?O~2iLS|T4 z#AIaWX!1wS`M<$`F!urB zF^ckCBv^rqvbU(n2Mo^RWuy$!6T5wsRKw<;$9w_9KX-*_dFMJS_4t z@A?8*3BPOzWGc5bGtT>S82i}=&QW?Zxw6iM?$n_A4QtxS<;?|mcKoLDfkq2r^7WCU zf$jT`{OqdhqwfHpQ0~~iy;UuB%oSMJogug=qdl_Aj?ak_3xp&RpmMpU3pQyrJNSb!QJb9te%hn~k?sO#eEgL?XN; z4g|C98<=fqe`TA2z5Rd5_Jd}B9BE1&mYo7v%{bSn1_zBH@k+*3O7Y9t3LTqXsm+^X z^#m?cS-iEU%N(a;!r87@^;OG_nS5R@}&FjH;W9uq$AzF+b46 zu$XFaS~a2~+=xg>JZKyq&Yz$h6lq{8Ac8hIMl7tt^@8<8e_SHSou3zJy_i8JWN26T zrRulAVrl!=ly}jTqqUCsRU(ibAYW&{wZctcIU8lNg((_-G2zg`9uU8`!36&pjo3uS zfhrt%z+_EnP_iT{W$)qB}-Ts@*;D!Y(&ZQbBv?~`bQ zcV}F0iAZ!oeD7AN_JEk=!7eFAZ*&T_Dm+UktR{4C&~-ZC7Wk#F!3)$sR@J3etZE`1|gHKLvkW10Vr_zw9-5Eco|S?Oz4$!KV3tPTW4md7Qrc zjg$x8gY+nw_gMIG!s)j#6T(m7f25xtBRr1Z|3)AKclod8|8LR!$AFK+;lBYXz*#Xc z;N#%2Fb7oW~#Ydy)DW<*|$P8zq+TC(0uS>t7wN z$Dn_=>AxWX02d+v;2#$LvH0I7>A#AvlKe&d*OaXw4GrcX0DuJkQGsck`6Dv}_&<1+ BE7Je~ literal 0 HcmV?d00001 From caacf36fb65fba2ab76bfed0bf1058f81a33088e Mon Sep 17 00:00:00 2001 From: Steven Daniels Date: Thu, 23 Jul 2015 11:38:26 +0800 Subject: [PATCH 09/14] Updated Changelog --- CHANGELOG.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 86a9484a..44256ea9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,6 @@ +## [Unreleased] - Unreleased +### Fixed invalid new lines with _x000D_ character[231](https://github.com/roo-rb/roo/pull/231) + ## [2.1.0] - 2015-07-18 ### Added - Added support for Excel 2007 `xlsm` files. [#232](https://github.com/roo-rb/roo/pull/232) From 0e79707a873466fe738f4465b0ddfa14f9eccded Mon Sep 17 00:00:00 2001 From: Steven Daniels Date: Sun, 26 Jul 2015 11:14:21 +0800 Subject: [PATCH 10/14] Updated readme --- README.md | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/README.md b/README.md index 2f091709..5f51a277 100644 --- a/README.md +++ b/README.md @@ -3,14 +3,11 @@ [![Build Status](https://img.shields.io/travis/roo-rb/roo.svg?style=flat-square)](https://travis-ci.org/roo-rb/roo) [![Code Climate](https://img.shields.io/codeclimate/github/roo-rb/roo.svg?style=flat-square)](https://codeclimate.com/github/roo-rb/roo) [![Coverage Status](https://img.shields.io/coveralls/roo-rb/roo.svg?style=flat-square)](https://coveralls.io/r/roo-rb/roo) [![Gem Version](https://img.shields.io/gem/v/roo.svg?style=flat-square)](https://rubygems.org/gems/roo) Roo implements read access for all common spreadsheet types. It can handle: - -* Excelx -* OpenOffice / LibreOffice +* Excel 2007 - 2013 formats (xlsx, xlsm) +* LibreOffice / OpenOffice.org formats (ods) * CSV - -## Additional Libraries - -In addition, the [roo-xls](https://github.com/roo-rb/roo-xls) and [roo-google](https://github.com/roo-rb/roo-google) gems exist to extend Roo to support reading classic Excel formats (i.e. `.xls` and ``Excel2003XML``) and read/write access for Google spreadsheets. +* Excel 97, Excel 2002 XML, and Excel 2003 XML formats when using the [roo-xls](https://github.com/roo-rb/roo-xls) gem (xls, xml) +* Google spreadsheets with read/write access when using [roo-google](https://github.com/roo-rb/roo-google) ## Installation From 47a7e60d686bf3effeae0cb6ff9076797033f945 Mon Sep 17 00:00:00 2001 From: Steven Daniels Date: Sun, 2 Aug 2015 07:02:54 +0800 Subject: [PATCH 11/14] Require URI. --- lib/roo/spreadsheet.rb | 2 ++ 1 file changed, 2 insertions(+) diff --git a/lib/roo/spreadsheet.rb b/lib/roo/spreadsheet.rb index aae511a2..1eef58d5 100644 --- a/lib/roo/spreadsheet.rb +++ b/lib/roo/spreadsheet.rb @@ -1,3 +1,5 @@ +require 'uri' + module Roo class Spreadsheet class << self From fa7feb79ff823e3a2238f1972f8b1c4a3a5e4e6e Mon Sep 17 00:00:00 2001 From: Steven Daniels Date: Sun, 2 Aug 2015 07:17:50 +0800 Subject: [PATCH 12/14] Roo Version 2.1.1 --- CHANGELOG.md | 5 +++-- lib/roo/version.rb | 2 +- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 44256ea9..cef37fb5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,6 @@ -## [Unreleased] - Unreleased -### Fixed invalid new lines with _x000D_ character[231](https://github.com/roo-rb/roo/pull/231) +## [2.1.1] - 2015-08-02 +### Fixed invalid new lines with _x000D_ character[#231](https://github.com/roo-rb/roo/pull/231) +### Fixed missing URI issue. [#245](https://github.com/roo-rb/roo/pull/245) ## [2.1.0] - 2015-07-18 ### Added diff --git a/lib/roo/version.rb b/lib/roo/version.rb index 8a38ad23..101b3f92 100644 --- a/lib/roo/version.rb +++ b/lib/roo/version.rb @@ -1,3 +1,3 @@ module Roo - VERSION = "2.1.0" + VERSION = "2.1.1" end From 76b606e4a1c171928f7410f3802474f3ff268216 Mon Sep 17 00:00:00 2001 From: Robert Eshleman Date: Thu, 13 Aug 2015 14:28:51 -0400 Subject: [PATCH 13/14] Add Passing Specs for `Roo::Base#row_with` * Test 1 case with matching row and 1 without a matching row --- spec/lib/roo/base_spec.rb | 21 +++++++++++++++++++-- 1 file changed, 19 insertions(+), 2 deletions(-) diff --git a/spec/lib/roo/base_spec.rb b/spec/lib/roo/base_spec.rb index 5c9b7183..213d0717 100644 --- a/spec/lib/roo/base_spec.rb +++ b/spec/lib/roo/base_spec.rb @@ -41,6 +41,8 @@ def sheets let(:spreadsheet_data) do { + [3, 1] => 'Header', + [5, 1] => Date.civil(1961, 11, 21), [8, 3] => 'thisisc8', @@ -91,7 +93,7 @@ def sheets describe '#first_row' do it 'should return the first row' do - expect(spreadsheet.first_row).to eq(5) + expect(spreadsheet.first_row).to eq(3) end end @@ -132,6 +134,21 @@ def sheets end end + describe '#row_with' do + context 'with a matching header row' do + it 'returns the row number' do + expect(spreadsheet.row_with([/Header/])). to eq 3 + end + end + + context 'without a matching header row' do + it 'raises an error' do + expect { spreadsheet.row_with([/Missing Header/]) }.to \ + raise_error("Couldn't find header row.") + end + end + end + describe '#empty?' do it 'should return true when empty' do expect(spreadsheet.empty?(1, 1)).to be_truthy @@ -185,7 +202,7 @@ def sheets < Date: Thu, 13 Aug 2015 14:47:30 -0400 Subject: [PATCH 14/14] Missing Header Raises `Roo::HeaderRowNotFoundError` Currently, Roo raises a generic `RuntimeError` with the message "Couldn't find header row" when a matching header row is not found. This behavior makes it difficult to rescue from the most specific error desired, because `RuntimeError` is the default error class for `raise`. Additionally, because Roo does not have its own library-level error class from which all errors inherit, it is difficult to differentiate Roo library errors from errors caused by the developer / user. From the [Ruby documentation][1]: > It is recommended that a library should have one subclass of > `StandardError` or `RuntimeError` and have specific exception types > inherit from it. This allows the user to rescue a generic exception > type to catch all exceptions the library may raise even if future > versions of the library add new exception subclasses. This commit adds a generic `Roo::Error` class which inherits from `StandardError` and a specific `Roo::HeaderRowNotFound` class that is raised when a matching header row could not be found. In the future, additional subclasses of `Roo::Error` can be added in order to allow users to more easily rescue the correct errors. Note: This is *not* a backwards-compatible change. Users who currently rescue `RuntimeError` for a missing header row will now see an error raised. [1]: http://ruby-doc.org/core-2.2.2/Exception.html --- lib/roo.rb | 1 + lib/roo/base.rb | 4 ++-- lib/roo/errors.rb | 9 +++++++++ spec/lib/roo/base_spec.rb | 2 +- spec/lib/roo/excelx_spec.rb | 2 +- 5 files changed, 14 insertions(+), 4 deletions(-) create mode 100644 lib/roo/errors.rb diff --git a/lib/roo.rb b/lib/roo.rb index bedcd5be..b93b3239 100644 --- a/lib/roo.rb +++ b/lib/roo.rb @@ -1,4 +1,5 @@ require 'roo/constants' +require 'roo/errors' require 'roo/spreadsheet' require 'roo/base' diff --git a/lib/roo/base.rb b/lib/roo/base.rb index eff444c3..25730cb8 100644 --- a/lib/roo/base.rb +++ b/lib/roo/base.rb @@ -392,10 +392,10 @@ def row_with(query, return_headers = false) @header_line = line_no return return_headers ? headers : line_no elsif line_no > 100 - fail "Couldn't find header row." + raise Roo::HeaderRowNotFoundError end end - fail "Couldn't find header row." + raise Roo::HeaderRowNotFoundError end protected diff --git a/lib/roo/errors.rb b/lib/roo/errors.rb new file mode 100644 index 00000000..fd3fc125 --- /dev/null +++ b/lib/roo/errors.rb @@ -0,0 +1,9 @@ +module Roo + # A base error class for Roo. Most errors thrown by Roo should inherit from + # this class. + class Error < StandardError; end + + # Raised when Roo cannot find a header row that matches the given column + # name(s). + class HeaderRowNotFoundError < Error; end +end diff --git a/spec/lib/roo/base_spec.rb b/spec/lib/roo/base_spec.rb index 213d0717..d00025d8 100644 --- a/spec/lib/roo/base_spec.rb +++ b/spec/lib/roo/base_spec.rb @@ -144,7 +144,7 @@ def sheets context 'without a matching header row' do it 'raises an error' do expect { spreadsheet.row_with([/Missing Header/]) }.to \ - raise_error("Couldn't find header row.") + raise_error(Roo::HeaderRowNotFoundError) end end end diff --git a/spec/lib/roo/excelx_spec.rb b/spec/lib/roo/excelx_spec.rb index 0eb6dee6..0162d311 100644 --- a/spec/lib/roo/excelx_spec.rb +++ b/spec/lib/roo/excelx_spec.rb @@ -82,7 +82,7 @@ this: 'This', that: 'That' ) - end.to raise_error("Couldn't find header row.") + end.to raise_error(Roo::HeaderRowNotFoundError) end end end