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

Fix indentation of inline partials (issue #715) #716

Merged
merged 2 commits into from
Sep 13, 2019

Conversation

yotammadem
Copy link
Contributor

No description provided.

@phillipj
Copy link
Collaborator

phillipj commented Aug 28, 2019 via email

@phillipj
Copy link
Collaborator

phillipj commented Sep 1, 2019

Interesting solution indeed!

Thinking out loud, my first hope was that we could fix this in a very isolated part of the source code, instead of involving several functions and new kinds of data/tokens being created. E.g. only in the Writer.prototype.indentPartial() function would have been ideal, as that function sounds very much in charge for indentation of partials.. Might be easier said than done though.

Thanks a lot for your quick response. I'll dive in as well to see if we end up with the same solution or maybe a combination of both.

@yotammadem
Copy link
Contributor Author

I agree. I tired to find the quickest solution that satisfies our requirements. It emphasises the complexity of having too much coupling between the parse function and the indentPartials function like you said. I will try to figure out a better way, now that we have all tests in place, it should be easy to verify the solution.

@phillipj
Copy link
Collaborator

It emphasises the complexity of having too much coupling between the parse function and the indentPartials function

Well said!

I've also tried looking for different ways to fix it, but couldn't find anything else that introduced less coupling between the different functions & tokens passed.

Therefore leaning towards merging this as is and pushing a fix, as that would at least be valuable for end-users, and postponing improvements to reduce coupling for later as that's a more general concern.

@phillipj phillipj changed the title Fix issue #715 Fix indentation of inline partials (issue #715) Sep 13, 2019
@phillipj phillipj merged commit d820d25 into janl:master Sep 13, 2019
@phillipj
Copy link
Collaborator

Thanks a lot for finding a fix to this indentation bug! 👍

@phillipj
Copy link
Collaborator

Published in v3.1.0.

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.

2 participants