implement split filter #44

bacoboy · 2016-06-14T13:25:02Z

One of the tests is commented out pending fix for comma/space issue: #41

Otherwise, I think this works based on the other test case I included.

Should probably update #11 as well.

johannhof · 2016-06-14T23:03:16Z

Can you rebase and try running your test again? :)

bacoboy · 2016-06-15T02:32:16Z

👍 Looks good. Just waiting on automated tests.

Pls let me know if you are going to cut a release with all these recent fixes! Thx!

johannhof · 2016-06-15T12:38:01Z

src/filters.rs

+                    // The split string argument is also in fact a String
+                    let v: Vec<&str> = string_to_split.split(split_string).collect();
+                    // Split and construct result
+                    Ok(Array(v.iter().map(|x| Str(String::from(*x))).collect()))


you're calling .collect() and immediately afterwards .iter(). I think you can simply remove both these calls.

OK I think I got it figured out in 1 line... Watch for new commit.

johannhof · 2016-06-15T14:21:00Z

Looks good, but can you please install rustfmt and run cargo fmt over the project? You have some pretty long lines there, it would be great to split them up.

bacoboy · 2016-06-15T14:39:28Z

Still getting some that aren't mine, but repushed the branch with changes

% cargo fmt
Rustfmt failed at /Users/shoffman/workspace/liquid-rust/src/lib.rs:127: line exceeded maximum length (sorry)
Rustfmt failed at /Users/shoffman/workspace/liquid-rust/src/lib.rs:132: line exceeded maximum length (sorry)
Rustfmt failed at /Users/shoffman/workspace/liquid-rust/src/tags/for_block.rs:86: line exceeded maximum length (sorry)
Rustfmt failed at /Users/shoffman/workspace/liquid-rust/src/tags/for_block.rs:93: line exceeded maximum length (sorry)
Rustfmt failed at /Users/shoffman/workspace/liquid-rust/src/tags/for_block.rs:419: line exceeded maximum length (sorry)
Rustfmt failed at /Users/shoffman/workspace/liquid-rust/src/tags/for_block.rs:420: line exceeded maximum length (sorry)
Rustfmt failed at /Users/shoffman/workspace/liquid-rust/src/tags/for_block.rs:421: line exceeded maximum length (sorry)

johannhof · 2016-06-15T14:47:05Z

Great 👍

johannhof reviewed Jun 15, 2016
View reviewed changes

implement split filter

e780506

johannhof merged commit b8946a9 into cobalt-org:master Jun 15, 2016

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

implement split filter #44

implement split filter #44

bacoboy commented Jun 14, 2016

johannhof commented Jun 14, 2016

bacoboy commented Jun 15, 2016

johannhof Jun 15, 2016

bacoboy Jun 15, 2016

johannhof commented Jun 15, 2016

bacoboy commented Jun 15, 2016 •

edited

Loading

johannhof commented Jun 15, 2016

implement split filter #44

implement split filter #44

Conversation

bacoboy commented Jun 14, 2016

johannhof commented Jun 14, 2016

bacoboy commented Jun 15, 2016

johannhof Jun 15, 2016

Choose a reason for hiding this comment

bacoboy Jun 15, 2016

Choose a reason for hiding this comment

johannhof commented Jun 15, 2016

bacoboy commented Jun 15, 2016 • edited Loading

johannhof commented Jun 15, 2016

bacoboy commented Jun 15, 2016 •

edited

Loading