Fix a bug where Formatted cells were lost.

This fixes the issue reported by Corey Martella in
http://rubyforge.org/forum/message.php?msg_id=63651
as well as other issues engendered by the decision to always shorten
Rows to the last non-nil value.

lib/spreadsheet/excel/worksheet.rb

@@ -36,11 +36,11 @@ def ensure_rows_read
     return if @row_addresses
     @dimensions = nil
     @row_addresses = []
-    @reader.read_worksheet self, @offset
+    @reader.read_worksheet self, @offset if @reader
   end
   def row idx
-    ensure_rows_read
     @rows[idx] or begin
+      ensure_rows_read
       if addr = @row_addresses[idx]
         row = @reader.read_row self, addr
         [:default_format, :height, :outline_level, :hidden, ].each do |key|
@@ -53,7 +53,7 @@ def row idx
       end
     end
   end
-  def row_updated idx, row, args={}
+  def row_updated idx, row
     res = super
     @workbook.changes.store self, true
     @workbook.changes.store :boundsheets, true
@@ -83,11 +83,11 @@ def recalculate_dimensions
                        index_of_first(@row_addresses) ].compact.min || 0
     @dimensions[1] = [ @rows.size, @row_addresses.size ].compact.max || 0
     compact = @rows.compact
-    first_rows = compact.collect do |row| index_of_first row end.compact.min
+    first_rows = compact.collect do |row| row.first_used end.compact.min
     first_addrs = @row_addresses.compact.collect do |addr|
       addr[:first_used] end.min
     @dimensions[2] = [ first_rows, first_addrs ].compact.min || 0
-    last_rows = compact.collect do |row| row.size end.max
+    last_rows = compact.collect do |row| row.first_unused end.max
     last_addrs = @row_addresses.compact.collect do |addr|
       addr[:first_unused] end.max
     @dimensions[3] = [last_rows, last_addrs].compact.max || 0

lib/spreadsheet/excel/writer/worksheet.rb

@@ -158,7 +158,7 @@ def write_calccount
     write_op 0x000c, [count].pack('v')
   end
   def write_cell type, row, idx, *args
-    xf_idx = @workbook.xf_index @worksheet.workbook, row.format(idx)
+    xf_idx = @workbook.xf_index @worksheet.workbook, row.formats[idx]
     data = [
       row.idx, # Index to row
       idx,     # Index to column
@@ -178,7 +178,7 @@ def write_cellblocks row
     # RK ➜ 6.82 (BIFF3-BIFF8)
     # RSTRING ➜ 6.84 (BIFF5/BIFF7)
     multiples, first_idx = nil
-    row.each_with_index do |cell, idx|
+    row.formatted.each_with_index do |cell, idx|
       cell = nil if cell == ''
       ## it appears that there are limitations to RK precision, both for
       #  Integers and Floats, that lie well below 2^30 significant bits, or
@@ -357,7 +357,7 @@ def write_eof
   # Write a cell with a Formula. May write an additional String record depending
   # on the stored result of the Formula.
   def write_formula row, idx
-    xf_idx = @workbook.xf_index @worksheet.workbook, row.format(idx)
+    xf_idx = @workbook.xf_index @worksheet.workbook, row.formats[idx]
     cell = row[idx]
     data1 = [
       row.idx,      # Index to row
@@ -556,7 +556,7 @@ def write_mulblank row, idx, multiples
     ]
     # List of nc=lc-fc+1 16-bit indexes to XF records (➜ 6.115)
     multiples.each_with_index do |blank, cell_idx|
-      xf_idx = @workbook.xf_index @worksheet.workbook, row.format(idx + cell_idx)
+      xf_idx = @workbook.xf_index @worksheet.workbook, row.formats[idx + cell_idx]
       data.push xf_idx
     end
     # Index to last column (lc)
@@ -573,7 +573,7 @@ def write_mulrk row, idx, multiples
     ]
     # List of nc=lc-fc+1 16-bit indexes to XF records (➜ 6.115)
     multiples.each_with_index do |cell, cell_idx|
