doctrine / rst-parser

MIT License
59 stars 23 forks source link

Let's talk about joined effort #156

Open jaapio opened 3 years ago

jaapio commented 3 years ago

Thanks @stof for reaching out to the phpDocumentor team. I would like to talk to you guys for a moment about the doctrine/rst-parser. Which we integrated in phpDocumentor. More that a year ago we had contact with Jonathan Wage about the library. If I remember it correctly he forked the original library to a doctrine version. Back than there was nearly any maintenance on the lib. And from the doctrine team there was not much interest to support any additional features and improvements. That’s why we (@mvriel and me) have forked the doctrine version and started working on it.

With phpDocumentor we are trying to get a combination of API docs like we are rendering for years, with hand written manuals like doctrine and symfony are providing. In combination with the templates phpDocumentor provides and all static information we know about the code this should raise the level of documentation for PHP in general. That’s our goal.

Beside the missing support from the doctrine team which slowed us down in the additional features for phpDocumentor we wanted to do some large refactoring in the code to make it faster and fit better into the application workflow of phpDocumentor. So we have split the parsing and rendering in our version and some other improvements which allow us in the end to cache the parser results of each document, do some compile steps like we always do on cached items and then start the rendering. This includes also some major changes that are reducing the memory footprint of the library.

But now the there is some more movement on this project I think we can all benefit from the changes we are doing on both sides. And maybe bring it back into a single project which is maintained by all of us.

Please let me know if you are interested to join our forces and make some nice magic happen with the documentation of all the great projects out there.

greg0ire commented 3 years ago

This sounds good to me! How do you propose we move forward with this? Are you going to send PRs to us, or would you rather have everyone work on a fork of doctrine/rst-parser in the phpDocumentor namespace, or some other namespace?

Consider joining #rst-parser at https://www.doctrine-project.org/slack

If you want to get merge permissions on this repository, that should be possible too.

wouterj commented 3 years ago

Hi @jaapio! Thanks for this issue, I didn't know phpDocumentor maintained their own fork.

As you might know already, we've started using this to render some docs on symfony.com (e.g. twig and swiftmailer) and are planning to slowly integrate it in all doc build processes. One of the main reasons to use this repository instead of starting from scratch was because we would start with something that is (somewhat) battle-tested and has other communities that are involved in maintenance. I think joining efforts makes all the sense, as it would mean that in the end we get a better package, and with less efforts from each individual contributor.

I'll let you and the Doctrine team figure out what the best way to join efforts is :)

jaapio commented 3 years ago

I think our first step would be it describe what all projects need from this library? Do you want just an easy way to parse the RST into a html website? Or do you have more tooling around this library? When I look at the doctrine website project, I see a lot of overlap between phpDocumentor & planned features of phpDocumentor for example.

From my point of view, it would make sense to move the library into the phpDocumentor organisation. Since we are focussing on documentation :-) while Symfony is a framework, and doctrine is about databases. That would help people to look around and find a working application with this component. Documentation is our primary focus, so you can focus on what is actually needed for your projects. Writing that documentation :-)

Merging the work from both sides could be a challenge, we moved around a lot of parts. And I'm not entirely sure if we can just extract the Guides namespace from phpDocumentor at this moment. We didn't focus on keeping it as a separate component. But we can start working on that of course. To be complete, we forked this library, but it took too much effort for us to update the library outside phpDocumentor. If you want to have a look at what we did: https://github.com/phpDocumentor/phpDocumentor/tree/master/src/Guides. The fork is unmaintained.

Use phpDocumentor?

What is holding you back from using phpDocumentor, if we would support RST? I'm not trying to force you to use phpDocumentor, but I'm trying to understand what you need from this lib. Since the output of this library for you both is html rendered by twig. Which is exactly what phpDocumentor does. I think our current templating system is flexible enough to support everything you need?

Our plans / dreams

We are planning to have a tight integration between the api and guides. We are planning to do much more that just RST. Like integrating class diagrams based on packages defined in your code. Which will automatically update with your code.

