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

Minor inconsistency between pest meta grammar and some tools #853

Open
Tartasprint opened this issue May 4, 2023 · 1 comment
Open

Minor inconsistency between pest meta grammar and some tools #853

Tartasprint opened this issue May 4, 2023 · 1 comment

Comments

@Tartasprint
Copy link
Contributor

Describe the bug
There is a difference between how pest_meta and the other tools/libraries handle the ' in range expressions.
In meta/src/grammar.pest the following is a valid range expression '''..''' as there is no check for it:

pest/meta/src/grammar.pest

Lines 167 to 179 in 7f8411a

range = { character ~ range_operator ~ character }
/// A single quoted character
character = ${ single_quote ~ inner_chr ~ single_quote }
/// A quoted string.
inner_str = @{ (!("\"" | "\\") ~ ANY)* ~ (escape ~ inner_str)? }
/// An escaped or any character.
inner_chr = @{ escape | ANY }
/// An escape sequence.
escape = @{ "\\" ~ ("\"" | "\\" | "r" | "n" | "t" | "0" | "'" | code | unicode) }

There is no check in the meta's parser either:

pest/meta/src/parser.rs

Lines 428 to 436 in 7f8411a

Rule::range => {
let mut pairs = pair.into_inner();
let pair = pairs.next().unwrap();
let start = unescape(pair.as_str()).expect("incorrect char literal");
let start_pos = pair.clone().as_span().start_pos();
pairs.next();
let pair = pairs.next().unwrap();
let end = unescape(pair.as_str()).expect("incorrect char literal");
let end_pos = pair.clone().as_span().end_pos();

Meanwhile other tools do not accept this syntax.
The highlighter for the playground has this line for handling ranges:

{ regex: /'(?:[^'\\]|\\(?:[nrt0'"]|x[\da-fA-F]{2}|u\{[\da-fA-F]{6}\}))'/, token: "string" },

And a similar rule seem to be defined for IDEs syntax highlighting.

Searching for usage the unescaped version ''' in public GitHub repositories gave no results, and some results for the escaped version '\''.

To Reproduce
Writing the following pest grammar in a file using a supported IDE or in the playground:

r={'''..'''}

Is deemed valid by Pest, the rule r matches the input ', but breaks the syntax-highlighting. Might break other things, I didn't check.

Expected behavior
I expected the behavior of the different tools listed up there to be consistent.

Additional context
I don't know if ''' being valid was a design choice that wasn't followed in the playground/ide tools, or if checking for this edge case was simply forgotten in pest meta grammar. I'm aware this "bug" doesn't have a great impact (maybe none?), but still I think it should be addressed. I'd happily submit changes to address it, just not sure if that is wanted.

@tomtau
Copy link
Contributor

tomtau commented May 4, 2023

I don't know if ''' being valid was a design choice that wasn't followed in the playground/ide tools, or if checking for this edge case was simply forgotten in pest meta grammar.

I'd guess the latter? @dragostis @CAD97 @pest-parser/triage do you know?

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

No branches or pull requests

2 participants