search

felixbuenemann / xlsxtream

Streaming & Fast XLSX Spreadsheet Writer for Ruby
MIT License
217 stars 38 forks source link

Use worksheet without block #42

Closed kukicola closed 4 years ago

kukicola commented 4 years ago

Hello! In my case using a block was a bit problematic. Made a small change. Current behaviour is preserved, so it is possible to use it with or without a block

felixbuenemann commented 4 years ago

The reason this method only allows calling with a block is to avoid corruption which would happen if writing to multiple worksheets interleaved.

Can you elaborate why the block semantics don't work for you?

kukicola commented 4 years ago

The reason this method only allows calling with a block is to avoid corruption which would happen if writing to multiple worksheets interleaved.

Right, that makes sense.

In my case I got a service that pulls data from API and exports it to multiple file formats. So I have few Writer classes, one for each format. On each one of them I got a method write_data which basically just writes one line to file. I could just change all the current writers (to use a block) or use a Fiber for this particular case (it wasn't very pretty solution). Also I prefer to avoid blocks because without them it's easier to pass the writer object to multiple services and let them handle additional writes on their own.

I guess my case is a bit specific and not that common so maybe it makes more sense just to use forked version instead of merging it here

felixbuenemann commented 4 years ago

I don't think a fork is necessary. I'll think about how to solve this while still communicating, that it is unsafe and can only be done if certain rules are followed.

felixbuenemann commented 4 years ago

I think The API is clearer if the add_worksheet method is changes to create and return an instance of the worksheet, while the behavior of write_worksheet is kept as is (calling add_worksheet internally).

Can you change your PR and also add a sentence to the README, that add_worksheet can only be used sequentially?

kukicola commented 4 years ago

Sure, updated

felixbuenemann commented 4 years ago

Looking good! – I'll try to get it merged and make a new release in a couple of days.

sandstrom commented 4 years ago

This would be very useful for us too! 🌟

Any ETA for this new release? 😄

sandstrom commented 4 years ago

@felixbuenemann I've added a PR which builds upon this one, adding some guards to the block functionality: https://github.com/felixbuenemann/xlsxtream/pull/45

@kukicola let me know what you think!

felixbuenemann commented 4 years ago

@sandstrom Oh sorry, I was very busy at the time and then forgot to get back to this.

Things have slowed down a bit, so I can probably do a release some time this week.

felixbuenemann commented 4 years ago

Closed in favor of #45, which is is a successor to this PR.

Thanks @kukicola!