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

Some documentation improvements in WP_Block_Parser_Block. #47947

Open
costdev opened this issue Feb 9, 2023 · 4 comments
Open

Some documentation improvements in WP_Block_Parser_Block. #47947

costdev opened this issue Feb 9, 2023 · 4 comments
Labels
[Feature] Parsing Related to efforts to improving the parsing of a string of data and converting it into a different f Good First Issue An issue that's suitable for someone looking to contribute for the first time Needs Dev Ready for, and needs developer efforts [Type] Code Quality Issues or PRs that relate to code quality

Comments

@costdev
Copy link
Contributor

costdev commented Feb 9, 2023

Description

There are some documentation improvements suggested in the WP_Block_Parse_Block class. These changes were originally committed to Core (references at the end), but resulted in a Ensure version-controlled files are not modified or deleted failure in GitHub Actions.

These included:
@since tags

  • WP_Block_Parser::$empty_attrs
  • WP_Block_Parser::next_token()
  • WP_Block_Parser::freeform()
  • WP_Block_Parser_Block::$innerContent

The @since tags referred to early Gutenberg versions instead of WordPress core. These properties and methods were introduced in WordPress 5.0, so 5.0.0 is the correct version.

It is suggested that some of the other @since tags are removed, as they are related to early Gutenberg development before it was merged into WordPress core, and are not relevant for core.

Return value

  • WP_Block_Parser::parse() - This appears to have been changed to array[] already.
  • WP_Block_Parser::$output**

Both the method and the property are documented as returning WP_Block_Parser_Block[] (an array of WP_Block_Parser_Block objects), but the result is in fact an array of arrays of various values, so array[] is the correct notation.

Step-by-step reproduction instructions

Related Trac ticket: https://core.trac.wordpress.org/ticket/56581

Related changesets:

Screenshots, screen recording, code snippet

No response

Environment info

No response

Please confirm that you have searched existing issues in the repo.

Yes

Please confirm that you have tested with all plugins deactivated except Gutenberg.

Yes

@Mamaduka
Copy link
Member

That's an unexpected error for this class and file. While we maintain a few classes and block files in this repo, we don't maintain WP_Block_Parse_Block anymore.

You were correct, and changes should be committed to the Core.

@costdev
Copy link
Contributor Author

costdev commented Feb 10, 2023

Thanks @Mamaduka! I've left a comment on the Trac ticket and will follow up here once there's confirmation that all went through as expected so that this issue can be closed. 🙂

@gziolo gziolo added [Feature] Block API API that allows to express the block paradigm. [Type] Code Quality Issues or PRs that relate to code quality labels Feb 11, 2023
@gziolo
Copy link
Member

gziolo commented Feb 11, 2023

The file is located in:

https://github.com/WordPress/gutenberg/blob/trunk/packages/block-serialization-default-parser/parser.php

It gets automatically updated in WordPress core when backporting changes from npm packages:

https://github.com/WordPress/wordpress-develop/blob/099ff6c5fe50c9009c79f9bbb9a0422db5e2f51f/tools/webpack/packages.js#L102

You were correct, and changes should be committed to the Core.

As far as I'm aware, the changes in WP core will get overridden with the file from Gutenberg because of the webpack line I shared. I think it's intended. That would also explain why there was an error on CI.

@Mamaduka
Copy link
Member

Oh, thank you, @gziolo!

My grep skills missed that somehow 😞

@gziolo gziolo added Good First Issue An issue that's suitable for someone looking to contribute for the first time Needs Dev Ready for, and needs developer efforts labels Feb 17, 2023
@gziolo gziolo mentioned this issue Feb 17, 2023
58 tasks
@gziolo gziolo added [Feature] Parsing Related to efforts to improving the parsing of a string of data and converting it into a different f and removed [Feature] Block API API that allows to express the block paradigm. labels Mar 3, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Parsing Related to efforts to improving the parsing of a string of data and converting it into a different f Good First Issue An issue that's suitable for someone looking to contribute for the first time Needs Dev Ready for, and needs developer efforts [Type] Code Quality Issues or PRs that relate to code quality
Projects
None yet
Development

No branches or pull requests

3 participants