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

Unexpected newline removal #447

Closed
tjkirch opened this issue Jun 18, 2021 · 6 comments · Fixed by #448
Closed

Unexpected newline removal #447

tjkirch opened this issue Jun 18, 2021 · 6 comments · Fixed by #448
Labels

Comments

@tjkirch
Copy link

tjkirch commented Jun 18, 2021

After #404, it seems lines are being removed a bit too aggressively. If I have a template like this:

{{#if a}}x{{/if}}
y

I would expect it to render like this, if the data is {"a": true}:

x
y

This is what happens in handlebars.js, which I tested on tryhandlebarsjs.com.

In handlebars-rust 4, this renders as:

xy
@sunng87
Copy link
Owner

sunng87 commented Jun 18, 2021

This seems like a bug of empty stripping feature. Let me look into it.

@sunng87
Copy link
Owner

sunng87 commented Jun 28, 2021

@tjkirch could you please help me to verify the fix in #448 has your use case covered?

@tjkirch
Copy link
Author

tjkirch commented Jun 29, 2021

@sunng87 I can confirm it fixes the issues I had in unit tests. I'm having a hard time testing it in a larger context because of the licensing changes of dependencies in v4, but I could raise separate issues if I find anything else after that's worked through.

@sunng87
Copy link
Owner

sunng87 commented Jun 30, 2021

@tjkirch could you be more specific on the licensing changes of dependencies? Perhaps we can find workaround for that

@tjkirch
Copy link
Author

tjkirch commented Jul 2, 2021

@tjkirch could you be more specific on the licensing changes of dependencies? Perhaps we can find workaround for that

I think I've narrowed it down:

  • The new rhai dependency depends on smartstring, which has an MPL-2.0+ license.
  • The new pprof dev-dependency:
    • depends on inferno, which has a CDDL-1.0 license.
      • inferno uses flamegraph, which seems to be a mix of CDDL-1.0 and GPLv2+.
    • depends on symbolic-demangle (MIT license) which vendors a subset of swift (slightly modified Apache-2.0 license, for most things) which includes some LLVM components (as far as I can tell, only ones under BSD-3-Clause).

None of this is bad, but for projects that need to track and approve licenses, it adds a fair amount of overhead to the update to v4.

@sunng87
Copy link
Owner

sunng87 commented Jul 3, 2021

I see. Thank you for getting back to me.

These issues could happen on 3.x with regular cargo update, since semver may not guarantee license compatibility.

I will release 4.1.0 soon to include this fix. Let me know if I could help.

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

Successfully merging a pull request may close this issue.

2 participants