zdavatz / spreadsheet

The Ruby Spreadsheet by ywesee GmbH
http://spreadsheet.ch
GNU General Public License v3.0
1.13k stars 240 forks source link

Unclear message if argument if frozen #234

Closed cyrilchampier closed 5 years ago

cyrilchampier commented 5 years ago

When you try to open a spreadsheet from a stringio, the underlying string must be mutable:

> ::Spreadsheet.open(StringIO.new('0123'.freeze), 'rb+')
FrozenError: can't modify frozen String
from gems/spreadsheet-1.2.3/lib/spreadsheet/excel/reader.rb:1280:in `force_encoding'

https://github.com/zdavatz/spreadsheet/blob/master/lib/spreadsheet/excel/reader.rb#L1279

It would be cool if it was possible to open a frozen io.

zdavatz commented 5 years ago

Yes indeed! Please let me know if you come up with a patch.

cyrilchampier commented 5 years ago

Would this PR be ok for you? https://github.com/zdavatz/spreadsheet/pull/235

zdavatz commented 5 years ago

Thank you for the PR. All tests are failing. The tests need to pass, all of them.

cyrilchampier commented 5 years ago

Of course, I will fix that ;) What do you think about support of ruby 1.8? Do I fallback to previous behaviour (which fails on frozen steams)?

zdavatz commented 5 years ago

Yes, I think that is best, if you do not have another solution.

cyrilchampier commented 5 years ago

It should be good now.

zdavatz commented 5 years ago

Thank you, Version 1.2.4 is out. Please test.

cyrilchampier commented 5 years ago

Tested on my side, it is working! Thanks for your reactivity.

zdavatz commented 5 years ago

thank you too!