phpDocumentor Guides integration#200
Conversation
| /* | ||
| * This file is part of the Docs Builder package. | ||
| * (c) Ryan Weaver <ryan@symfonycasts.com> | ||
| * This file is part of the Guides SymfonyExtension package. |
| $this->filesystem->put('index.rst', $contents); | ||
| } | ||
|
|
||
| #[\Override] |
There was a problem hiding this comment.
we don't use this attribute in Symfony (and Psalm has a config setting to avoid requiring its usage)
|
Thanks for the detailed review, @stof :) This was just a blunt import of the commits I made in my test project, which I started from scratch. So the goal now is to make sure the code in this PR matches whatever we were doing in this repository before. Your comments are a great start with this, as they show some unintentionally difference that we should revert from this PR before merging. |
|
The CI also tells me that existing tests must be updated (which will help avoiding regressions if they can still run) |
This will help differentiating between the Doctrine RST parser based code and phpDocumentor Guides based code for now. Once everything is cleaned up, we can flatten the structure again.
bf5952b to
fa579ee
Compare
86ae537 to
1682f32
Compare
|
Little update after this weekend: I've applied the changes applied to this repository since starting with the guides one 2 years ago (like AI github links and code highlight changes). I've also merged the integration test fixtures from the Guides integration and this repository. This allows you to see the differences in the output, although 99% of it is whitespace so it's very hard to review. While working on the integration, I've been very careful with updating the expected files so I'm very confident the resulting HTML is still equal. Next up: moved to PR description |
|
Wouter, thanks a lot for the detailed update and for all this work! |
| use SymfonyTools\DocsBuilder\GuidesExtension\Build\BuildEnvironment; | ||
| use SymfonyTools\DocsBuilder\GuidesExtension\Build\StringBuildEnvironment; | ||
|
|
||
| final class DocBuilder |
There was a problem hiding this comment.
shouldn't this DocBuilder class stay outside the GuidesExtension namespace ? This looks like the entrypoint using guides more than an extension of the guides library.
There was a problem hiding this comment.
Yes, for now (to easily differentiate between new and old code), I've moved everything in the GuidesExtension. But some of it will be moved back once the repository is cleaned up.
| "autoload": { | ||
| "psr-4": { | ||
| "SymfonyTools\\DocsBuilder\\GuidesExtension\\": "src/GuidesExtension/src", | ||
| "SymfonyDocsBuilder\\": "src" |
There was a problem hiding this comment.
is this SymfonyDocsBuilder namespace still used ?
| @@ -1,3 +1,5 @@ | |||
| :orphan: | |||
|
|
|||
There was a problem hiding this comment.
! orphaned documents are not automatically detected
(probably not a major blocker: symfony.com only renders Prev/Next page links for very specific sections of the docs)
| $context->getDestination()->put( | ||
| $context->getDestinationPath().'/'.$context->getCurrentFileName().'.fjson', | ||
| json_encode([ | ||
| 'parents' => [], |
There was a problem hiding this comment.
No easy support for fetching the parent document. As far as I can find the symfony.com code, this property is never used so probably safe to ignore (cc @javiereguiluz please double check my assumption 😄)
There was a problem hiding this comment.
@wouterj would a proper document tree in phpDocumentor/guides help?
There was a problem hiding this comment.
It would be nice for a fully compatible fjson renderer, but it's not a blocker for Symfony adaption as far as I can see (as we don't use the parent data).
There was a problem hiding this comment.
As long as this renderer is in this repo (rather than being submitted upstream), it is not an issue if it does not support full compatibility with Sphinx (it might make sense to document the known differences though, for easier maintenance).
|
Sooo, I've managed to hack this version of the DocsBuilder into the symfony.com repository. It's not nice yet, but it somewhat works. As an example, let's take https://symfony.com/doc/8.1/quick_tour/the_architecture.html Locally with phpDocumentor Guides, this now renders like this: https://github.com/user-attachments/assets/da0d8b1f-7b5b-44ce-97df-4d5df05cd390 Comparing this with the live version, pixel by pixel, the only differences are found in the diff code blocks. And the phpDocumentor variant is actually more correct! I've updated the JSON tests to represent the actual data we need: we need a lot of TOC information that I forgot to implement. Also, I want to refactor things a bit to merge We also need better reporting on failures in But, this is looking very promising! |
This merges the SymfonyExtension from https://github.com/wouterj/symfony-docs-guides into this project.
Todo
DocsKernelandDocBuilderin this repositoryDocBuilder::build()