-
Notifications
You must be signed in to change notification settings - Fork 2
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
Support for PLAIN BB-Code and CODE=rich #36
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yep, adding phpunit to our GitHub actions would be cool.
Thank you for fixing/addressing these bugs/issues as well as your additional improvements of the code in general.
I have already provided some feedback, even though you are probably still working on it.
I will go ahead and fix your mistakes in the comments and extract the helper methods for the tests into a trait.
I had to remove the parser-trait, because currently there is no autoloader in place to load classes inside the tests-folder. |
there is no reason the check the text-buffer after a unclosed bbcode, because the unclosed bbcode consumes the rest of the input anyway.
Argh, i missed the change in composer.json ... autoloading in tests/ was possible |
Yep, I actually set everything up for phpunit. |
I think we're good to go now 😃 |
I decided to implement CODE=rich (as mentioned in #37) in this pr, too, because this requires another rewrite of the parser. |
Alright, take your time. All looks very good to me. 😃 |
Should work now 😃 |
Somehow my latest push is not showing up here? |
Well, anyway @jr-cologne i'm not 100% satisfied with the api. What do you think? |
Sorry, I have kinda lost track of the changes you have applied here. 😄 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, to me, the API and such looks perfectly fine.
What do you exactly not like yet?
Skipping the formatter if the language is "text" would probably speed things up a little bit as the formatter wouldn't have to go through all potential formatters to finally find anything which formats plain text. So feel free to make this change.
"Besser wirds erstmal nicht" i guess 😆 |
Oops, I accidentally pushed something. I hope I've not broken anything. 😅 |
Ok, besides one error I get when formatting with prettier, everything looks pretty good. 👍 Here's the output:
Test input:
This might just be an issue on my end, btw. Before you merge and I forget it: Thank you so much for this huge amount of work you put into this PR! 🥇 🥳 🙌 |
Fixes #29
Fixes #30
Fixes #32
Fixes #37
I also added phpunit and wrote some tests as mentioned in #31
We can add phpunit to our phan action, too.