Versions

It is not visible yet, but phpDocumentor will support versions in the future like the symfony documentation has right now. Which will allow you to link between versions.

Linking between projects

Inspired by sphinx, we would introduce some inter-project links. Which would allow you to link between projects, even from symfony to doctrine for example. If symfony and doctrine would both use phpDocumentor it would even allow other private projects to link to the symfony and doctrine documentation.

AST

Like API docs, RST parser in phpDocumentor should produce a cached AST, which would allow consumers of phpDocumentor to do the post processing themself. Like the transformation from objects to HTML or json?

Plugins

Support everything in phpDocumentor will be impossible. So phpDocumentor will support a way to affect our internal AST. Or extend the AST to render your own annotations for example. This is what we are planning to do for wordpress for example.

Final remark

Reading all this, I notice that I'm selling phpDocumentor here :-) and to be clear. I'm not stating that you MUST move to phpDocumentor. it is just to make clear what we are up to. ofcourse it would be an honor to serve projects like Doctrine and Symfony :-). Having users of your projects make working on it even more fun :sunglasses:

greg0ire commented 3 years ago

What strikes me at first, is how different both trees look when I compare https://github.com/doctrine/rst-parser/tree/0.5.x/lib and https://github.com/phpDocumentor/phpDocumentor/tree/master/src/Guides … but maybe things are not so different in terms of usage?

Also, the README is not very reassuring :sweat_smile:

Anything regarding Guide generation, in this folder or elsewhere, is to be considered experimental and prone to be changed or removed without notice or consideration for BC. This is a follow-up to an earlier POC and can, at best, seen as an incubator project.

From my point of view, it would make sense to move the library into the phpDocumentor organisation. Since we are focussing on documentation :-) while Symfony is a framework, and doctrine is about databases

Doctrine is no longer solely about databases, we have packages that are used independently from doctrine/dbal, like our coding standard, or the annotations library, or this library. Still, it would make sense to move the library to phpDocumentor. But it would really have to be a move I think, as in "a copy of the project", with only the namespaces changed, to make the migration easy. Then maybe you could port changes from https://github.com/phpDocumentor/phpDocumentor/tree/master/src/Guides while respecting semver (picking different branches depending on whether the change is BC or not?)

BC is important here because we have packages that depend on us: https://packagist.org/packages/doctrine/rst-parser/dependents?order_by=downloads

mvriel commented 3 years ago

Also, the README is not very reassuring 😅

Please view the README from phpDocumentor's perspective. We needed to do a lot of work to make the library fit into a more abstract architecture.

As for the difference, we have had to (and still are) refactoring significant parts to make the library generate a cacheable AST. The version from which we took the fork had cross dependencies between the Node, Environment and Configuration that made it nigh impossible to split the process into the generation of a cachable AST, and in loading that AST and producing generated output with it.

Another big investment that we needed to do was to change the way the TOC was handled so that we could unify the TOCs of the API parser and the RST parser. We have brought the API parser towards the RST parser and vice versa to make this work.

And another significant challenge, is that the Span processing is currently not functioning to spec; the spec states it should be able to parse a Block, but the current implementation has a limited subparser which we are changing into handling complete blocks.

mvriel commented 3 years ago

The thing I forgot to mention, I have tried hard to keep the top level API between the libraries similar so that the entrypoint of the original library could still be made to work. But for the Nodes and Processors it was inevitable to break BC due to architectural issues and coupling

jaapio commented 3 years ago

Doctrine is no longer solely about databases, we have packages that are used independently from doctrine/dbal, like our coding standard, or the annotations library, or this library.

you are right, I was a bit too short there. :-)

I noticed I pointed you to the wrong directory... https://github.com/phpDocumentor/phpDocumentor/tree/master/src/Guides/RestructuredText should look more familiar :-D

greg0ire commented 3 years ago

It does look less different :smile:

Do you plan to extract RestructuredText or at least Guides to a separate repository at some point? If yes, then a plan could be:

javiereguiluz commented 3 years ago

From the Symfony point of view, the current situation is:

1) We use Doctrine RST Parser strictly as a RST parser and nothing more 2) We use Symfony Doc Builder (https://github.com/symfony-tools/docs-builder) on top of it to provide customizations and things strictly related to Symfony's RST and not RST in general

Would "Guides" be (1) + (2) (an RST parser + customizations needed for phpDocumentor) or keep being only (1) like current Doctrine RST parser?

If "Guides" is (1) + (2) and Doctrine RST parser is abandoned, we'll need to keep our own fork of this project. We really need an agnostic RST parser that only does that. Thanks!

wouterj commented 3 years ago

@javiereguiluz the "Guides" project started based on the symfony-tools/docs-builder, and now also includes doctrine/rst-parser. So "Guides" is (1) + (2).

However, we don't need to maintain a fork of this library, as it is also our docs-builder. We would only maintain a project that contains e.g. our custom theme and node customizations. At least, that is what I concluded when @jaapio shared some experiments he did with the Symfony docs (see also my posts on slack about this).

javiereguiluz commented 3 years ago

I'm all for joining efforts 💪 but if the new project is more than a RST parser, then sooner or later it will do things that we don't need or that even "hurt us" and we need to undo or neutralize with code somehow.

I really think we need "parser + custom doc builder" ((1) and (2) separated) instead of a "custom but general-purpose doc builder" ((1) + (2)).

wouterj commented 3 years ago

If the general-purpose doc builder is just as (if not more) configurable as the original parser, I don't see why it would be bad.

As I see it, we get more maintainers for both the parser and the docs builder. At the very least, this allows us to remove some features from the docs builder (as they are maintained by phpDocumentor Guides). In the most ideal scenario, this would allow us to remove everything but custom nodes, node visitors (to be implemented) and templates from the docs-builder.

mvriel commented 3 years ago

The goal with phpDocumentor is to provide a complete documentation solution that will help you write documentation in RestructuredText but also generate (Code) API Documentation; and allow linking between the two with complete support for multiple versions and components.

In all honesty, the Symfony documentation is our inspiration on which features to support since it is the best, most extensive and feature-rich documentation we currently see. Our conviction is that if we can provide a splendid solution for Symfony, including customizability, then it will excel for other projects as well.

As such, I find it hard to think of a situation where we would hurt the documentation efforts; but rather make it easier. Especially tasks such as crosslinking between different versions of documents is a very interesting use-case that we want to fully support.

mvriel commented 3 years ago

If you have any concerns or concrete examples where you see a possible issue; please do mention this. At the very least we could consider an extension/customization point for a challenge that falls beyond the realm that we can include in the core.

javiereguiluz commented 3 years ago

@mvriel first of all, thanks again for your offering of joining efforts. I think it's the smart thing to do. My disagreement is only about the technical implementation of this idea, so please don't see it as something against you or phpDocumentor 🙏

Now, let me try to explain again my point using another example:

phpDocumentor publishes three packages related to Reflection:

Why 3 packages? They all are about the same thing. Why not a single package that does many things related to Reflection? If someone needs to add or remove a feature provided by that super package, they can use the extensibility points to do that.

Or ... even better ... given that Reflection and phpDoc are super closely related, why not merge the code inside this other package:

https://github.com/phpDocumentor/phpDocumentor

We all know the answer: a stand-alone reflection package is great because it provides an entire useful feature that others can use to build higher level features. I think the same happens with a RST parser: it's a very complex feature that it's perfectly suited to be published as a stand-alone package for others to build higher level features (phpDocumentor Guides, Doctrine website builder, Symfony doc builder, etc.)


Finally, please remember that Symfony Doc Builder doesn't follow any good practices from the rest of the Symfony project. We can follow or break semver, we can introduce tons of BC breaks, we can add or remove support for PHP and Symfony versions as we wish, etc. We need to do that ... but we can't do that with a published package such as Guides.

I hope it's more clear now. Thanks!

mvriel commented 3 years ago

I would love to separate the RST Parser back into a separate repository; I choose to integrate it into phpDocumentor first with the intention to separate it again later because the original RST Parser had coupling in key area's that prevented us from integrating it into another project without major rework.

As you may have noticed yourself, the Gregwar RST Parser, and by extension Doctrine RST Parser, does not generate a standalone AST but rather has the whole parsing and transformation bit mashed together in the Nodes (if you check the SpanNode and SpanRenderer for example, it renders HTML during the parsing phase.

What we have done so far, is to integrate the library into phpDocumentor and start extracting all coupling related to rendering from the Nodes and change the TOC generation so that it can integrate into another over-arching TOC. This is a lot of work and we are not done yet, but my hope is that we can extract the library from phpDocumentor again (like we did with the libraries you mention above) as soon as it stabilizes and it only generates an AST that can be consumed by another application as a basis for rendering.

mvriel commented 3 years ago

As an FYI: at phpDocumentor, we have been reworking the DocumentParser to bring it more in line with the theory and concepts of a context-sensitive Recursive Descent Parser. The effect of this is that adding new features will become easier since every Production Rule is responsible for furthering the line cursor, instead of doing this in the DocumentParser and switching back and forth between a state.

This also introduces the ability for elements to process a subset of elements. An example is the List Item that only supports Body Elements; something which is impossible in the previous implementation.

In the upcoming weeks more iterations will be done to allow for better support on the ReST spec, and also to move the definitions of (Compound) Production Rules onto configuration so that custom Directive Handlers and Production Rules can be applied by projects.

See https://github.com/phpDocumentor/phpDocumentor/pull/3020 for the changes

mvriel commented 3 years ago

Oh, and these changes fix a couple of bugs as well that occur. Such as the lack of support for all title letters as mentioned in the spec

javiereguiluz commented 3 years ago

Mike, thanks a lot for your immense work to improve the RST parser 🙇 Having a fully-compliant RST parser in PHP (ideally as a stand-alone package) would be a dream come true 😍 Thanks

jaapio commented 3 years ago

When we are ready with the major refactoring steps we can start extracting the parser and maybe some related components from the main repository of phpdocumentor.

I can't promise when that will happen. But the moment will come for sure.

jaapio commented 3 years ago

An update from our side:

This week we started another enormous improvement in the parser. As many of you might now the parser and rendering are both using an Environment class which is used everywhere. This forces us to create the renderer and some related services every time when a new document is rendered. Since phpDocumentor has a service container, we wanted to have reusable services registered in the service container. For phpDocumentor this means that we are moving towards a option to render documents whenever we want. Like blocks of content that can be included in a twig template.

Extracting the environment also learned us a lot about the decoupling between the parser and renderers. Which will make us end with a pure parser, with a separate rendering layer.

https://github.com/phpDocumentor/phpDocumentor/pull/3032

mvriel commented 3 years ago

The quest continues:

We have just merged a change (https://github.com/phpDocumentor/phpDocumentor/pull/3048) where we have moved the Guides into a path-based composer package as an incubator package. This doesn't mean it is available on packagist soon; but with this step, I want to segregate the boundaries between the phpDocumentor main application and the upcoming library or libraries.

There is a new PR, https://github.com/phpDocumentor/phpDocumentor/pull/3049, in the works as well that splits the library even further where the main library provides the infrastructure and AST nodes, and subsequent libraries provide the translation for a specific language (such as RestructuredText and Markdown).

With phpDocumentor, it is our aim to at least support both RestructuredText and CommonMark Markdown; by making guides flexible enough by swapping 'transport' this should be doable without peppering the library with exceptions :D

(As an analogy, think symfony messenger with its redis-messenger and doctrine-messenger transports)

wouterj commented 3 years ago

Thanks for all the updates and work @mvriel and @jaapio, you're making incredible progress every week!

jaapio commented 2 years ago

We had some more time to refactor things more. I started to extract all rendering from the parser phase. The parser is now just using the renderer in de directives. That will be a next step to take. Restructure the way directives work.

The directives are at the moment enriching the nodes produced by the parser, and in some cases the directives are calling the renderer to render sub nodes. We will try to get rid of this connection, so we will have a cachable AST that can be rendered.

Main while the shortcuts we took to integrate the library in to phpDocumentor are coming to the surface. As I see it right now we will be able to start extracting those as will and create a solid integration layer that will allow people to use the guides library with the RestructuredText as a stand alone library.

I hope we can continue making progress as we are doing right now, so I can soon notify you all about a new phpDocumentor library that allows everyone to improve the documentation tools that are used by the projects.

jaapio commented 2 years ago

Beyond my own expectations I was able to refactor all existing directives and turn them in to normal nodes. :tada:

And the parser is now almost clean from any output format. This means that we can start thinking about a way we could add a cache layer, so we only have to parse changed files since the last run. We have a lot of tests to write, and many things can be improved. But I mark this step as a major step forward towards a real RST parser in php.

I will send you more updates once we are making progress again.

javiereguiluz commented 2 years ago

These are great news. Thanks for your efforts so far and we're excited about being able to test your parser refactor when it's ready. Please, continue keeping us posted about the progress. Thanks!

jaapio commented 2 years ago

Spend some more time to dig in the parser code. And I think that I discovered a missing link. A simple step that would allow a lot of advanced features.

phpDocumentor is using 3 major steps in its process.

  1. Parsing
  2. Compiling
  3. Rendering

Each stage produces an output, which could be stored in some way. However we do not store the compiled AST consumed by the rendering stage. As the compiler uses dynamic information from other nodes to enrich the output. Saving this output could result into a complex mechanism to ensure the cache is correctly cleared when a parsed file is updated.

In this library, at least the version we are working on right now in the phpDocumentor repo, doesn't have the compile step. And this looks like the missing link in the process. As far as I get it right now the parser is complete decoupled from the rendering step, but internally the parser adds each Document into a map called Metas. Which also contains the output url. This url makes it more or less impossible to change the output format without reparsing all documents.

I did a small experiment to see what would happen if we wouldn't create the normal output url as it does right now. And basically nothing happens... just all urls are incorrect. But the output is still generated. So I think when we would introduce an in-between step we would clear the paths to have a cached AST output from the parser.

This compiler step would also allow people to manipulate the AST after the parser is completed. I think this is part of #145. By simply adding more steps into the compiler, extra information can be added to the nodes when needed.

See you in the next update :wave:

jaapio commented 2 years ago

While I was trying a prototype of a possible caching layer I figured out that during the parsing phase of the parser a set of references is built to create the TOC for example. In my next update, this has been refactored. Allowing us to replace the way links are created.

In the phpDocumentor context this means that we can now render links like

:PHP:class:`phpDocumentor\Application`

Using our internal router, with this change the hacks are removed to link to project source code and other odd link handling. Allowing us and hopefully in the future you all to inject your own way of source code linking without any new directives or other complex stuff.

The other downside of the way references were handled, the parser always needed to run on each file. As we are now consuming the meta's, this dependency is gone. I will continue the experimental cache layer to see if we can skip the parsing of the unchanged files, and still be able to render them.

jaapio commented 2 years ago

Today was a great day, I'm in-between two customer projects which allowed me to have a full day focus on the rst rendering part. The result is good enough to share it with you again.

I was able to create an example script that is now doing some basic parsing and rendering using the new infrastructure of the library. For the people interested in what is now needed:
https://github.com/phpDocumentor/phpDocumentor/pull/3212/files#diff-2894f97ac5114facabb48d0348c126f0f0e02e299c3978ee1fc87819623486bb

When we started the refactoring we moved all the templates out of the library to be able to include them in the phpDocumentor templates. Today I implemented a way in phpDocumentor itself that allows us to move the templates back into the library. Besides that I added an extra layer to allow phpDocumentor to manipulate the twig environment creation as we need some more global variables which are not required by the rst lib.

This brings us yet another step closer to the extraction of the library in a separate project. As the example file shows you it is runnable as a stand-alone lib again with some bootstrapping and wiring. Tomorrow I will continue to improve the setup. But also want to have a look at the functional tests this repo contains. As that will tell me how much it will take to align both projects.

wouterj commented 2 years ago

Very cool!

I've just tried it out on the Symfony docs, and (as expected) I get quite a lot of PHP warnings about little things that we've fixed in this library in the past 2 years. I guess after the Guides extraction is finished, we can create a plan on how to include the bug fixes/features from this project in Guides (without having to redo all the tedious debugging efforts). E.g. I noticed none of the references (the `Something`_ things) are parsed in the Symfony docs at the moment.

Also, I guess it's already known, but information about the original file/directory names is not included in the document renderer - making it impossible to render a directory of documents.

All in all, it's great to actually see the progress locally!

jaapio commented 2 years ago

@wouterj can you elaborate on what you executed? Did you try the example I linked on the Symfony docs?

javiereguiluz commented 2 years ago

@jaapio thanks for sharing your progress.

I haven't tested it but I want to ask you something. It's about a shortcoming of the current parser that I faced recently. For some reason, I needed to access the filename (or the absolute path) of the RST file being parsed inside a directive PHP class (e.g. CodeBlockDirective or AdmonitionDirective).

Apparently there's no way to get that information, or any other context information, inside directive classes. So, my question: is there some "context object" or similar mechanism to access this kind of context metadata from any class related to the parser? Thanks!

wouterj commented 2 years ago

@jaapio I've just pushed my local test project https://github.com/wouterj/symfony-docs-guides (I sort of made up the instructions, might not work 100%, but it does tell you the steps you need)

jaapio commented 2 years ago

@javiereguiluz I'm not sure how this works in the current version you are using, but in the phpDocumentor version we have a ParserContext which is an extraction of the Environment class in the version you are using. This contains in our case the absolute path on the filesystem. The path handling is quite messy in the original version. I'm having a hard time today as well to get this working properly.

If I remember it correctly the getCurrentFileName will give you the file path without the extension. I hope this helps, otherwise please find me on slack. That will make it easier to answer questions quickly.

@wouterj thanks, I will have a look. I did some major rework on the span handling which did this linking behavior so it's kind of expected that I broke some thinks in there :-P

jaapio commented 2 years ago

Based on the test @wouterj did I was able to fix some small bugs:

The recent changes to make the library work stand alone introduced some more issues regarding paths. including the reference links and TOCtree. I think I will be able to fix those tomorrow. As I have yet another day to work on the library. When those issues are resolved I will make a start to split the libraries into separate repositories.

There are still a lot of improvements that can be done in the parser to make the code cleaner and better to maintain. But extracting it now will help us to keep it disconnected from the phpDocumentor core. And all major refactor work is done.

jaapio commented 2 years ago

I'm very happy to announce that I fixed most of the issues @wouterj mentioned in a slack message after he did his experiment.

This means that I will create separate repositories for the phpdocumentor/guides libraries. Which will bring us a very big step forward toward a proper parser library in PHP for RST! I will let you know when the repositories are ready. So we can start planning the port of the improvements you did in the library.

jaapio commented 2 years ago

Still under heavy development, as it turned out that we need to restructure the table parsing for this library will have it's final shape.

https://github.com/phpDocumentor/guides

I have a few questions:

stof commented 2 years ago

@jaapio to me, the plantuml support should probably be kept separate from the main library, relying on some extension point instead (then, whether it is in phpDocumentor itself or in a separate reusable package is a topic that can be discussed, but AFAIK the symfony docs does not rely on UML)

wouterj commented 2 years ago

Nice! Am I correct that this only includes the guides package, and not the restructured text package?

Also, fyi I just tried out my project again (with the incubator packages from phpDocumentor master) and got a hard error on my build script (which seems to be caused by the package):

PHP Fatal error:  Uncaught ArgumentCountError: Too few arguments to function phpDocumentor\Guides\RestructuredText\Directives\AdmonitionDirective::__construct(), 0 passed in /home/wouter/projects/github.com/symfony/symfony-docs-phpdoc/lib/phpDocumentor/incubator/guides-restructured-text/src/RestructuredText/MarkupLanguageParser.php on line 74 and exactly 1 expected in /home/wouter/projects/github.com/symfony/symfony-docs-phpdoc/lib/phpDocumentor/incubator/guides-restructured-text/src/RestructuredText/Directives/AdmonitionDirective.php:20
Stack trace:
#0 /home/wouter/projects/github.com/symfony/symfony-docs-phpdoc/lib/phpDocumentor/incubator/guides-restructured-text/src/RestructuredText/MarkupLanguageParser.php(74): phpDocumentor\Guides\RestructuredText\Directives\AdmonitionDirective->__construct()
#1 /home/wouter/projects/github.com/symfony/symfony-docs-phpdoc/build.php(49): phpDocumentor\Guides\RestructuredText\MarkupLanguageParser::createInstance()
#2 {main}
  thrown in /home/wouter/projects/github.com/symfony/symfony-docs-phpdoc/lib/phpDocumentor/incubator/guides-restructured-text/src/RestructuredText/Directives/AdmonitionDirective.php on line 20

Also a quick question, for when we start contributing little fixes: Is Guides going to become a fully working repository? I.e. should we contribute PRs to this repository or will phpDocumentor become the monorepo?


phpDocumentor supports rendering of uml diagrams, We implemented this as a special node type in this library. This requires plantuml to be installed but also requires some additional code. would it be interesting for you as well to have this in this library? or should we move this into phpDocumentor specific libraries?

We don't have a use-case for this in the Symfony docs. We only have a handful diagrams and have defined a custom standard using Dia (not recommended btw).

Could somebody provide some information about what is changed in the list processing? I saw some merged prs. As we need to rework the table processing, are there any specific issues you have faced using the original library that we need to keep in mind?

Regarding these, I think your original proposal would be a good start: moving the functional tests from this and symfony-tools/docs-builder to the Guides repository. All bugs that we've fixed in the past years are covered by a functional test. That would give us a clear indication of the missing fixes.

Some open issues about tables are https://github.com/doctrine/rst-parser/issues/157 and https://github.com/doctrine/rst-parser/issues/105

jaapio commented 2 years ago

Yes this first repo only contains the nodes and rendering part. As we are planning te support other input formats those will be plugins on this library.

I think I prefer to have separate repo's and not a mono repo structure. All phpdocumentor repo's are independent projects. But I'm open for suggestions, as you have more experience with a mono repo.

I'm still working on the split, this might be the cause of the error you are facing. Unfortunately my 3 days of focus on this project are over. So progress will slow down again. I will keep you posted on what is going on.

Next stop is refactoring the table node. As this is still in the old shape. I must have missed that. It is to much connected to the rst format. All other nodes are generic structures now.

wouterj commented 2 years ago

I'm still working on the split, this might be the cause of the error you are facing. Unfortunately my 3 days of focus on this project are over. So progress will slow down again. I will keep you posted on what is going on.

Thanks a lot for the work and enthusiasm this week! You've made big steps towards decoupling it from phpDocumentor :)

I think I prefer to have separate repo's and not a mono repo structure. All phpdocumentor repo's are independent projects. But I'm open for suggestions, as you have more experience with a mono repo.

I think it makes sense to develop Guides separately from phpDocumentor. This way, you might assign people to allow helping you on Guides without making them also phpDocumentor maintainers. I personally do favor to combine all guide repositories (guides + the format specific guides) in a monorepository. I guess you often end up having to tweak code in either 3 libraries when you update something, and doing this in 1 PR instead of 3 reduces a lot of overhead in my experience. But ultimately, it's a phpDocumentor package and thus up to you to decide :)


Meanwhile, today was my time to play with this package. I've updated my specialized docs repository with a lib/docs-builder "package": https://github.com/wouterj/symfony-docs-guides/tree/main/lib/docs-builder This allows us to more formally test the Guide package (and test out adding custom themes, directives and roles). I've done 4 things:

This along with https://github.com/phpDocumentor/guides/pull/2 and https://github.com/phpDocumentor/phpDocumentor/pull/3218 allows me to render the Symfony docs using the rtd theme :tada:

I've noticed a couple other bugs in a quick review, which I've listed in the repository's readme: https://github.com/wouterj/symfony-docs-guides#readme

All in all, I'm pretty amazed with the current state of the project. Apart from the bugs, which obviously are expected with a project that has had such major refactorings in 2 separate branches, things are truly working pretty good.

jaapio commented 2 years ago

I decided to have a mono-repo for all guides related packages. This will help us to use the existing tests, but also maintain the libraries. As I do agree with you that in most cases we will need to add new nodes to the core project to be able to support them in the format-specific libraries.

I used the same repo. The current guides package will be renamed to guides-core as it will contain the core code to have guides rendering. I'm open to better names. :-D

stof commented 2 years ago

The current package could keep the existing name, if you don't publish the mono-repo to Packagist.

jaapio commented 2 years ago

I thought packagist forces the name of a repo to be equal to the name of the package since composer 2.

stof commented 2 years ago

No. It never did that. The repo URL can be what you want.

What composer 2 did is forcing package names to be lowercase (instead of making them case-insensitive) and forbidding some chars in them that are not safe for usage in filesystem paths or URLs (packagist.org was already blocking those btw)

wouterj commented 2 years ago

Cool, looks great. Let's keep phpdocumentor/guides for the core package, that makes the most sense to me.

If you have any questions about splitsh, feel free to reach out to the Symfony team :)

javiereguiluz commented 2 years ago

Maybe I'm not understanding things well, but I expected these packages:

phpdocumentor/rst-parser
phpdocumentor/guides
phpdocumentor/...

"rst-parser" would be the package required by Symfony and other projects needing RST parsing. "guides" would be the RST/formatting customizations needed by PHPDocumentor and it would be similar to the docs-builder Symfony package, which adds customizations on top of Doctrine's rst-parser

jaapio commented 2 years ago

What the markup specific packages like the RST package are doing is parsing an text format into an AST, the guides lib contains the rendering part of the AST.

So the symfony docs-builder will use the guides library. and the guides-restructuredtext.

phpdocumentor specific stuff is part of our application. And will not be exposed in a separate repo.

wouterj commented 2 years ago

You can think of phpdocumentor/guides as replacement for the hardcore stuff in symfony-tools/docs-builder (like rendering). Unlike rst-parser, it'll do a bit more than just parsing reStructured Text. Which is great news, the more complexity is maintained by a bigger team, the better :)

Our symfony-tools/docs-builder package can be stripped down quite a bit this way. It'll only contain the custom theme for symfony.com and our custom directives and roles (e.g. :class: and .. versionadded::). You can see how Guides and Docs-Builder work together in my basic tryout: https://github.com/wouterj/symfony-docs-guides/tree/main/lib/docs-builder

javiereguiluz commented 2 years ago

I think I disagree 🤔 For example, if this library does things such as traverse directories to look for RST files and then parses them ... this might be a drawback if we have other requirements:

(1) Traverse directories in certain way, excluding some dirs and some files applying certain custom logic (2) Loading RST contents in memory, not via files, because we're parsing things on-the-fly (e.g. a blog post written in RST)

These are not imaginary examples, these are real requirements that I have today when using RST.

I know I'm insisting a lot ... but I think it'd be a mistake to have anything different from "a pure RST parser" + "other independent libraries that implement rich features on top of it".