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

Tags do not apply to silent rules #1035

Closed
phi-go opened this issue Sep 2, 2024 · 7 comments · Fixed by #1036
Closed

Tags do not apply to silent rules #1035

phi-go opened this issue Sep 2, 2024 · 7 comments · Fixed by #1036
Labels

Comments

@phi-go
Copy link

phi-go commented Sep 2, 2024

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.

@phi-go phi-go added the bug label Sep 2, 2024
@tomtau
Copy link
Contributor

tomtau commented Sep 2, 2024

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
Copy link
Author

phi-go commented Sep 2, 2024

Done: pest-parser/site#61

@tomtau
Copy link
Contributor

tomtau commented Sep 3, 2024

@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
Copy link
Author

phi-go commented Sep 3, 2024

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.

@phi-go phi-go changed the title Tags do not apply to built-in rules Tags do not apply to silent rules Sep 3, 2024
@tomtau
Copy link
Contributor

tomtau commented Sep 3, 2024

@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
Copy link
Contributor

tomtau commented Sep 5, 2024

@phi-go
Copy link
Author

phi-go commented Sep 5, 2024

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants