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

Interpolations are parsed incorrectly #876

Closed
ghost opened this issue Feb 4, 2015 · 5 comments
Closed

Interpolations are parsed incorrectly #876

ghost opened this issue Feb 4, 2015 · 5 comments
Assignees
Milestone

Comments

@ghost
Copy link

ghost commented Feb 4, 2015

I started working on Issue #615 recently and even submitted a pull request (#854) for it.

During the process, I realized that the root problem was something different; something definitely not addressed by my pull request.

I believe the root problem is that we are not parsing interpolations correctly.

Throughout the code, there are quite a few very similar blocks like the following:

p = find_first_in_interval< exactly<hash_lbrace> >(i, end_of_selector);
    if (p) {
    // accumulate the preceding segment if there is one
    if (i < p) (*schema) << new (ctx.mem) String_Constant(pstate, Token(i, p, Position(0, 0)));
    // find the end of the interpolant and parse it
    const char* j = find_first_in_interval< exactly<rbrace> >(p, end_of_selector);

If I read the code correctly, the block is looking for the beginning of an interpolation, then it’s trying to seek to the end of it (seeking the closing right-brace).

But there could be many kinds of expressions between the two braces, including nested interpolations (my pull request was trying to fix that), quoted strings (with or without escaped characters), etc., etc.

The right way, I think, is that we should recursively parse whatever in-between the braces - like going back to the main Parser::parse() loop.

The question is: in our parser, do we have some mechanism that we can push any expressions onto a stack of some sort, then parse them recursively?

Please advise. Thanks!

@xzyfer
Copy link
Contributor

xzyfer commented Feb 4, 2015

I believe @mgreter has start heavily refactoring interpolation parsing and quote normalisation. Pieces of this work have already shipped in #847.

@ghost
Copy link
Author

ghost commented Feb 4, 2015

Awesome. I will cancel my pull request and wait for the right fix.

@ghost
Copy link
Author

ghost commented Feb 4, 2015

@mgreter are you thinking of parsing interpolations recursively? without recursion, we might end up chasing lots of edge cases and having boilerplate code...

@mgreter
Copy link
Contributor

mgreter commented Feb 4, 2015

Since interpolants can be nested, this is kind of the key point. I already merged some improvements in the prelexer and currently working on extracting more stuff from my WIP branch, which includes a lot more of these improvements, but there are also a lot of places where those need to be applied correctly.

@mgreter
Copy link
Contributor

mgreter commented Mar 2, 2015

This should be solved by #910!

@mgreter mgreter closed this as completed Mar 2, 2015
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