-      xf_idx = @workbook.xf_index @worksheet.workbook, row.format(idx + cell_idx)
+      xf_idx = @workbook.xf_index @worksheet.workbook, row.formats[idx + cell_idx]
       data.push xf_idx, encode_rk(cell)
       fmt << 'vV'
     end

lib/spreadsheet/helpers.rb

@@ -0,0 +1,11 @@
+class Array
+  def rcompact
+    dup.rcompact!
+  end
+  def rcompact!
+    while !empty? && last.nil?
+      pop
+    end
+    self
+  end
+end

lib/spreadsheet/row.rb

@@ -1,3 +1,5 @@
+require 'spreadsheet/helpers'
+
 module Spreadsheet
   ##
   # The Row class. Encapsulates Cell data and formatting.
@@ -23,10 +25,7 @@ def format_updater *keys
             alias_method :"unupdated_#{key}=", :"#{key}="
             define_method "#{key}=" do |value|
               send "unupdated_#{key}=", value
-              if @worksheet
-                @formatted = true
-                @worksheet.row_updated @idx, self, :formatted => true
-              end
+              @worksheet.row_updated @idx, self if @worksheet
               value
             end
           end
@@ -36,9 +35,7 @@ def updater *keys
         keys.each do |key|
           define_method key do |*args|
             res = super
-            if @worksheet
-              @worksheet.row_updated @idx, self, :formatted => @formatted
-            end
+            @worksheet.row_updated @idx, self if @worksheet
             res
           end
         end
@@ -55,12 +52,7 @@ def updater *keys
     def initialize worksheet, idx, cells=[]
       @worksheet = worksheet
       @idx = idx
-      while !cells.empty? && !cells.last
-        cells.pop
-      end
       super cells
-      @first_used ||= index_of_first self
-      @first_unused ||= size
       @formats = []
       @height = 12
     end
@@ -69,17 +61,13 @@ def initialize worksheet, idx, cells=[]
     # stored for the cell.
     def default_format= format
       @worksheet.add_format format if @worksheet
-      @worksheet.row_updated @idx, self, :formatted => true if @worksheet
+      @worksheet.row_updated @idx, self if @worksheet
       @default_format = format
     end
     ##
-    # #first_unused (really last used + 1) - the 0-based index of the first of
-    # all remaining contiguous blank Cells.
-    alias :first_unused :size
-    ##
     # #first_used the 0-based index of the first non-blank Cell.
     def first_used
-      index_of_first self
+      [ index_of_first(self), index_of_first(@formats) ].compact.min
     end
     ##
     # The Format for the Cell at _idx_ (0-based), or the first valid Format in
@@ -89,22 +77,48 @@ def format idx
         || @worksheet.column(idx).default_format if @worksheet
     end
     ##
-    # Set the Format for the Cell at _idx_ (0-based).
-    def set_format idx, fmt
-      @formats[idx] = fmt
-      @worksheet.add_format fmt
-      @worksheet.row_updated @idx, self, :formatted => true if @worksheet
-      fmt
+    # Returns a copy of self with nil-values appended for empty cells that have
+    # an associated Format.
+    # This is primarily a helper-function for the writer classes.
+    def formatted
+      copy = dup
+      @formats.rcompact!
+      if copy.length < @formats.size
+        copy.concat Array.new(@formats.size - copy.length)
+      end
+      copy
+    end
+    ##
+    # Same as Row#size, but takes into account formatted empty cells
+    def formatted_size
+      @formats.rcompact!
+      sz = size
+      fs = @formats.size
+      fs > sz ? fs : sz
     end
+    ##
+    # #first_unused (really last used + 1) - the 0-based index of the first of
+    # all remaining contiguous blank Cells.
+    alias :first_unused :formatted_size
     def inspect
       variables = instance_variables.collect do |name|
         "%s=%s" % [name, instance_variable_get(name)]
       end.join(' ')
       sprintf "#<%s:0x%014x %s %s>", self.class, object_id, variables, super
     end
