spout
spout copied to clipboard
Added support for mergeCells, cell height, shrink to fit
Added support for mergeCells: // mergeCells (B2:G2), you may use CellHelper::getColumnLettersFromColumnIndex() to convert from "B2" to "[1,2]" $writer->mergeCells([1,2], [6, 2]);
cell height:
$row->setHeight(30);
shouldShrinkToFit:
$style->setShouldShrinkToFit();
These changes are implemented for XLSX as that's what I need and test spout on.
Should close https://github.com/box/spout/issues/529#issuecomment-607089153 & https://github.com/box/spout/issues/729
Seems that one test is failing, any ideea why? I'm not familiar with phpunit or travis
@quamis From what I can see it's a code style issue. Run the following command in your terminal to fix it: vendor/bin/php-cs-fixer fix --config=.php_cs.dist
[Edit] Maybe wait for the https://github.com/box/spout/pull/747 PR to be merged as it will fix most code style issues unrelated to your changes
@jmsche it seems that I'll wait, as my spare time this month is pretty much zero :)
添加了对 mergeCells的支持: // mergeCells(B2:G2),您可以使用CellHelper :: getColumnLettersFromColumnIndex()从“ B2”转换为“ [1,2]” $ writer-> mergeCells([1,2],[ 6,2]);
cell height: $row->setHeight(30); shouldShrinkToFit: $style->setShouldShrinkToFit();这些更改是针对XLSX实施的,因为这正是我所需要的,并且对测试喷口进行了测试。
How to write the code, I also need to merge the cells
Hey guys and thanks for your work @quamis! This feature would be awesome for us. Can I do anything to make it happen?
Hey guys and thanks for your work @quamis! This feature would be awesome for us. Can I do anything to make it happen?
You could provide the missing Unit Tests!
Hey guys and thanks for your work @quamis! This feature would be awesome for us. Can I do anything to make it happen?
You could provide the missing Unit Tests!
@madflow Better late than never: https://github.com/quamis/spout/pull/1 Let me know if I could help with docs or anything else.
@jmsche any ideea why "continuous-integration/travis-ci Expected — Waiting for status to be reported " hasn't finished yet? I made the commit 2 weeks ago
@ignaczistvan I do not understand. Did you made the unitTests (I'm not seeing anything, but again, I'm not familiar with github's way of mergeing stuff)? Or you would prefer helping with the docs?
@ignaczistvan I do not understand. Did you made the unitTests (I'm not seeing anything, but again, I'm not familiar with github's way of mergeing stuff)? Or you would prefer helping with the docs?
@quamis Yep, I've made the tests and filed a pull request to your fork. You can check my commits here: https://github.com/quamis/spout/pull/1 If you like what you see, you can accept the PR. When you accept the PR, my changes will appear on this 'main' PR too.
I'm also open to helping with the docs if it's needed to release a new version.
@madflow Any update on this one?
@ignaczistvan Not sure what you mean... Are you referring to my hint above, that unit tests where missing?
@ignaczistvan Not sure what you mean... Are you referring to my hint above, that unit tests where missing?
Well, I’ve added tests for the new features so basically yes. Is anything missing, something I missed on the tests, docs, etc?
@ignaczistvan Not sure what you mean... Are you referring to my hint above, that unit tests where missing?
Well, I’ve added tests for the new features so basically yes. Is anything missing, something I missed on the tests, docs, etc?
You asked what you can do to bring this PR forward. After quick glance I noticed the missing unit tests.
From my experience: In order to raise chances that a maintainer will consider this PR "mergable" it should
- [ ] include tests
- [ ] include documentation
- [ ] focus on a single feature
- [ ] have feature parity in CSV/ODS/XLSX
From what I gather - the chances that a maintainer will have a look and give feedback are slim at the moment.
This will add the merged cells on all sheets, not just the current active sheet.
Merged in https://github.com/openspout/openspout/pull/21