pest-parser / pest

The Elegant Parser
https://pest.rs
Apache License 2.0
4.67k stars 261 forks source link

Tags do not apply to silent rules #1035

Closed phi-go closed 2 months ago

phi-go commented 2 months ago

Describe the bug A clear and concise description of what the bug is:

Probably best seen by the examples below, though, happy to add further information.

To Reproduce Steps to reproduce the behavior:

Here is an example where I would expect to get the matches to "1" under tag "start" and "3" for tag "end". https://pest.rs/?g=N4Ig5gTghgtjURALhNAdmApgAgLzeGwB0QAKE7AP21PSwH0A3KAGwFdMBnASiuJG4UAvkTR1MTVh054C2AMTj6nAC4IVsgIIBlAMIBJffQAi%2BgOL6AKgGo%2BJAHT2K1RVAwTMaACZa9hk%2BZWtiJooiAANCAAlmgADmwqyGQAjI4AzIJoIEJAA#editor

This example can be made to work when promoting the built-in rule to a separate rule: https://pest.rs/?g=N4Ig5gTghgtjURALhNAdmApgAl33AvNsNgDogAU52AftulgPoBuUANgK6YDOtZIASmoBfUmgaYW7LryIkAxNwAuCJdiIATAJZgtS3nXIA6I9TrzMaDeuzbd%2B7KKs69vfIWLYAggGUAwgCSAYwAIgEA4gEAKgDUjmIgADQgWmgADhxKyJQAjCYAzEJoIMJAA#editor

Additionally, it seems that rules on built-in tags can override otherwise working tags: https://pest.rs/?g=N4Ig5gTghgtjURALhNAdmApgAl33AvNsNgDogAU52AfthelgPoBuUANgK6YDOAlLTIg%2B1AL6k0jTKw7ce2IiQDEPAC4JVC7ABMAlmF2r5dcgDpT1OksxptWgIIBlAMIBJV0wAirgOKuAKgDU2OK2%2Boby%2BITE2E5uHt5%2BQSESIAA0ILpoAA6cqsiUAIzmAMwiaCCiQA#editor

Also, probably a separate issue but I think the share link should also store the selected rule. I can add another issue for that if you want.

Expected behavior A clear and concise description of what you expected to happen:

See examples above. Though, this could probably also just be updated in the documentation or having a check that disallows having tags on built-in rules.

Additional context Add any other context about the problem here.

tomtau commented 2 months ago

Also, probably a separate issue but I think the share link should also store the selected rule. I can add another issue for that if you want.

@phi-go you can open an issue for it here: https://github.com/pest-parser/site/issues

phi-go commented 2 months ago

Done: https://github.com/pest-parser/site/issues/61

tomtau commented 2 months ago

@phi-go I think this issue is because built-in rules are "silent" (i.e. they don't produce output parse nodes). The same behaviour happens when those separate rules are made silent:

https://pest.rs/?g=N4Ig5gTghgtjURALhNAdmApgAl33AvNsNgDogAU52AftulgPoBuUANgK6YDOtZIASmoBfUmgaYW7LryIkAxNwAuCJdiIATAJZgtS3nXIA6I9TrzMaDeuzbd%2B7KKs69vfIWyMSAQQDKAYQBJQMYAEUCAcUCAFQBqRzEQABoQLTQABw4lZEoARhMAZiE0EGEgA#editor

With parts of expressions that are silent rules (including built-in rules), there won't be any output nodes to attach tags onto. But yes, I think this can be better clarified in the book/docs.

phi-go commented 2 months ago

Ah, I see. I can give the doc update a try if you want, I think it should at least be mentioned in this section. Though, in my opinion there should probably be a warning (maybe an error) that a tag on a silent rule will be ignored, as this can be quite confusing.

Also, I think the third case in my original message is an actual bug and should be fixed in some way.

tomtau commented 2 months ago

@phi-go sure, feel free to go ahead to propose changes to that book section. I agree it'll be good to see if the validation logic can be extended to check for these tag cases: https://github.com/pest-parser/pest/blob/master/meta/src/validator.rs

tomtau commented 2 months ago

@phi-go you can check if errors like this are OK: https://github.com/pest-parser/pest/pull/1036/files#diff-44015bd7886a43717eb7a6a5a281e264f75128ab44dbf7a1a731920f441816e7R1879

phi-go commented 2 months ago

That was quick. Yeah these look good to me, they would have been very useful.