Skip to content
This repository has been archived by the owner on Jul 9, 2018. It is now read-only.

Add package: autop #53

Merged
merged 2 commits into from
Jan 4, 2018
Merged

Add package: autop #53

merged 2 commits into from
Jan 4, 2018

Conversation

aduth
Copy link
Member

@aduth aduth commented Dec 15, 2017

Related: WordPress/gutenberg#4005

This pull request seeks to introduce a new autop package with two named exports: autop and removep. Originally these were extracted verbatim from core's editor.js. However, it was found that the JavaScript implementation of autop was lacking in many ways when I sought to port the equivalent PHP tests for wpautop. You can see the failing skipped tests in f31289b.

Instead, I re- ported the function directly from the PHP implementation as faithfully as possible. With the reimplementation in 4846972, all test cases ported from the PHPUnit tests pass.

Testing instructions:

Ensure unit tests pass:

npm test

@codecov
Copy link

codecov bot commented Dec 15, 2017

Codecov Report

Merging #53 into master will decrease coverage by 19%.
The diff coverage is 59.47%.

Impacted file tree graph

@@             Coverage Diff             @@
##           master      #53       +/-   ##
===========================================
- Coverage   97.36%   78.36%   -19.01%     
===========================================
  Files          17       18        +1     
  Lines         152      305      +153     
  Branches       43       67       +24     
===========================================
+ Hits          148      239       +91     
- Misses          4       55       +51     
- Partials        0       11       +11
Impacted Files Coverage Δ
packages/autop/src/index.js 59.47% <59.47%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 6cbae56...a0aa145. Read the comment docs.

@aduth aduth merged commit 724b7c4 into master Jan 4, 2018
@aduth aduth deleted the add/wpautop branch January 4, 2018 21:16
@azaozz
Copy link

azaozz commented Jan 5, 2018

However, it was found that the JavaScript implementation of autop was lacking in many ways

Not exactly :)
The JS autop is meant to work together with TinyMCE and the browser auto-correct: white space doesn't matter, closing </p> tags are optional, TinyMCE converts text that is directly in the body to paragraphs, etc. The three combined have generally less edge case errors than the PHP autop. Some PHP tests were probably failing as they look also at white space between tags.

Nevertheless this version should work properly, a bit slower perhaps. Did some "manual" testing in the current editor and didn't see regressions except one case where extra empty paragraphs were added (but couldn't repeat/confirm).

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants