Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement various missing features for the ODT writer #1796

Merged
merged 7 commits into from
Dec 29, 2020
Merged

Implement various missing features for the ODT writer #1796

merged 7 commits into from
Dec 29, 2020

Conversation

oleibman
Copy link
Contributor

@oleibman oleibman commented Jan 5, 2020

Implement a number of features implemented in PhpWord,
but not yet supported in PhpWord ODT Writer.

  1. Add default file to tests/PhpWord/_includes/XmlDocument.php to make it
    considerably easier to test ODT changes (and Word2007 changes involving
    files other that document.xml).
  2. Page break before each section.
  3. Page numbering start.
  4. Font style for Headings.
  5. Alignment for images.
  6. Paragraph style for TextRun.
  7. "Hide grammatical errors" for whole document.
  8. Page layout for each section.
  9. For each page layout, support user-specified page width, page height,
    orientation, margin top, margin bottom, margin left, margin right.
  10. Page header and footer.
  11. Named colors.
  12. NoProof font style.
  13. Paragraph Style - spaceBefore, spaceAfter, lineHeight, pageBreakBefore,
    indentation, text alignment.
  14. Tab stops.
  15. Basic support for some Fields (DATE, PAGE, NUMPAGES).
  16. Link had an error in how it was handling internal links (needs leading #).
  17. In addition to tests for all the above, added some tests for Tables.

Item 11 above needs 1 module from Pull Request 1775, which is targeted
for v0.18.0 but not yet merged, so the relevant module is also here.
Item 15 above needs 1 module from Pull Request 1774, which is targeted
for v0.18.0 but not yet merged, so the relevant module is also here.
Testing change from Pull Request 1771 is included here, but was
merged after my fork.

Description

Please include a summary of the change and which issue is fixed. Please also include relevant motivation and context.

Fixes # (issue)

Checklist:

  • I have run composer run-script check --timeout=0 and no errors were reported
  • The new code is covered by unit tests (check build/coverage for coverage report)
  • I have updated the documentation to describe the changes

Implement a number of features implemented in PhpWord,
   but not yet supported in PhpWord ODT Writer.
1. Add default file to tests/PhpWord/_includes/XmlDocument.php to make it
   considerably easier to test ODT changes (and Word2007 changes involving
   files other that document.xml).
2. Page break before each section.
3. Page numbering start.
4. Font style for Headings.
5. Alignment for images.
6. Paragraph style for TextRun.
7. "Hide grammatical errors" for whole document.
8. Page layout for each section.
9. For each page layout, support user-specified page width, page height,
   orientation, margin top, margin bottom, margin left, margin right.
10. Page header and footer.
11. Named colors.
12. NoProof font style.
13. Paragraph Style - spaceBefore, spaceAfter, lineHeight, pageBreakBefore,
    indentation, text alignment.
14. Tab stops.
15. Basic support for some Fields (DATE, PAGE, NUMPAGES).
16. Link had an error in how it was handling internal links (needs leading #).
17. In addition to tests for all the above, added some tests for Tables.

Item 11 above needs 1 module from Pull Request 1775, which is targeted
    for v0.18.0 but not yet merged, so the relevant module is also here.
Item 15 above needs 1 module from Pull Request 1774, which is targeted
    for v0.18.0 but not yet merged, so the relevant module is also here.
Testing change from Pull Request 1771 is included here, but was
    merged after my fork.
@coveralls
Copy link

coveralls commented Jan 5, 2020

Coverage Status

Coverage increased (+0.3%) to 94.904% when pulling 4e347b3 on oleibman:odtchanges into 733f845 on PHPOffice:develop.

I do not understand one suggestion, and I believe one is wrong.
I will add comments to my ticket once this is pushed.

One that I can discuss up front
PhpWord/Style/Paragraph indicates that Indentation must be of type
   \PhpOffice\PhpWord\Style\Indentation, but it can also be null.
   My test for instanceof ... is one of the Scrutinizer reports.
   I did not change PhpWord/Style/Paragraph, but this commit does so
   by updating @var for indentation.
Still one report that I don't understand at all, and one I'm not sure of.
@oleibman
Copy link
Contributor Author

oleibman commented Jan 5, 2020

I am completely baffled by the remaining Scrutinizer issue. In Writer\ODText\Element\Title,
I have the following code at line 43:
if ($elems[0] === $element) {
$hdname = 'HE';
}
Scrutinizer reports that the test condition is always false. However, this is the only place where I set
the header name to HE*, and I have a number of tests which assert that HE* is the style name in use,
and these tests all pass, so it's clear, at least to me, that the test condition is not always false.
These tests are:
tests/PhpWord/Writer/ODText/ElementTest.php line 152 (in testTitleAndHeading)
tests/PhpWord/Writer/ODText/Styles/ParagraphTest.php lines 288, 304, 313, and 336
(in testHeadingPageBreakBefore).
I also added an echo in there in a test version, and confirmed that it is reaching the code in
question, by the way.
So, any advice on what needs to be done here?

Two lesser Scrutinizer items that I may as well mention now.

Writer/ODText/Style/Paragraph needs to test whether Indentation is null or not before using it.
Scrutinizer said it was always an instance, but it can be null.
But Style/Paragraph had not said this was a possibility, so I changed the docblock to allow this. Nevertheless, Scrutinizer still complained.
I changed my test from (instanceof Indentation) to (empty), and now Scrutinizer was happy.

Scrutinizer reported a problem in Writer\ODText\Element\TextRun on the following code:
$element = $this->getElement();
$pStyle = $element->getParagraphStyle();
The error was that getParagraphStyle doesn't exist on PhpOffice\PhpWord\Element\AbstractElement.
But it does exist on PhpOffice\PhpWord\Element\TextRun. It said I could eliminate the error
with an annotation, which I did, but I'm not proud of it. I'm just not sure what a better
method of correcting the problem would be. Incidentally, the same code is in
Writer\ODText\Element\Text.

Changes to improve test coverage based on Coveralls report.
Attempt to work around demonstrably incorrect Scrutinizer analysis
  (flags code as bug because "condition is always false"
   even though Coveralls reports that code which would be
   executed only if condition is true is indeed executed).
Cover one line previously omitted from coverage.
Coverage for Writer/ODText is now 100%.
@troosan troosan added this to the v0.18.0 milestone Jul 6, 2020
@troosan troosan merged commit b2335d2 into PHPOffice:develop Dec 29, 2020
@troosan troosan changed the title ODT Changes Implement various missing features for the ODT writer Feb 7, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants