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

Newline slurp and indentation #89

Closed
pauldijou opened this issue May 27, 2015 · 6 comments · Fixed by #105
Closed

Newline slurp and indentation #89

pauldijou opened this issue May 27, 2015 · 6 comments · Fixed by #105

Comments

@pauldijou
Copy link

Lately, I've been trying to use the trim-mode closing tag -%> and I understand what it's doing but I wonder if the current behaviour does make sense or not really. So it's more of an open question than an issue. I was thinking, but I might be wrong, that the purpose of this tag was to write such template:

<ul>
<% users.forEach(function (user) { -%>
  <li><%= user %></li>
<% }) -%>
</ul>

And it would output something like:

<ul>
  <li>John</li>
  <li>Jane</li>
</ul>

And actually, it will! Nice. But let's take a code just a little bit more complex with more indentation.

<div>
  <ul>
  <% users.forEach(function (user) { -%>
    <li><%= user %></li>
  <% }) -%>
  </ul>
</div>

I would expect:

<div>
  <ul>
    <li>John</li>
    <li>Jane</li>
  </ul>
</div>

But I got:

<div>
  <ul>
        <li>John</li>
        <li>Jane</li>
      </ul>
</div>

That's because -%> only remove the following new line (just as mentioned in the doc), meaning it will concatenate both the indentation of the EJS tag and the indentation of the following line. Not sure this makes sense. So the deeper your EJS tag, the worse it will become.

Don't you think it would be better to remove the following new line and any following white spaces? This way, you could write:

<div>
  <ul>
    <% users.forEach(function (user) { -%>
    <li><%= user %></li>
    <% }) -%>
  </ul>
</div>

(yeah, it's not exactly the same as the previous one because the EJS tag would now needs to be at the same indentation as its content). This would output:

<div>
  <ul>
    <li>John</li>
    <li>Jane</li>
  </ul>
</div>

Another solution would be to have a syntax to indicate that an opening tag should remove all white spaces before him, meaning that the tag is actually only indented to be more readable but we don't actually want to output it's indentation. I would have suggest to use <%- but since it's already taken, I have no idea so I will use <%+ but that's just for the purpose of the sample. Pro: you can indent any EJS tag as you want. Con: you have to use both a custom opening and closing tags to remove everything you don't want.

<div>
  <ul>
  <%+ users.forEach(function (user) { -%>
    <li><%= user %></li>
  <%+ }) -%>
  </ul>
</div>

Would output:

<div>
  <ul>
    <li>John</li>
    <li>Jane</li>
  </ul>
</div>
@andidev
Copy link
Contributor

andidev commented Aug 6, 2015

This would be really great!

An alternative to introducing new syntax would be to pass an option that changes the behaviour.

Another suggestion for a syntax would be

<div>
  <ul>
  <% users.forEach(function (user) { --%>
    <li><%= user %></li>
  <% }) --%>
  </ul>
</div>

@andidev
Copy link
Contributor

andidev commented Aug 6, 2015

Are PR's accepted?

@andidev
Copy link
Contributor

andidev commented Aug 8, 2015

The proposed PR #105 introduces the following new syntax

<%_ removes any spaces or tabs before the ejs-tag
_%> removes any spaces or tabs and a newline after the ejs-tag

this means one can write

<div>
  <ul>
  <%_ users.forEach(function (user) { _%>
    <li><%= user %></li>
  <%_ }) _%>
  </ul>
</div>

and then get the requested output mentioned in this issue

<div>
  <ul>
    <li>John</li>
    <li>Jane</li>
  </ul>
</div>

@mde mde closed this as completed in #105 Aug 9, 2015
@mde
Copy link
Owner

mde commented Aug 9, 2015

Thanks for doing this. This is great.

@trevordmiller
Copy link

This is so helpful. Thank you, thank you!

@mde
Copy link
Owner

mde commented Sep 7, 2015

Apologies for the lengthy wait on this.

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 a pull request may close this issue.

4 participants