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

Bug: Array Indexes #127

Closed
booyaa opened this issue Jul 9, 2017 · 4 comments · Fixed by #141
Closed

Bug: Array Indexes #127

booyaa opened this issue Jul 9, 2017 · 4 comments · Fixed by #141
Labels
bug Not as expected

Comments

@booyaa
Copy link

booyaa commented Jul 9, 2017

liquid-rust version: 0.10.0
rust version: rustc 1.18.0 (03fc9d622 2017-06-06)
OS: macOS Sierra / 10.12.4

{% assign tags = "foo,bar" | split: "," %}
tags[0]: {{ tags[0] }}

running the code results in

thread 'main' panicked at 'called `Result::unwrap()` on an `Err` value: Parser("Expected |, found [")', sr
c/libcore/result.rs:859
note: Run with `RUST_BACKTRACE=1` for a backtrace.
{% for tag in tags %}
   {{ tag }}
{% endfor %}

Will build and display each tag.

I've tested using another liquid parser and this definitely a problem with liquid-rust.

@Albibek
Copy link
Contributor

Albibek commented Oct 18, 2017

I've started investigating this issue. Not done yet, but working on it.

Here's what I've found atm:

I think I can fix this eventually.

@epage
Copy link
Member

epage commented Oct 18, 2017

That'd be a big help!

I've not been able to set aside time to try to step through the regex based parser. I'd actually love to switch it to nom or some other parser though it does lose the comparison to shopify's but as you pointed it, it can be misleading.

@Albibek
Copy link
Contributor

Albibek commented Oct 23, 2017

I think I could look into it in future as well, but after fixing this bug.
I'd (personally) prefer using https://github.com/Marwes/combine instead of nom.

@epage
Copy link
Member

epage commented Oct 23, 2017

I've created #138 to track changing the parser.

I've not explored the parser options, so I'll be deferring to whoever does the work. It would be helpful if you posted to #138 what your recommendation is and why so that if someone comes along and goes "you did X, why not Y" we have a reason.

@epage epage closed this as completed in #141 Nov 7, 2017
epage pushed a commit that referenced this issue Nov 7, 2017
This fixes #127 adding feature to access identifiers via index
- array indexes `arr[0]` are supported
- object indexes `obj["child"]` are supported and interchangeable with `obj.child`
- index expressions are not yet supported.
epage added a commit that referenced this issue Nov 8, 2017
* **syntax:** Add `arr[0]` and `obj["name"]` indexing (PR #141, fixes #127)
* **value:**  Add nil value to support foreign data (PR #140)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Not as expected
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants