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

Don't output empty lines for control flow template arguments #9

Open
FSMaxB opened this issue Jan 21, 2016 · 7 comments
Open

Don't output empty lines for control flow template arguments #9

FSMaxB opened this issue Jan 21, 2016 · 7 comments
Assignees

Comments

@FSMaxB
Copy link

FSMaxB commented Jan 21, 2016

I noticed, that when I write something like this:

some text
#{for i = 1, 4 do}#
#{= i}#
#{end}#
some more text

The output is something like:

some text

1
2
3
4

some more text

If I don't want these two empty lines, I would have to write everything into one line like:

#{for i = 1, 4 do}##{= i}##{end}#

This is really ugly to read though. Would you be willing to accept a pull request that would check if a line only contains a control flow template parameter and, if so, doesn't emit those empty lines. (This could be hidden behind some kind of configuration parameter if necessary).

If this is a feature that you think will fit into the scope of slt2, I'd take a look if it is possible to implement this.

@henix
Copy link
Owner

henix commented Feb 2, 2016

Design

The design principle of slt2: keep the implementation simple. We should have a consistent and well-designed rule for dealing with whitespaces. I don't want to turn this project into a pile of special cases.

What others do

So I investigated some other template processors to see how they deal with whitespaces:

Handlebars introduced a grammar that user can control whitespace stripping in a fine-grained way: http://handlebarsjs.com/expressions.html#whitespace-control handlebars-lang/handlebars.js#336 . But I think this seems too complicated.

Jinja2's trim_blocks + lstrip_blocks: http://jinja.pocoo.org/docs/dev/templates/#whitespace-control

If an application configures Jinja to trim_blocks, the first newline after a template tag is removed automatically (like in PHP). The lstrip_blocks option can also be set to strip tabs and spaces from the beginning of a line to the start of a block. (Nothing will be stripped if there are other characters before the start of the block.)

And a shocking fact: PHP deletes newlines after closing tag: https://brian.moonspot.net/php-history-newline-closing-tag

My idea

I think the Jinja's trim_blocks + lstrip_blocks is better than your rule. Because for the following template:

some text #{for i = 1, 4 do}#
#{= i}#
#{end}#
some more text

Your code will output:

some text 
1

2

3

4
some more text

My idea is slightly different than Jinja's rule. The option name is trim_blocks, if set to true, slt2 will do a trim for code block and include block, but not expression block (#{= }#):

  1. delete the first newline after end tag
  2. delete whitespaces from the beginning of a line to start tag if there are no other characters

Implementation

I will implement this feature myself because I think the current code need some refactor and unit testing.

@henix
Copy link
Owner

henix commented Feb 2, 2016

Maybe I don't have enough time to refactor it soon (in this week). You are welcome to contribute if you like :).

@FSMaxB
Copy link
Author

FSMaxB commented Feb 5, 2016

Thanks for the feedback, I'll take a look at it.

@FSMaxB FSMaxB mentioned this issue Feb 5, 2016
@FSMaxB
Copy link
Author

FSMaxB commented Feb 5, 2016

My Ideas on implementing this feature (and refactor slt2 at the same time):

Instead of precompile and include_fold as well as some of the complexity in loadstring, there would be one single function parse that splits the template into a table of lines that consist of chunks. If a template expression is split over multiple lines, only one entry is created in the table. Includes will be done by the same parse function.
This parse function would split the template into the following types of chunks:

  • Blocks of whitespace at the beginning or end of a line (including the newline character)
  • Blocks of normal text
  • Template parameters

Example:

     bla    
  #{some template stuff}#  

#{
some multiline template argument
}#

This would create the following table:

{
  { "     ", "bla", "    " },
  { "  ", "#{some template stuff}#", "  " },
  { "    " },
  { "#{\nsome multiline template argument\n}#" }
}

trim_blocks would then simply be a matter of removing whitespace chunks in front of template chunks and after template chunks (if they are the only chunks in that line) and not putting a newline after it. trim_blocks would then be passed to slt2.render, not slt2.loadstring or slt2.loadfile.

One possible way to handle includes: Call parse with the file that was included and sort the resulting table back into the table that parse is currently working on.

@henix
Copy link
Owner

henix commented Feb 5, 2016

Your idea is very similar to mine: adding a lexing phase, just like what compilers do. And yes, include_fold and loadstring have a common structure that should be extracted into a function.

But for me, I prefer there would be only 2 types of chunks:

  1. Template (which delimited by start_tag and end_tag)
  2. Normal text

After parse, you will get a list of chunks. Then trim_blocks can be implemented by modifying normal text chunks before and after current template chunk.

I think you can just implement trim_blocks in the current direction of #10 (if you want to use it in your projects in a short time). It's almost finished. If you did, I would release a v1.1. Then I will take care of the refactor and release a v2.0.

My current plan is the API will be changed in v2.0. loadstring(tmpl, start_tag, end_tag, tmpl_name, trim_blocks) should be changed to loadstring(tmpl, tmpl_name, options) or something like that. options is a table, containing fields like start_tag, end_tag, trim_blocks. Because after adding an option, the function's signature is not so readable.

And #12 may also be considered in v2.0 (or v2.1).

@FSMaxB
Copy link
Author

FSMaxB commented Feb 5, 2016

After parse, you will get a list of chunks. Then trim_blocks can be implemented by modifying normal text chunks before and after current template chunk.

So no table of lines, only chunks. That's much easier to implement as I just noticed while trying to implement what I had proposed.

@henix
Copy link
Owner

henix commented Feb 5, 2016

So no table of lines, only chunks.

Yes, your example will be parsed as:

{
"     bla    \n  ",
"#{some template stuff}#",
"  \n    \n",
......
}

I think there's no need to break the template into lines. And the chunk's type can be determined by starts_with(chunk, start_tag).

@FSMaxB FSMaxB mentioned this issue Feb 5, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants