Initial commit to support Cell styling and number formats in styles

[Rushleader]

Quick fix for issue #205, #182 and #151; Also added a feature being able register the style to the sheet without having to call addRowWithStyle (required to add it to the cell as a single style)

Edit: I know it's a dirty fix. Will update this to tidy it up and support default format by using constants but would like this to be in the master so it's available to everyone.

Edit 2: For now its only for XLSX, since I dont have OpenOffice available atm

[boxcla]

Verified that @Rushleader has signed the CLA. Thanks for the pull request!

[Rushleader]

Usage example:

• Defining a number format:

$styleRedFloat = (new StyleBuilder())->setNumberFormat('[Red]0.00000')->build();
$writer->registerStyle($styleRedFloat); # If using it for a single cell

• Append to the cell/row

$row = array(
    array('cell1', $styleRedFloat)
);
$writer->addRow($row);

// Or

$row = array('cell1', 'cell2');
$writer->addRowWithStyle($row, $styleRedFloat);

[Rushleader]

In an unexpected turn of events the number formatter can actually parse dates as custom format aswell :|

$styleDate = (new StyleBuilder())->setNumberFormat('d-mmm-YY HH:mm:ss')->build();

// Where time is a unix timestamp
$row[] = array(25569 + (time() / 86400), $styleDate);

This will format it correctly since the unix timestamp is changed to a julian excel date

[Rushleader]

@adrilo Could you take a look at this ? kinda sitting here doing nothing atm

[matthewtrask]

As users of this repo, we would love to see this feature merged in. It would help us immensely.

[adrilo]

@Rushleader, I'm gonna be really busy this week but I'll try to take a look at it next week. It's been a while since you pushed this PR... thanks for your patience!

[adrilo]

There are 2 different changes here: alignment and number format. Can you please split these changes into 2 branches? It will reduce the scope of each commit and make the review easier/faster. Thanks!

[adrilo]

@Rushleader, your PR changes many things: alignment, number format, per-cell styling. That's too many things at once to be reviewed and merged safely. It will need to split into multiple PRs.

Now regarding your actual changes, number format does need per-cell styling (number format at the row level is not really useful). I'm not a fan of the way you did per-cell styling (with multiple possible types (scalar/array) for the same variable). I believe that the cell granularity should be exposed explicitly, with meaningful APIs, etc. This will come with v3...

So what we can do is eventually merge your change for the alignment but wait until v3 to get the number formatting change merged.

[Rushleader]

@adrilo Yea the alignment change came from a mate of mine that wanted to use the number_formatting. He kinda pushed it into the same branch =/

I personally think that decent number formatting is kind of a big deal in excel. That's why I was surprised in the first place when I couldn't find it in the code. So in my opinion V3 might be a bit late for that feature.

Touching on the multi feature PR, that's always gonna be there since these features are dependent on one another (required the per cell/column styling for both of them).

[adrilo]

Alignment can be done on a per-row basis for now and can be migrated to a per-cell basis whenever this is ready, right? Even though some users will need to only apply the alignment to some cells, it may help with some use cases. Still it's not top priority (left alignment is acceptable in 99% of the cases).

Also regarding number formatting, I agree that this is pretty important. Spout was initially created as a CSV replacement: this means no formatting at all! Formatting was added progressively and there still is a lot more than can be added to it. This must also be balanced with performance, as the more features you add, you more you degrade performance. So not all features are necessarily worth being added.

[Rushleader]

Hmm, i agree with you on the last point 'Not all features are worth being added' but I think for this to grow we need to atleast support a way to format a single cell (which requires both these features).

The performance issue you touch on is a valid one, but it'll take a while before the performance hit will be noticeable. As with features not all being worth to be added, same goes for arbitrary restrictions (i.e. a performance decrease for a feature set). I understand that's hard to keep balanced though.

Ill try and see if i can tidy this up and split this into separate PRs. One for cellstyling, one for number formatting and one for alignment settings. (I think it's easier to call this "Column Styling" in the future since that's what we're shooting for.) Would this be easier to accept and keep in check ?

p.s. If Spout is getting this much bigger, it might be a good idea to start using a git-flow like structure and make the master branch protected and working in release branch cycles. That way it's also easier for people to create their own branch of Spout.
We've been using this structure for a while now and it makes it easy to read the changes.
http://nvie.com/posts/a-successful-git-branching-model/

[adrilo]

I'd rather go with Cell styling and skip column styling all together. Column styling can be achieved with cell styling so we should try to implement the most specific case that encapsulates more generic cases.
Alignment or number formatting are not necessarily column only options and can be applied to cells independently.

Anyways, splitting up this PR into 3 would make things a lot easier to review and therefore faster to merge. I'm maintaining this project on my spare time so when I see a big commit, I need to have quite some time ahead of me to start looking at it. Not something I can do in few minutes.
Regarding cell styling, I have some strong opinions on that so it may be better for me to tackle it. Then you can build all your improvements on top of it. Or we can do it in parallel and reconcile the 2 at the end.

Thanks for the link. I was looking into it a few weeks ago, as I started thinking about v3. Instead of committing to master, all v3 commits should be merged into the v3 branch and once done, we can merge the v3 branch back to master. Nothing new here though, just need to set it up :)
The git-flow approach is really interesting too. I'm wondering if this is preventing some developers to submit pull requests though, as it's yet another process that needs to be followed.

[Rushleader]

I think it would make developer commit sooner since the merging process for develop should be simpler and less restrictive. During the time it's in develop it'll also get airtime on other branches and get tested on a per use basis (which by default will cover more weak points then unit testing, not excluding those btw).

And on the cell styling part, strong opinions? Whats ur vision for that specific scenario since I kinda need it to format it to specific float and date notations without losing the value typing.

About the column styling, this can be an extension on the cell styling by just setting it once for a column.

[adrilo]

Staled. Closing

[ic-hv]

Why not just merge these changes in ...? They are useful!

[adrilo]

It's not that easy unfortunately. This PR did not have tests, and had a few other things that did not meet the quality requirements we have on this project.
Now I agree that the features it brought are great and will definitely be useful. By the way, cell styling was recently added on the develop branch. We don't have number formatting nor alignment but it's coming, for all file types (not just XLSX)

[bs-thomas]

Hello there, I'm wondering if there are updates on this issue? Thanks guys!

[adrilo]

Some of the features have been merged into the 3.0 branch (yet to be released).

[bs-thomas]

Thanks for the quick response. Wow looking forward to it!

[juanmiguel431]

Now exists the method setFormat(). I think the documentation should be update it with this.

Example:
$styleDate = (new StyleBuilder())->setFormat('dd-mm-YYYY')->build();

[adrilo]

Included in #687