-
-
Notifications
You must be signed in to change notification settings - Fork 19
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
Cleanup deprecated Line->input
#64
Conversation
we actually need this internally to store the raw input
I did a quick review. This looks good to me.
Yes, maybe we have to add more tests. Regarding getUnsafeInput in code block, i guess this will explain that behavior:
|
👍
I verified all listener changes with new tests. Feels better :)
I was actually wrong about the script listener. I thought it was the The unsafe is for custom listeners where the editor-user can add html which should be executed as html in the reader-user's browser. But we don't have such listener built-in anyway. I'd be comfortable about merging this. |
Code Climate has analyzed commit 4f3834c and detected 0 issues on this pull request. The test coverage on the diff in this pull request is 100.0% (50% is the threshold). This pull request will bring the total coverage in the repository to 98.0% (0.0% change). View more on Code Climate. |
This does a cleanup of the deprecation mark on
Line->input
.There's still calls to that property for different reasons I think:
Line->getInput()
: https://github.com/nadar/quill-delta-parser/blob/master/src/listener/Color.php#L28, https://github.com/nadar/quill-delta-parser/blob/master/src/listener/Font.php#L42-L45I haven't dived into the tests yet, however all tests are green. That also worries me a little though. While making and debugging I never got failed tests. For example switching
Script
to usegetInput
instead, no test breaks. So we probably need to add some tests to make sure this doesn't break anything.