Skip to content

Grammar: Reduce and rename for clarity#6127

Merged
mcsf merged 1 commit intomasterfrom
update/grammar-reduce-and-rename
Apr 17, 2018
Merged

Grammar: Reduce and rename for clarity#6127
mcsf merged 1 commit intomasterfrom
update/grammar-reduce-and-rename

Conversation

@mcsf
Copy link
Copy Markdown
Contributor

@mcsf mcsf commented Apr 11, 2018

Description

Spin-off from #6116.

Grammar changes:

  • Rename Token to Block
  • Rename Block_Void to Block_Empty
  • Port WS+ to __, merging together white-space rules

How Has This Been Tested?

Screenshots (jpeg or gifs if applicable):

Types of changes

Checklist:

  • My code is tested.
  • My code follows the WordPress code style.
  • My code has proper inline documentation.

@mcsf mcsf added the [Feature] Parsing Related to efforts to improving the parsing of a string of data and converting it into a different f label Apr 11, 2018
@mcsf mcsf requested a review from dmsnell April 11, 2018 20:02
Token
= Block_Void
Block
= Block_Empty
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

It’s worth considering the context in which void was chosen as a name. It borrows from the HTML spec which specifies certain tags as void and those are not allowed to have a closing tag - hr, img, br, etc... in Gutenberg it’s optional the distinction between empty and void but the intention was to convey void elements, not to present an optimization for empty elements

Copy link
Copy Markdown
Member

@dmsnell dmsnell left a comment

Choose a reason for hiding this comment

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

I should clarify what I was saying about Void. I approve these changes though I think it may still be worth considering not changing Void to Empty because they carry different semantic nuances.

Make your choice; I support this either way.

* Rename Token to Block
* Rename Block_Void to Block_Empty
* Port WS+ to __
* Update PHP parser
@mcsf mcsf force-pushed the update/grammar-reduce-and-rename branch from 970fa74 to 446b67d Compare April 17, 2018 15:13
@mcsf
Copy link
Copy Markdown
Contributor Author

mcsf commented Apr 17, 2018

Thanks for the review and confidence, Dennis. Your comment on void/empty makes a lot of sense.

I'll keep Block_Void, then. What I think we'll do with/after #6116 is add comments to the rules, and one for Block_Void would include your HTML-spec-inspired reasoning, and that it "represents an empty block", so to speak. Whatever makes the grammar doc easier to read.

@mcsf mcsf merged commit 1d05c53 into master Apr 17, 2018
@mcsf mcsf deleted the update/grammar-reduce-and-rename branch April 17, 2018 15:29
@dmsnell
Copy link
Copy Markdown
Member

dmsnell commented Apr 17, 2018

The point is that void blocks aren’t empty blocks but thy are blocks that cannot contain content. Maybe we muddy that in our parser/serializer. If it ends up that actually we use it as a shorthand then we may actually want to rename it. Nice work in any case!

nuzzio pushed a commit to nuzzio/gutenberg that referenced this pull request Apr 25, 2018
* Rename Token to Block
* Rename Block_Void to Block_Empty
* Port WS+ to __
* Update PHP parser
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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants