liquid.cr icon indicating copy to clipboard operation
liquid.cr copied to clipboard

Implement Liquid::Drop.

Open hugopl opened this issue 3 years ago • 1 comments

~This still a draft.~

All public methods with no arguments from a Drop is exported. I implemented a small stack machine to be able to process the expressions given to the Context, but it still missing some things:

  • ~The ! and - prefixes aren't implemented yet~
  • I'm not 100% sure of the .blank? and .missing? semantics.
  • I need to review the code too, since I believe it can be even simpler.

~Besides two tests about ! and - prefix, these are the tests that still failing.~

  1) Liquid Liquid::Context returns nil for missing key in strict mode with ?
     Failure/Error: ctx.get("missing?").should be_nil

       Expected: false to be nil

     # spec/liquid_spec.cr:55

  2) Liquid::Template should support array access via literal
     Failure/Error: tpl.render(ctx).should eq "second, first, third, third"

       Expected: "second, first, third, third"
            got: "second, first, , third"

     # spec/template_spec.cr:238

  3) Liquid::Template should support #blank?
     Failure/Error: tpl.render(ctx).should eq "blank"

       Expected: "blank"
            got: ""

     # spec/template_spec.cr:290

  4) Liquid::Template should support combinations of array/hash access and property access
     Failure/Error: tpl.render(ctx).should eq "3 2 second-b val val val"

       Expected: "3 2 second-b val val val"
            got: "3 2 second-b  val val"

     # spec/template_spec.cr:332

Most, if not all, are about how the built in methods .size, '.blank, present` must behave.

Any early feedback are appreciated.

Fix #36

hugopl avatar Jul 28 '22 21:07 hugopl

Now there's just 2 tests failing:

  1) Liquid Liquid::Context returns nil for missing key in strict mode with ?
     Failure/Error: ctx.get("missing?").should be_nil

       Expected: nil to be nil

     # spec/liquid_spec.cr:55

  2) Liquid::Template should support #blank?
     Failure/Error: tpl.render(ctx).should eq "blank"

       Expected: "blank"
            got: ""

     # spec/template_spec.cr:290

To pass on other tests, the current code is evaluating the variables not found to nil when they have a ? suffix instead or raise an error. IMO they should raise an error or evaluate to nil depending on strict mode.

Second tests tests if {% if var.blank? %}blank{% endif %} renders blank when there's no variable set in the context

According to https://jumpseller.com/support/liquid-sandbox/ both tests are about invalid expressions that uses variables that not exist in the context and must raise an error on strict mode or evaluate to nil in non-strict mode.

hugopl avatar Jul 29 '22 21:07 hugopl

oops, removed the branch by mistake.

BTW I noticed that these .blank? and .present? are liquid.cr extensions to liquid language, they do not exist in any liquid implementation I saw so far.

I have no idea about how many people are using this project and how it would deal with breaking changes... probably not much.

So, I'll update the PR to match behavior of https://shopify.github.io/liquid/, since my needs are for ruby compatibility at first, then the community decides what to do with the code.

hugopl avatar Dec 07 '22 20:12 hugopl

Closing this in favor of https://github.com/amberframework/liquid.cr/pull/69

hugopl avatar Dec 16 '22 20:12 hugopl