Writer ODS : Writer Border Style for cells

[Progi1984]

This is:

• a bugfix
• a new feature
• refactoring
• additional unit tests

Checklist:

• Changes are covered by unit tests
  • Changes are covered by existing unit tests
  • New unit tests have been added
• Code style is respected
• Commit message explains why the change is made (see https://github.com/erlang/otp/wiki/Writing-good-commit-messages)
• CHANGELOG.md contains a short summary of the change and a link to the pull request if applicable
• Documentation is updated as necessary

Why this change is needed?

Fixes #3690

[oleibman]

@Progi1984 It's late here. I will try to start reviewing tomorrow. In the meantime, can you add some tests to demonstrate that your change works as expected?

[PowerKiKi]

See comments, and should add tests as mentioned by @oleibman

[Progi1984]

@Progi1984 It's late here. I will try to start reviewing tomorrow. In the meantime, can you add some tests to demonstrate that your change works as expected?

@oleibman Can i add XML Tester like in PHPWord unit tests : for example, https://github.com/PHPOffice/PHPWord/blob/master/tests/PhpWordTests/Writer/Word2007/ElementTest.php#L109 ?

[oleibman]

@Progi1984 Take a look at tests/PhpSpreadsheet/Writer/Ods/ContentTest. It puts the generated Xml in a string via:

$content = new Content(new Ods($workbook));

I'm not sure offhand if the results of your change will go into the xml file content.xml (in which case the code above will do) or styles.xml (in which case you'll have to tweak it a bit).

[oleibman]

@Progi1984 I agree that your test confirms that your change is working, but ... it seems unmaintainable (e.g. the unnecessary number-columns-repeated in the xml seem like a bug, and, if we fix that, your test breaks) and opaque. It is just not clear why that xml demonstrates the correctness of the change. I will let @PowerKiKi weigh in on this - if it's okay with him, it's okay with me. But, if not, what I had in mind was for you to load the xml string as a DomDocument, and then query the DomDocument (possibly using normal Dom methods or xpath) to confirm the following:

• the first row first cell is linked to a style which sets the left and top borders to the expected color and thickness, and does not specify a bottom or right border
• the first row second cell ... sets right and top, does not set bottom or left
• second row first cell ... sets left and bottom, does not set top or right
• second row second cell ... sets right and bottom, does not set left or top

Does that sound like a test you can set up? Some tests in PhpWord Writer/Html (e.g. ElementTest) use this technique.

[Progi1984]

@oleibman No soucis. I work on it.

[oleibman]

@Progi1984 Your new test looks good (and has definitely expanded my horizons on how to use xpath). I will try to merge this over the next day or two.

[Progi1984]

@oleibman I rebase the PR ;)

[oleibman]

Thank you for your contribution.

[Progi1984]

@oleibman It's not the first contribution. I've been contributing since 2011.