Merge pull request #240 from stevendaniels/proper-cell-types-for-excelx

Add Proper Type support to Excelx.

Guardfile

@@ -3,7 +3,7 @@
 
 guard :minitest, test_folders: ['test'] do
   watch(%r{^test/(.*)\/?test_(.*)\.rb$})
-  watch(%r{^lib/(.*/)?([^/]+)\.rb$})     { |m| "test/#{m[1]}test_#{m[2]}.rb" }
+  watch(%r{^lib/(.*/)?([^/]+)\.rb$})     { |m| "test/#{m[1].to_s.sub('roo/', '')}test_#{m[2]}.rb" }
   watch(%r{^test/test_helper\.rb$})      { 'test' }
 end
 
@@ -21,4 +21,3 @@ guard :rspec, cmd: 'bundle exec rspec' do
   watch('spec/spec_helper.rb')  { "spec" }
   watch(%r{^spec/support/(.+)\.rb$})                  { "spec" }
 end
-

lib/roo/base.rb

@@ -680,47 +680,47 @@ def write_csv_content(file = nil, sheet = nil, separator = ',')
 
   # The content of a cell in the csv output
   def cell_to_csv(row, col, sheet)
-    if empty?(row, col, sheet)
-      ''
-    else
-      onecell = cell(row, col, sheet)
-
-      case celltype(row, col, sheet)
-      when :string
+    return '' if empty?(row, col, sheet)
+
+    onecell = cell(row, col, sheet)
+
+    case celltype(row, col, sheet)
+    when :string
+      %("#{onecell.gsub('"', '""')}") unless onecell.empty?
+    when :boolean
+      # TODO: this only works for excelx
+      onecell = self.sheet_for(sheet).cells[[row, col]].formatted_value
+      %("#{onecell.gsub('"', '""').downcase}")
+    when :float, :percentage
+      if onecell == onecell.to_i
+        onecell.to_i.to_s
+      else
+        onecell.to_s
+      end
+    when :formula
+      case onecell
+      when String
         %("#{onecell.gsub('"', '""')}") unless onecell.empty?
-      when :boolean
-        %("#{onecell.gsub('"', '""').downcase}")
-      when :float, :percentage
+      when Float
         if onecell == onecell.to_i
           onecell.to_i.to_s
         else
           onecell.to_s
         end
-      when :formula
-        case onecell
-        when String
-          %("#{onecell.gsub('"', '""')}") unless onecell.empty?
-        when Float
-          if onecell == onecell.to_i
-            onecell.to_i.to_s
-          else
-            onecell.to_s
-          end
-        when DateTime
-          onecell.to_s
-        else
-          fail "unhandled onecell-class #{onecell.class}"
-        end
-      when :date, :datetime
+      when DateTime
         onecell.to_s
-      when :time
-        integer_to_timestring(onecell)
-      when :link
-        %("#{onecell.url.gsub('"', '""')}")
       else
-        fail "unhandled celltype #{celltype(row, col, sheet)}"
-      end || ''
-    end
+        fail "unhandled onecell-class #{onecell.class}"
+      end
+    when :date, :datetime
+      onecell.to_s
+    when :time
+      integer_to_timestring(onecell)
+    when :link
+      %("#{onecell.url.gsub('"', '""')}")
+    else
+      fail "unhandled celltype #{celltype(row, col, sheet)}"
+    end || ''
   end
 
   # converts an integer value to a time string like '02:05:06'

lib/roo/excelx.rb

@@ -17,6 +17,7 @@ class Excelx < Roo::Base
     require 'roo/excelx/relationships'
     require 'roo/excelx/comments'
     require 'roo/excelx/sheet_doc'
+    require 'roo/excelx/coordinate'
 
     delegate [:styles, :workbook, :shared_strings, :rels_files, :sheet_files, :comments_files] => :@shared
 
@@ -102,7 +103,6 @@ def initialize(filename_or_stream, options = {})
       @tmpdir = make_tmpdir(basename, options[:tmpdir_root])
       @shared = Shared.new(@tmpdir)
       @filename = local_filename(filename_or_stream, @tmpdir, packed)
-
       process_zipfile(@filename || filename_or_stream)
 
       @sheet_names = workbook.sheets.map do |sheet|
@@ -112,7 +112,6 @@ 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, @shared, n, sheet_options)
         [sheet_name, @sheets[n]]
       end]
@@ -266,8 +265,8 @@ def empty?(row, col, sheet = nil)
       sheet = sheet_for(sheet)
       key = normalize(row, col)
       cell = sheet.cells[key]
-      !cell || !cell.value || (cell.type == :string && cell.value.empty?) \
-      || (row < sheet.first_row || row > sheet.last_row || col < sheet.first_column || col > sheet.last_column)
+      !cell || cell.empty? || (cell.type == :string && cell.value.empty?) ||
+        (row < sheet.first_row || row > sheet.last_row || col < sheet.first_column || col > sheet.last_column)
     end
 
     # shows the internal representation of all cells
@@ -478,6 +477,20 @@ def process_zipfile_entries(entries)
       end
     end
 
+    # NOTE: To reduce memory, styles, shared_strings, workbook can be class
+    #       variables in a Shared module.
+    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

lib/roo/excelx/cell.rb

@@ -1,12 +1,22 @@
 require 'date'
+require 'roo/excelx/cell/base'
+require 'roo/excelx/cell/boolean'
+require 'roo/excelx/cell/datetime'
+require 'roo/excelx/cell/date'
+require 'roo/excelx/cell/empty'
+require 'roo/excelx/cell/number'
+require 'roo/excelx/cell/string'
+require 'roo/excelx/cell/time'
 
 module Roo
   class Excelx
     class Cell
       attr_reader :type, :formula, :value, :excelx_type, :excelx_value, :style, :hyperlink, :coordinate
       attr_writer :value
 
+      # DEPRECATED: Please use Cell.create_cell instead.
       def initialize(value, type, formula, excelx_type, excelx_value, style, hyperlink, base_date, coordinate)
+        warn '[DEPRECATION] `Cell.new` is deprecated.  Please use `Cell.create_cell` instead.'
         @type = type
         @formula = formula
         @base_date = base_date if [:date, :datetime].include?(@type)
@@ -29,10 +39,29 @@ def type
         end
       end
 
+      def self.create_cell(type, *values)
+        case type
+        when :string
+          Cell::String.new(*values)
+        when :boolean
+          Cell::Boolean.new(*values)
+        when :number
+          Cell::Number.new(*values)
+        when :date
+          Cell::Date.new(*values)
+        when :datetime
+          Cell::DateTime.new(*values)
+        when :time
+          Cell::Time.new(*values)
+        end
+      end
+
+      # Deprecated: use Roo::Excelx::Coordinate instead.
       class Coordinate
         attr_accessor :row, :column
 
         def initialize(row, column)
+          warn '[DEPRECATION] `Roo::Excel::Cell::Coordinate` is deprecated.  Please use `Roo::Excelx::Coordinate` instead.'
           @row, @column = row, column
         end
       end
@@ -57,20 +86,20 @@ def type_cast_value(value)
       def create_date(date)
         yyyy, mm, dd = date.strftime('%Y-%m-%d').split('-')
 
-        Date.new(yyyy.to_i, mm.to_i, dd.to_i)
+        ::Date.new(yyyy.to_i, mm.to_i, dd.to_i)
       end
 
       def create_datetime(date)
         datetime_string = date.strftime('%Y-%m-%d %H:%M:%S.%N')
         t = round_datetime(datetime_string)
 
-        DateTime.civil(t.year, t.month, t.day, t.hour, t.min, t.sec)
+        ::DateTime.civil(t.year, t.month, t.day, t.hour, t.min, t.sec)
       end
 
       def round_datetime(datetime_string)
         /(?<yyyy>\d+)-(?<mm>\d+)-(?<dd>\d+) (?<hh>\d+):(?<mi>\d+):(?<ss>\d+.\d+)/ =~ datetime_string
 
-        Time.new(yyyy.to_i, mm.to_i, dd.to_i, hh.to_i, mi.to_i, ss.to_r).round(0)
+        ::Time.new(yyyy.to_i, mm.to_i, dd.to_i, hh.to_i, mi.to_i, ss.to_r).round(0)
       end
     end
   end

lib/roo/excelx/cell/base.rb

@@ -0,0 +1,94 @@
+module Roo
+  class Excelx
+    class Cell
+      class Base
+        attr_reader :cell_type, :cell_value, :value
+
+        # FIXME: I think style should be deprecated. Having a style attribute
+        #        for a cell doesn't really accomplish much. It seems to be used
+        #        when you want to export to excelx.
+        attr_reader :style
+
+
+        # FIXME: Updating a cell's value should be able tochange the cell's type,
+        #        but that isn't currently possible. This will cause weird bugs
+        #        when one changes the value of a Number cell to a String. e.g.
+        #
+        #           cell = Cell::Number(*args)
+        #           cell.value = 'Hello'
+        #           cell.formatted_value # => Some unexpected value
+        #
+        #        Here are two possible solutions to such issues:
+        #        1. Don't allow a cell's value to be updated. Use a method like
+        #          `Sheet.update_cell` instead. The simple solution.
+        #        2. When `cell.value = ` is called, use injection to try and
+        #           change the type of cell on the fly. But deciding what type
+        #           of value to pass to `cell.value=`. isn't always obvious. e.g.
+        #           `cell.value = Time.now` should convert a cell to a DateTime,
+        #           not a Time cell. Time cells would be hard to recognize because
+        #           they are integers. This approach would require a significant
+        #           change to the code as written. The complex solution.
+        #
+        #        If the first solution is used, then this method should be
+        #        deprecated.
+        attr_writer :value
+
+        def initialize(value, formula, excelx_type, style, link, coordinate)
+          @link = !!link
+          @cell_value = value
+          @cell_type = excelx_type
+          @formula = formula
+          @style = style
+          @coordinate = coordinate
+          @type = :base
+          @value = link? ? Roo::Link.new(link, value) : value
+        end
+
+        def type
+          if formula?
+            :formula
+          elsif link?
+            :link
+          else
+            @type
+          end
+        end
+
+        def formula?
+          !!@formula
+        end
+
+        def link?
+          !!@link
+        end
+
+        alias_method :formatted_value, :value
+
+        def to_s
+          formatted_value
+        end
+
+        # DEPRECATED: Please use link instead.
+        def hyperlink
+          warn '[DEPRECATION] `hyperlink` is deprecated.  Please use `link` instead.'
+        end
+
+        # DEPRECATED: Please use cell_value instead.
+        def excelx_value
+          warn '[DEPRECATION] `excelx_value` is deprecated.  Please use `cell_value` instead.'
+          cell_value
+        end
+
+        # DEPRECATED: Please use cell_type instead.
+        def excelx_type
+          warn '[DEPRECATION] `excelx_type` is deprecated.  Please use `cell_type` instead.'
+          cell_type
+        end
+
+        def empty?
+          false
+        end
+      end
+    end
+  end
+end

lib/roo/excelx/cell/boolean.rb

@@ -0,0 +1,27 @@
+module Roo
+  class Excelx
+    class Cell
+      class Boolean < Cell::Base
+        attr_reader :value, :formula, :format, :cell_type, :cell_value, :link, :coordinate
+
+        def initialize(value, formula, style, link, coordinate)
+          super(value, formula, nil, style, link, coordinate)
+          @type = @cell_type = :boolean
+          @value = link? ? Roo::Link.new(link, value) : create_boolean(value)
+        end
+
+        def formatted_value
+          value ? 'TRUE'.freeze : 'FALSE'.freeze
+        end
+
+        private
+
+        def create_boolean(value)
+          # FIXME: Using a boolean will cause methods like Base#to_csv to fail.
+          #       Roo is using some method to ignore false/nil values.
+          value.to_i == 1 ? true : false
+        end
+      end
+    end
+  end
+end

lib/roo/excelx/cell/date.rb

@@ -0,0 +1,28 @@
+require 'date'
+
+module Roo
+  class Excelx
+    class Cell
+      class Date < Roo::Excelx::Cell::DateTime
+        attr_reader :value, :formula, :format, :cell_type, :cell_value, :link, :coordinate
+
+        def initialize(value, formula, excelx_type, style, link, base_date, coordinate)
+          # NOTE: Pass all arguments to the parent class, DateTime.
+          super
+          @type = :date
+          @format = excelx_type.last
+          @value = link? ? Roo::Link.new(link, value) : create_date(base_date, value)
+        end
+
+        private
+
+        def create_date(base_date, value)
+          date = base_date + value.to_i
+          yyyy, mm, dd = date.strftime('%Y-%m-%d').split('-')
+
+          ::Date.new(yyyy.to_i, mm.to_i, dd.to_i)
+        end
+      end
+    end
+  end
+end