Skip to content
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 documents starting with a literal block marker #134

Merged
merged 1 commit into from
Mar 8, 2021

Conversation

wouterj
Copy link
Collaborator

@wouterj wouterj commented Mar 5, 2021

This fixes https://github.com/weaverryan/docs-builder/issues/36:

In EasyAdmin docs we have the following in https://github.com/EasyCorp/EasyAdminBundle/blob/master/doc/fields.rst

Design Options
~~~~~~~~~~~~~~

::

    TextField::new('firstName', 'Name')
        // ...

In current symfony.com it's correctly displayed as a code block: [...] However, docs-builder parses it as a blockquote

Related reStructuredText specs section: https://docutils.sourceforge.io/docs/ref/rst/restructuredtext.html#literal-blocks I'm a bit unsure about the tests that I modified. Looking at this spec and Sphinx behavior, the previously expected HTML is not correct. They appear to be introduced in cb831fb, so that also doesn't reveal any original reasoning behind these tests.


This was already implemented 😄
Meanwhile, I also took some freedom to slightly modify how the functional tests are run. This allows running a specific functional test using:

vendor/bin/phpunit tests/Functional/FunctionalTest.php \
    --filter testFunctional@code-html

This is very useful for me, as I need to var_dump quite a lot to make sense of this new code base.


I'm a bit unsure what version to use for this repository: there is 0.1.x, 0.2.x, 1.0.x and master(and I couldn't find any contributing guidelines, probably because this is a quite internal project). So I just took the default one. Please let me know if this is wrong and I need to rebase.

@greg0ire
Copy link
Member

greg0ire commented Mar 6, 2021

Hi @wouterj! Are you aware of #132? I think it might be related. I think I will be able to make the situation clearer when I am back at my desktop in 30 hours or so.

@wouterj
Copy link
Collaborator Author

wouterj commented Mar 6, 2021

Yes, I am. Unless I'm missing something, this "bug"/missing feature does exists in all branches of the repo

@greg0ire
Copy link
Member

greg0ire commented Mar 6, 2021

OK just checking, because all this sudden interest for the package looked suspicious :)

@wouterj
Copy link
Collaborator Author

wouterj commented Mar 6, 2021

@greg0ire fyi, we're going to use this package in a new section on symfony.com. So we're now polishing lots of the great work done by Ryan over the past 2 years to try out this package on the Symfony docs. You can expect some activity from us the next weeks (and hopefully overall more activity in the years to come).

@greg0ire
Copy link
Member

greg0ire commented Mar 7, 2021

Ok so since this is a bugfix, 0.2.x is definitely the correct branch 👍
It might become incorrect very soon if I release 1.0.0 , in which case you will have to target 1.0.x

@greg0ire greg0ire added the Bug Something isn't working label Mar 7, 2021
@greg0ire greg0ire requested a review from weaverryan March 7, 2021 20:19
@greg0ire
Copy link
Member

greg0ire commented Mar 7, 2021

They appear to be introduced in cb831fb

Neither does bae8bc7@jwage , can you please help with this?

@wouterj
Copy link
Collaborator Author

wouterj commented Mar 7, 2021

Neither does bae8bc7@jwage , can you please help with this?

That's a great find, as that commit does show the correct behavior (<pre><code>) whereas the commit I found changed it to <blockquote> (which is wrong, according to the reStructuredtext spec)

@greg0ire
Copy link
Member

greg0ire commented Mar 7, 2021

Indeed! I didn't spot that subtle change. I found that commit with git log -S 'Test code block with whitespace'

@greg0ire greg0ire changed the base branch from 0.2.x to 0.3.x March 7, 2021 21:40
@greg0ire
Copy link
Member

greg0ire commented Mar 7, 2021

There is a conflict you will need to fix.

@greg0ire greg0ire added this to the 0.3.1 milestone Mar 7, 2021
@wouterj
Copy link
Collaborator Author

wouterj commented Mar 7, 2021

Thanks for the notification! Rebased :)

Copy link
Contributor

@javiereguiluz javiereguiluz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks Wouter!

@greg0ire greg0ire merged commit 7907706 into doctrine:0.3.x Mar 8, 2021
@greg0ire
Copy link
Member

greg0ire commented Mar 8, 2021

Thanks @wouterj !

@wouterj wouterj deleted the advanced-literal-blocks branch March 8, 2021 20:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Minor issue in the parsing of code blocks with automatic PHP highlighting
3 participants