Re: ruby spreadsheet patches

[Zeno Davatz]

Dear Mina

Thank you for your Mail.

Can you please clone from here,

https://github.com/zdavatz/spreadsheet

and send me a pull request based on above clone. This is the latest code. Please also send me your tests.

Best
Zeno

Am 08.06.2012 um 13:45 schrieb Mina Naguib <mina....@bloomdigital.com>:

>
> Hi Artem, Zeno
>
> I have a small series of commits which you can see at the top here:
> https://github.com/minaguib/spreadsheet/commits/master
>
> They *significantly* improve write speed for large (thousands of cells) sheets, which is the case in my app. An example report I produce dropped from 370 seconds to 33 seconds using these changes.
>
> I'd like to contribute these patches upstream - normally I'd send a github pull request, but your primary repo is managed elsewhere.
>
> Could you let me know how/what format you'd like me to submit these commits as ?
>
> Or, feel free to add my github repo as a remote in your copies, and do the pull/merge yourselves. It should be a clean merge on top of master in your primary repo.
>
> Thank you, and thank you for an amazingly useful gem.
>
>
>

[Zeno Davatz]

Dear Mina

Thanks for you Mail.

Please fix the Testsuite, your option 2.

Best
Zeno

On Fri, Jun 8, 2012 at 5:26 PM, Mina Naguib
<mina....@bloomdigital.com> wrote:
>
> Thanks
>
> I'll be adding tests for my changes, then sending a pull request.
>
> Speaking of tests, I want to run something by you for your opinion:
>
> This commit
> ( https://github.com/minaguib/spreadsheet/commit/175523d6879353596310bedaefb06d12fe35049c
> ) actually broke the test suite on ruby 1.8.  The reason is that switching
> from an array to a hash no longer preserves the order when writing the SST.
>  For example the string at worksheet[0]row[0]cell[0] might not be written to
> SST[0], but SST[x]
>
> This is not a technical problem, nor a bug, and the generated file is still
> legal - however the test suite makes the assumption that the order will be
> preserved, so it failed (randomly, on ruby 1.8).
>
> There are 2 possible solutions:
>
> 1. Preserve the order when writing the SST, just to satisfy the existing
> test suite:
> I've already done this, without the reversion back to a slow array, in this
> branch+commit: https://github.com/minaguib/spreadsheet/commit/786163d04cd8c0156d583e1e17098f07bbcc2a7f
> The fix makes the hash data structure slightly more complex, but doesn't
> slow down writing
>
> OR
>
> 2. Fix the test suite to not expect SST to have the same order as the
> strings in the book
> I haven't done this (at least, not yet).  I'd imagine this work would look
> something like editing test/integration.rb and replacing instances of :
>
> str1 = book.shared_string 0
> assert_equal 'Shared String', str1
>
> with
>
> assert_equal 20, book.sst.length
> assert book.sst.include?("Shared String")
>
>
> Let me know what your opinions are and which approach you prefer.
>
>
> --
> Mina Naguib :: Director, Infrastructure Engineering
> Bloom Digital Platforms :: T 514.394.7951 #208
> http://bloom-hq.com/