Use doctrine/rst-parser 0.6 #148
Closed
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Happy to see this |
|
I must admit that I'm surprised by the green builds here, as the test suite breaks (in weird ways) locally 😄 PHPunit output locally |
|
Ooh nevermind, this apparently has something to do with PHP 8.1: #147 |
|
@wouterj if you have some time for this, let's try to change this from draft to a real PR. It looks OK and it would fix the current failing tests in PHP 8.2. Thanks a lot! |
|
@javiereguiluz I still have one major breaking change for v. 0.6 pending before we can release and only do Bugfixes thereafter. If you and or@wouterj could review that would be helpful. I would not suggest to use v 0.6 in productive before that or is merged. |
|
After the changes in 0.6 in the past 2 months, there is quite some more work to do on this library in order to be compatible. I've done the first big part of the work, with 14 failing tests left (in some cases, the tests are wrong, in others our implementation and for some I believe there might be a bug on rst-parser). |
|
This PR is now nearly ready. Lots of code removed and two regressions left in RST Parser (doctrine/rst-parser#265 & doctrine/rst-parser#266). |
wouterj
commented
Mar 9, 2023
javiereguiluz
approved these changes
Mar 9, 2023
There was a problem hiding this comment.
I reviewed this and it looks correct to me. I love how many code we can remove thanks to the directives added upstream.
You also kept all the directives/roles specific for Symfony Docs, so I think we're good.
By the way, in symfony.com we build docs like this:
$buildConfig = (new BuildConfig())
->setContentDir($buildDir)
->setOutputDir($tempBuildDir)
// ...
$result = $this->docBuilder->build($buildConfig);Can you see any possible changes that we might need to do because of this PR? Thanks!
No, the public APIs of those 2 classes you show in the example ( You might want to check templates/themes though. Does symfony.com use the theme provided in this repository, or does it have custom Twig templates? If it's the latter, those might need some updates |
|
Thanks for verifying this. No, we don't use custom templates because we rely on the default theme. Thanks! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
TYPO3 documention team member @linawolf is doing lots of great work over on the doctrine/rst-parser to (a) fix bugs in the current parser and (b) move some of the features that we wrote in this library over to the parser.
This PR is just a draft to see if these changes broke something for us. I'm planning on also removing some of the features implemented on the parser in this PR before 0.6 is stable (e.g. most admonition directives are now integrated in rst-parser).BuilderFactory. The Kernel no longer serves a purpose with the introduction of theSymfonyDirectiveFactory.