celer / fire-ts

Fire TS is a templating library for NodeJS which specializes in generating non HTML templates (c, c++, Java, JavaScript, Ruby, Perl, etc)
MIT License
7 stars 1 forks source link

Whitespace forwarding weirdness #3

Open Rush opened 10 years ago

Rush commented 10 years ago

I am generating some type definitions in C++ and it seems that whitespace before <%parameter.type%> arg<%=i%> is not forwarded until some other character appears there. ;<%parameter.type%> arg<%=i%> would forward the whitespace but obviously I would like to change this behaviour. Is that a bug?

  <%func.parameters.forEach(function(parameter, i) {%>
    <%parameter.type%> arg<%=i%> = getArgument<<%=parameter.type%>>(args[<%=i%>]);
  <%}); %>
celer commented 10 years ago

Ok, so the answer to this question is yes, and no :P So your feedback would be nice to have here.

So I tried to be too smart, and make it so you could properly indent your templates, so you could read them. So what I tried to do is account for the fact that you'd indent your block, like you did and remove the extra spacing for the block. So for example, lets say I want a list of items:

<%items.forEach(function(i){%>
  anItem<%i%>
<%}%>

I look for an opening '{' bracket, and keep track of the fact that you may have actually tabbed out the template so it is more readable, and then try to remove the tabbing in the generated content, and remove the tab when generating the string.

I got frustrated having to choose between well formatted templates, or well formatted generated code - so I tried to achieve both.

When you use an actual tab before your whitespace, depending upon the depth of the surrounding template logic the associated number of tabs are removed from the generated output. (This only works with tabs)

Now that you've filed a bug I realize how non-intuitive it is :P What are your thoughts?

celer commented 10 years ago

So I to work around this try doubling the spacing

Rush commented 10 years ago

I think that what you did with detecting the opening { bracket and then removing one indentation level is spot on. That is also how I wish to write my code templates for maximum readability.

I did not know about the need to use tabs in firets`, it is kind of important to know to get good results then. (Un)fortunately some time ago I switched to using space-only indentation and I usually use 2 spaces. When I converted my template to tabs it seems to fix the case I mentioned in the issue report. Could you support other indentation schemes? Explicitly configurable could be sufficient as auto-detection is hard and not reliable.

Another important use case is indentation of nested templates:

namespace test {
  class %<=className %> {
    <% methods.forEach(method) {
      <%@ method.ts (method) %>
    <% }); %>
  }

I would like embedded method.ts to have proper indentation prepended. What's your take on this?

celer commented 10 years ago

This should be improved in 0.0.5, and you can now set the indent option in the options field. You can specify two spaces or a tab, etc and it will try to do the right thing. I've also updated the README to reflect this change.

Thanks!

celer commented 10 years ago

I just realized I haven't resolved the spacing of nested templates, I will work on that next.

Rush commented 10 years ago

Thanks a lot, will try the new changes after I get some sleep. :)

celer commented 10 years ago

So I was just about to write my test case for this (before I fixed it) and then realized my test case for nested templates does remove the extra indention. Does it work for you ?

Rush commented 10 years ago

I have tried the auto-indentation with tabs and it seems to work fine. (before today's changes) Now that other indentation styles are supported I think that nested templates indentation is the only indentation problem remaining. I will test your new code as soon as I can.

Rush commented 10 years ago

I wonder, if current indentation was available in scope it could be possible to do the following in pure js

<%= include('header', currentIndent, variableA, variableB) %>

or simply (if currentIndent was available globally)

<%= include('header', variableA, variableB) %>

A bit more verbose but more clean in my opinion. Now what could <%@ ... %> do is rewrite itself to javascript version of include. :)

Rush commented 10 years ago
fireTs.parseSync(filename, { indent: '  ' });

Working good!