Skip to content

Bunch of fixes for PHP and Twig parsing #44

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 3 commits into from
Jun 13, 2022
Merged

Bunch of fixes for PHP and Twig parsing #44

merged 3 commits into from
Jun 13, 2022

Conversation

HeahDude
Copy link
Contributor

@HeahDude HeahDude commented Jun 9, 2022 •
edited
Loading

Summary of my attempt to run the script locally against 4.4 version of the docs:

@HeahDude HeahDude mentioned this pull request Jun 9, 2022
Closed
@Nyholm
Copy link
Member

Nyholm commented Jun 10, 2022

When I run your PR I dont get the same errors...

➜  code-block-checker git:(pr-44) ✗ php ./code-block-checker.php verify:docs /my/path/symfony-docs testing.rst cache.rst controller.rst --env=dev --symfony-application /my/path/symfony-application

I get some errors, but they are all expected:

[PHP syntax] Syntax error, unexpected T_VARIABLE, expecting ':' in file testing.rst at 517
[PHP syntax] Syntax error, unexpected ',' in file testing.rst at 517
[PHP syntax] Syntax error, unexpected ',' in file testing.rst at 518
[PHP syntax] Syntax error, unexpected ',' in file testing.rst at 519
[PHP syntax] Syntax error, unexpected ',' in file testing.rst at 520
[PHP syntax] Syntax error, unexpected ')' in file testing.rst at 522

@HeahDude
Copy link
Contributor Author

This is the result I get for the whole repository (. as argument instead of files), sorry for the confusion.

@Nyholm
Copy link
Member

Nyholm commented Jun 10, 2022

Ah. Then I think you are all correct.

The docs are broken in many places =)

@HeahDude HeahDude mentioned this pull request Jun 11, 2022
Closed
Nyholm
Nyholm approved these changes Jun 11, 2022
View reviewed changes
Copy link
Member

@Nyholm Nyholm left a comment

Choose a reason for hiding this comment

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

Im happy with this PR. Just remove the log.txt.

@HeahDude
Copy link
Contributor Author

Alright then!
Maybe I should also revert the last commit about ellipsis given the feedback from the docs (symfony/symfony-docs#16868 (comment)), WDYT?

@Nyholm
Copy link
Member

Nyholm commented Jun 13, 2022

Na, I liked that one. Let's keep it

@HeahDude HeahDude force-pushed the add-missing-packages branch from 25945c2 to 2f591b4 Compare June 13, 2022 10:11
@HeahDude HeahDude changed the title [WIP] Try to fix 4.4 docs parsing Bunch of fixes for PHP and Twig parsing Jun 13, 2022
@HeahDude
Copy link
Contributor Author

Just remove the log.txt.

Done and rebased, PR title and description updated.

Nyholm
Nyholm approved these changes Jun 13, 2022
View reviewed changes
Copy link
Member

@Nyholm Nyholm left a comment

Choose a reason for hiding this comment

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

Thank you

@Nyholm Nyholm merged commit 7934f6d into symfony-tools:main Jun 13, 2022
@HeahDude
Copy link
Contributor Author

Thank you for these tools, your help and merging <3

@HeahDude HeahDude deleted the add-missing-packages branch June 13, 2022 11:33
@Nyholm
Copy link
Member

Nyholm commented Jun 13, 2022

Thank you.

Let's see if it helps in the symfony-docs repo

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Nyholm Nyholm Nyholm approved these changes

Assignees
No one assigned
Labels
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@HeahDude @Nyholm