+    ##
+    # Set the Format for the Cell at _idx_ (0-based).
+    def set_format idx, fmt
+      @formats[idx] = fmt
+      @worksheet.add_format fmt
+      @worksheet.row_updated @idx, self if @worksheet
+      fmt
+    end
     private
     def index_of_first ary # :nodoc:
-      ary.index(ary.find do |elm| elm end)
+      if first = ary.find do |elm| !elm.nil? end
+        ary.index first
+      end
     end
   end
 end

lib/spreadsheet/worksheet.rb

@@ -173,11 +173,8 @@ def row_count
     ##
     # Tell Worksheet that the Row at _idx_ has been updated and the #dimensions
     # need to be recalculated. You should not need to call this directly.
-    def row_updated idx, row, opts={}
+    def row_updated idx, row
       @dimensions = nil
-      unless opts[:formatted]
-        row = shorten(row)
-      end
       @rows[idx] = row
       format_dates row
       row
@@ -188,7 +185,7 @@ def update_row idx, *cells
       res = if row = @rows[idx]
               row[0, cells.size] = cells
               row
-            elsif cells = shorten(cells)
+            else
               Row.new self, idx, cells
             end
       row_updated idx, res
@@ -251,8 +248,9 @@ def recalculate_dimensions # :nodoc:
       @dimensions[0] = index_of_first(@rows) || 0
       @dimensions[1] = @rows.size
       compact = @rows.compact
-      @dimensions[2] = compact.collect do |row| index_of_first row end.compact.min || 0
-      @dimensions[3] = compact.collect do |row| row.size end.max || 0
+      @dimensions[2] = compact.collect do |row| row.first_used end.compact.min || 0
+      @dimensions[3] = compact.collect do |row| row.first_unused end.max || 0
+      puts caller if @dimensions.nil?
       @dimensions
     end
     def shorten ary # :nodoc:

test/excel/row.rb

@@ -19,8 +19,8 @@ def test_date
     assert_equal Date.new(1975,8,21), row.date(1)
   end
   def test_datetime
-    row = Row.new @worksheet, 0, [nil, 27627.6789]
-    d1 = DateTime.new(1975,8,21) + 0.6789
+    row = Row.new @worksheet, 0, [nil, 27627.765]
+    d1 = DateTime.new(1975,8,21) + 0.765
     d2 = row.datetime 1
     assert_equal d1, d2
   end

test/integration.rb

@@ -63,7 +63,7 @@ def test_version_excel97__ooffice__utf16
         enc = Encoding.find enc
       end
       assert_equal enc, book.encoding
-      assert_equal 24, book.formats.size
+      assert_equal 25, book.formats.size
       assert_equal 5, book.fonts.size
       str1 = book.shared_string 0
       assert_equal @@iconv.iconv('Shared String'), str1
@@ -92,10 +92,10 @@ def test_version_excel97__ooffice__utf16
       end
       assert_equal long, str4
       sheet = book.worksheet 0
-      assert_equal 10, sheet.row_count
-      assert_equal 11, sheet.column_count
-      useds = [0,0,0,0,0,0,0,1,0,0]
-      unuseds = [2,2,1,1,1,2,1,11,1,2]
+      assert_equal 11, sheet.row_count
+      assert_equal 12, sheet.column_count
+      useds = [0,0,0,0,0,0,0,1,0,0,11]
+      unuseds = [2,2,1,1,1,2,1,11,1,2,12]
       sheet.each do |row|
         assert_equal useds.shift, row.first_used
         assert_equal unuseds.shift, row.first_unused
@@ -154,6 +154,7 @@ def test_version_excel97__ooffice__utf16
       assert_equal 0.0001, row[0]
       row = sheet.row 9
       assert_equal 0.00009, row[0]
+      assert_equal :green, sheet.row(10).format(11).pattern_fg_color
     end
     def test_version_excel97__ooffice
       path = File.join @data, 'test_version_excel97.xls'
@@ -166,7 +167,7 @@ def test_version_excel97__ooffice
         enc = Encoding.find enc
       end
       assert_equal enc, book.encoding
-      assert_equal 24, book.formats.size
+      assert_equal 25, book.formats.size
       assert_equal 5, book.fonts.size
       str1 = book.shared_string 0
       assert_equal 'Shared String', str1
