-
-
Notifications
You must be signed in to change notification settings - Fork 142
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
Fix indent v2 #654
Fix indent v2 #654
Conversation
bc67e05
to
16109f6
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have been super busy these days. Really need some time to deep dive into this. But the test cases look good. I think this approach is more promising.
src/template.rs
Outdated
@@ -143,10 +144,11 @@ pub struct HelperTemplate { | |||
pub inverse: Option<Template>, | |||
pub block: bool, | |||
pub chain: bool, | |||
pub indent_before_write: bool, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we can make this field private, as well as the one for DecoratorTemplate
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure! I need them in render_helper
, so I guess pub(crate)
it is.
I've been using this in practice for a bit more now and found two minor issues with it. Edit: Well ok, you always miss something I guess :). Similar fix for partials below. |
This looks good to me. Thank you for the fix! I have been struggling with those indent for quite a while. This approach seems practical and also keeps complexity at a acceptable level. |
I just tried the case reported in #611, and it seems we still this corner case to cover. |
I'm happy to look into that, though I'm quite busy this week so it might have to wait until the weekend. |
Thank you and no rush @cmrschwarz . Just take your time. |
This is my second attempt at fixing the more involved indenting issues first brought up in #650 .
It turns out that the
Indent
node is simply not powerful enough to handle all cases :(.We will unfortunately have to dynamically track whether a partial ended on a newline or not.
This PR does exactly that.
Here are some of the more difficult edge cases that this solves:
Input
Output
Input
Output
(yes, this is indeed correct according to handlebars js)
Input
Json Input:
{"a": "line\ndynamic_trailing_newline\n"}
Output