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

Fixed render crash when using inline include with a string template #651

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

wisskid
Copy link
Contributor

@wisskid wisskid commented Mar 22, 2021

Fixes #639

@wisskid
Copy link
Contributor Author

wisskid commented Mar 22, 2021

@AnrDaemon have a look please?

@AnrDaemon
Copy link
Contributor

Had one. There's at least two issues with this PR.

  1. It evaluates incorrectly (see modified unit test in Added regression test for smarty-php/smarty#639 #652 ).
  2. It does not cover the regression reported (see specific regression test in the same PR).

@wisskid
Copy link
Contributor Author

wisskid commented Mar 22, 2021

so... is the fuss about the newline or about the fatal error?

@AnrDaemon
Copy link
Contributor

Now it is about different rendering result before and after the fix.

There's no more crashes as far as I can see.

@wisskid
Copy link
Contributor Author

wisskid commented Mar 23, 2021

OK, but In my experience there was always a fatal error when rendering a string template inline.
I see the extra newline in the output, but wouldn't that be a separate, somewhat less important issue? Also: why would one render a string template as inline?

@AnrDaemon
Copy link
Contributor

string: source was used to easily demonstrate the issue using as few extra files as possible.
In real world, it was indeed an external file, but I made sure the results are exactly same.

@AnrDaemon
Copy link
Contributor

To clarify: I've never had crashes with string:… inline alone.

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

Successfully merging this pull request may close these issues.

Render crash on changing file include modes
2 participants