@@ -195,10 +196,10 @@ def test_version_excel97__ooffice
       end
       assert_equal long, str4
       sheet = book.worksheet 0
-      assert_equal 10, sheet.row_count
-      assert_equal 11, sheet.column_count
-      useds = [0,0,0,0,0,0,0,1,0,0]
-      unuseds = [2,2,1,1,1,2,1,11,1,2]
+      assert_equal 11, sheet.row_count
+      assert_equal 12, sheet.column_count
+      useds = [0,0,0,0,0,0,0,1,0,0,11]
+      unuseds = [2,2,1,1,1,2,1,11,1,2,12]
       sheet.each do |row|
         assert_equal useds.shift, row.first_used
         assert_equal unuseds.shift, row.first_unused
@@ -577,10 +578,10 @@ def test_change_cell
       book.write path
       assert_nothing_raised do book = Spreadsheet.open path end
       sheet = book.worksheet 0
-      assert_equal 10, sheet.row_count
-      assert_equal 11, sheet.column_count
-      useds = [0,0,0,0,0,0,0,1,0,0]
-      unuseds = [2,2,1,1,1,2,1,11,1,2]
+      assert_equal 11, sheet.row_count
+      assert_equal 12, sheet.column_count
+      useds = [0,0,0,0,0,0,0,1,0,0,11]
+      unuseds = [2,2,1,1,1,2,1,11,1,2,12]
       sheet.each do |row|
         assert_equal useds.shift, row.first_used
         assert_equal unuseds.shift, row.first_unused
@@ -686,10 +687,10 @@ def test_change_cell__complete_sst_rewrite
       assert_equal str3, book.shared_string(2)
       assert_equal str4, book.shared_string(3)
       sheet = book.worksheet 0
-      assert_equal 10, sheet.row_count
-      assert_equal 11, sheet.column_count
-      useds = [0,0,0,0,0,0,0,1,0,0]
-      unuseds = [2,2,1,1,1,2,1,11,1,2]
+      assert_equal 11, sheet.row_count
+      assert_equal 12, sheet.column_count
+      useds = [0,0,0,0,0,0,0,1,0,0,11]
+      unuseds = [2,2,1,1,1,2,1,11,1,2,12]
       sheet.each do |row|
         assert_equal useds.shift, row.first_used
         assert_equal unuseds.shift, row.first_unused

test/row.rb

@@ -0,0 +1,33 @@
+#!/usr/bin/env ruby
+# TestRow -- Spreadsheet -- 08.01.2009 -- hwyss@ywesee.com
+
+$: << File.expand_path('../../lib', File.dirname(__FILE__))
+
+require 'test/unit'
+require 'spreadsheet'
+
+module Spreadsheet
+  class TestRow < Test::Unit::TestCase
+    def setup
+      @workbook = Excel::Workbook.new
+      @worksheet = Excel::Worksheet.new
+      @workbook.add_worksheet @worksheet
+    end
+    def test_formatted
+      row = Row.new @worksheet, 0, [nil, 1]
+      assert_equal 2, row.formatted.size
+      row.set_format 3, Format.new
+      assert_equal 4, row.formatted.size
+    end
+    def test_concat
+      row = Row.new @worksheet, 0, [nil, 1, nil]
+      assert_equal [nil, 1, nil], row
+      row.concat [2, nil]
+      assert_equal [nil, 1, nil, 2, nil], row
+      row.concat [3]
+      assert_equal [nil, 1, nil, 2, nil, 3], row
+      row.concat [nil, 4]
+      assert_equal [nil, 1, nil, 2, nil, 3, nil, 4], row
+    end
+  end
+end

test/suite.rb

@@ -0,0 +1,14 @@
+#!/usr/bin/env ruby
+# suite.rb -- oddb -- 08.01.2009 -- hwyss@ywesee.com
+
+require 'find'
+
+here = File.dirname(__FILE__)
+
+$: << here
+
+Find.find(here) do |file|
+	if /(?<!suite)\.rb$/o.match(file)
+    require file
+	end
+end