Add multiline comments to Pest's PEG format

[felix91gr]

Implementation of #330. At least, that's what I think.

Probably needs tests. How do I make one, or update the existing ones?

And how can I test it manually?

Thanks <3

[CAD97]

If we want this to mirror Rust's block comments, they should probably nest:

block_comment = _{ "/*" ~ (block_comment | !"*/" ~ ANY)* ~ "*/" }

You can test by patching a use site to use a path dependency on your local pest (or your git branch).

[felix91gr]

You can test by patching a use site to use a path dependency on your local pest (or your git branch).

Yo, that is awesome! It took me less than 5 minutes to make that work and test it with my project :D (PS: it accepts my multiline comments now c: )

If we want this to mirror Rust's block comments, they should probably nest

I have no opinion on this, but it sounds better. I didn't think of nested multiline comments, that's probably a pretty good use case. It also makes more sense from the ergonomics standpoint: when I used to work in C#, the multiline comments were as I have just implemented them and it was very weird that a single */ could stop all the /* in the universe. Letting them be nested makes it a lot more useable, I think.

So, what's missing now are some tests for this, I'd think. Where can I put such a test? Something like a couple of example grammars with multiline comments would be nice to have as a test :)

[felix91gr]

Now I've uploaded an update that makes multiline comments nestable. Thanks @CAD97 ^^

[felix91gr]

Now that it has tests, I think the PR is ready for review. What do you think?

This is what's implemented:

• Multiline comments
• Test for single-lined comments
• Test for multiline one-line comments
• Test for multiline n-line comments
• Test for nested:
  • Single line in multiline
  • (Multiline in) multiline
  • Invalid grammar pattern definition inside multiline: if there are rules generated from inside a multiline comment, this should fail to produce a parse tree. If, on the other hand, definitions inside multiline comments are ignored, then this test should pass.

[dragostis]

Thanks a lot for working on this! Everything looks good, but I'd like to ask a favor: will you please symlink all *.pest files from derive/tests in vm/tests so they don't have to be updated separately? As I said before, my testing can get sloppy sometimes...

[felix91gr]

Will you please symlink all *.pest files from derive/tests in vm/tests so they don't have to be updated separately?

Sure, I can do that. But is that something related to this PR (like, something I might have broken), or is it something else that the repo needs and hasn't been done yet? If it's the second, then maybe it should be done in a separate PR? To separate these two additions and all that. Or maybe just in a separate commit, and then we can merge two commits with the same PR. What do you think?

[dragostis]

It should be a separate PR, yes, but it's a very small change I'd hoped to get away with. 😃 We're currently not squashing PR commits before merging, so the whole history is there.

[felix91gr]

Hahaha, okay 😄
I'll do it, then. Which files should link where?

Something like: "for each file file in vm/tests, file should link to derive/tests/file"?

[dragostis]

Yes, but only the *.pest grammar files need to be replaced.

[felix91gr]

Okay, I'll add it :)

[felix91gr]

Added! :)

[dragostis]

bors r+

[bors]

Build succeeded

• continuous-integration/travis-ci/push

[felix91gr]

Wooo :D

[felix91gr]

⛵