cedricziel commented 8 years ago

Request for comments: Add a syntax-element for controlling whitespace trimming

Currently, the compiler adds a lot of whitespace to the resulting templates, in a form that the amount of whitespace is roughly equivalent to the amount of space the syntax took right before the compile step.

Background

Having a more complex snippet of fluid code like this:

<f:if condition="expression">
<ul>
  <f:for each="{fruit} as apple">
    <li>{apple.color}</li>
  </f:for>
</ul>
</if>

will compile to something like this:

<!-- whitespace line here -->
<ul>
  <!-- whitespace line here -->
  <li>green</li>  <li>red</li>
  <!-- whitespace line here -->
</ul>
<!-- whitespace line here -->

Which is a lot of noise added through a simple control structure.

While machines are able to compress / collapse the whitespace and easily parse the relevant parts, as software developers that have to look a lot at compiled and evaluated templates it's vital to be able to mentally parse the resulting html as well. It would be desirable to have a syntax element that controls when and how whitespace is stripped and which allow for more fine-grained control of that stripping process than f:spaceless.

Example solution with Jinja

The Jinja 2 and Twig (as implementation of the Jinja spec in PHP) templating languages support a "strip whitespace operator" on the node level, implementing a similar feature should be desirable.

Links to relevant documentation:

Jinja 2
Twig

For example given a naive example:

<ul>
{% for apples in fruit %}
  <li>{{ apple.color }}</li>
{% endfor %}
</ul>

Would roughly compile down to:

<ul>
<!-- whitespace line here -->
  <li>green</li>  <li>green</li>
<!-- whitespace line here -->
</ul>

Now using the "strip whitespace operator" - the example would look like this:

<ul>
{%- for apples in fruit -%}
  <li>{{ apple.color }}</li>
{%- endfor -%}
</ul>

Would roughly compile down to:

<ul><li>green</li><li>green</li></ul>

Notice that the dash-operator - can be added to any expression, such as blocks or variable nodes ({{- value }}) and can be added to both sides of a node or only one (to control the whitespace prior to a node, but not the one produced by the node, for example).

Needed solutions

ViewHelperNodes

Well. To my ad-hoc knowledge, xhtml doesn't allow additional characters that apart from [a-z] as element names, so we may need a different, xhtml compliant "signal" to trigger the compiler to strip whitespace.

Expression nodes

For simple variable nodes, I propose we add a dash as stripping operator so that

<li>
  {- identifier -}
</li>

would compile to:

<li>value</li>

I would be glad to discuss this over time, if someone wants to share their thoughts, please go ahead. Maintainers: feel free to edit the text and form to match your liking.

a-r-m-i-n commented 8 years ago

What about a viewhelper for that, like this one? https://bitbucket.org/ArminVieweg/dce/src/72e9dcc3d2c1da0067ff47c844bdc0c02bcef598/Classes/ViewHelpers/Format/TinyViewHelper.php?at=develop&fileviewer=file-view-default

cedricziel commented 8 years ago

Thank you for the proposal, @ArminVieweg!

There's f:spaceless already that will perform a similar operation. The problems I see with solving this through a viewhelper are:

controlling whitespace is no extension-problem, in the sense that a language-construct is always preferrable to adding a layer of (again verbose) logic to the code to be written
controlling whitespace is a very recurring problem - i don't see me adding av:tiny around every template i write, since it collapses the whole content and again does a disservice to readability
a language construct through a distinct operator is less invasive when adding it, compared to surrounding a block of code with a viewhelper (think adding a dash - to a variable node {- foo.bar -} instead of surrounding it with a viewhelper, which would need to follow indentation etc.
an operator that could be added to both ends of nodes allows for really fine grained whitespace control instead of stripping it all away

georgringer commented 8 years ago

Couldn't we make that a bit smarter by just removing whitespaces which would be added just by the VH? I mislike to add - all over the place but would more prefer that

<ul>
  <f:for>
    <li>{item}

</li>
  </f:for>
</ul>

would be transferred to

<ul>
    <li>bla

</li>
    <li>blub

</li>
</ul>

You know what I mean?

a-r-m-i-n commented 8 years ago

I know. <f:for> would cause two line breaks, because of the line break behind <ul> and behind <f:for> I like this idea!

And namespace declarations should also act like viewhelpers then (and strip their newline in rendered template)

cedricziel commented 8 years ago

Jup, I know what you mean, thx for sparing a thought, @georgringer !

While I think your proposal is a step in the right direction, I still think if we attempt to tackle the problem, we should do it in a way that would allow to control the output with a little more detail. Stripping the space of a viewhelper goes half way and then stops-still leaving a ton of whitespace.

Maybe a combination? An operator would be entirely optional and would only be added where explicitly wanted by the template designer.

georgringer commented 8 years ago

Or maybe make a VH to stop the stripping and otherwise just strip it?

a-r-m-i-n commented 8 years ago

A raw viewhelper for whitespaces? ;-)

a-r-m-i-n commented 8 years ago

You could add an option to fluid which configures the default behaviour (always strip all whitespaces/just strip VH or namespace declaration whitespaces/keep all whitespaces).

And then you could introduce such a viewhelper which allows you to define the whitespace-behaviour for the wrapped section in your template.

helhum commented 8 years ago

Can you explain what the problem you want to solve? What is the problem with the whitespaces being present? Does it harm functionality, or is it just cosmetics?

a-r-m-i-n commented 8 years ago

Whitespaces in html can influence the rendering of items in browser, when they are not floating. This is the reason, why HMENU in TYPO3 CMS always strips whitespaces from navigations, afaik.

helhum commented 8 years ago

Any resource to look this up? I only quickly found: http://stackoverflow.com/questions/17925953/will-line-breaks-whitespace-in-html-affect-how-the-page-is-displayed

helhum commented 8 years ago

Probably this?: https://css-tricks.com/fighting-the-space-between-inline-block-elements/

Still I think that this can be tackled with CSS, can't it?

a-r-m-i-n commented 8 years ago

https://jsfiddle.net/uu6jg79b/ and https://jsfiddle.net/uu6jg79b/1/

Yes you can fix this with CSS by adding a float:left; https://jsfiddle.net/uu6jg79b/2/

But maybe you don't want the items to get floated.

helhum commented 8 years ago

https://jsfiddle.net/uu6jg79b/3/

or flexbox

But, ok. I understand.

a-r-m-i-n commented 8 years ago

@helhum Regarding your last update (3): margin-right: -4px; will not work, when you increase the font-size ;-) https://jsfiddle.net/uu6jg79b/5/

helhum commented 8 years ago

There are solutions to that. Here Flexbox: http://codepen.io/nordisk/pen/NRdaaO?editors=1100

In general I think it is wrong to adapt HTML (what is shown) and not CSS (how is it shown) to tackle layout issues, no matter how relevant they are.

I think the inline-block thing is the only case where this is relevant and introducing a whole new syntax in a to tackle space issues.

But as pointed out, other templating engines support that, so I'd better stay out of the discussion, as I may miss some important points here.

mneuhaus commented 8 years ago

i had similar issues with "roque whitespace" as well, so i'd be all in on a solution to ease that pain. i don't like {- identifier -} though, because it's a complete new type of syntax we'd introduce.

instead i could imagine something like a general viewhelper argument like "trim":

<ul>
  <f:for ... trim="1">
    <li>{item}</li>
  </f:for>
</ul>

to remove whitespace around the for loop children. but i'm not sure, if this could interfere with the escaping ( @helhum @NamelessCoder ?)

NamelessCoder commented 8 years ago

Just to clarify this:

The issue isn't that the ViewHelpers add whitespace when they are replaced with whatever content they render. The issue is that they don't remove the whitespace before or after themselves. Which may not seem like a big difference - but internally it makes a huge difference.

It would not be possible to do this on any other level than during parsing, I think. It would have to be done in a way that XYZ node detected with whitespace trimming (regardless of type) would imply that the Parser must then process whitespace of nodes coming before or after the currently processed node. And there is currently no such support whatsoever, so that would be quite a challenge.

But I will keep thinking about this, even though at the top of my head it does seem like we need/have to solve this using a ViewHelper like f:spaceless, or a global toggle for an entire template to be processed to remove all inter-tag whitespaces.

helhum commented 8 years ago

solve this using a ViewHelper like f:spaceless, or a global toggle for an entire template to be processed to remove all inter-tag whitespaces

I would agree with that. Should be relatively easy to do and would solve all related issues, without adding additional logic to the parser

tobiwollender commented 8 years ago

I'm removing ALL whitespace with EXT:tinysource from @ArminVieweg - works like a charm and is good for Google Pagespeed. So no problems with whitespace at all.

When you debug your webpage in the frontend you normally use e. g. the DevTools in Chrome and there you got your html well structured. Don't know why I would need f:spaceless or some other viewhelper for "nice" html output.

danielkestler commented 8 years ago

@tliegl I think that's the way to go. We've long used EXT:sourceopt that does essentially the same, but I don't know if it's still being updated.

BenjaminBeck commented 7 years ago

I have not tested EXT:tinysource or EXT:sourceopt. But parsing HTML correctly is really hard..

Can they deal with none closing li tags?
Can they deal with inline JS/CSS
Can they deal with inline Handlebars-Templates (HTML in a script Tag)
Do they respect the newlines in for example textareas or pre-tags?
Do they respect the CSS "white-space: pre " definition? (well, obviously not ..) ...

a-r-m-i-n commented 7 years ago

Inline CSS & JS is being ignored by tinysource (also not compressed).

Tinysource is obsolete btw. EXT:min is its successor.

Also EXT:min uses a library which compresses css and js (removing comments, whitespaces, etc.), when you add CSS/JS properly over page.includeCSS/JS, etc.

NamelessCoder commented 7 years ago

I may have an idea for this, but could use a logic check. We may be able to target the whitespace while parsing, and use plain trimming on the parsed result to remove the matched whitespace. We would need to target three different things during parsing:

Opening ViewHelperTags (excluding self-closing)
Closing ViewHelper tags
Self-closing ViewHelper tags

Each one needs a slightly different detection of whitespace:

Opening tag match optional any whitespace backwards to and including a line break, plus any optional whitespace (including line breaks) after tag until non-whitespace begins.
Closing tag match optional any whitespace backwards until and including a line break, plus line break immediately after tag closes.
Self-closing tag combines the above; whitespace backwards to and including a line break, plus line break immediately after tag.

I'll try to illustrate: in the below source example, mentally read | as a "matched line break that will be removed" and read - as "matched optional whitespace that will be removed".

<span>|
----<f:if condition="1">|
--------<f:then>Yes</f:else>|
--------<f:else>|
------------<f:format.date date="{date}" />|
--------</f:else>|
----</f:if>|
</span>

Which would result in:

<span>Yes</span>

Or:

<span>24-02-2017 15:00</span>

We could also make such a feature optional by a rather simple condition in the TemplateParser: if a toggle is enabled, we trim() on detected ViewHelper tags - if the toggle is disabled, we instead create TextNodes containing the whitespace before/after tag and attach those nodes before and after the ViewHelperNode.

It would cost a bit more during parsing if the feature is off (which it should be by default imho), but there would be no cost once the template is compiled, regardless of the toggle being enabled or disabled.

We could then implement this via a TemplatePreProcessor which for example detects {trim on} in the template to enable the toggle in TemplateParser - and/or we could make this possible to do globally with a configuration option. Opting in to the trimming from a controller would also be fairly easy, for example $this->view->getRenderingContext()->getTemplateParser()->enableTrimming(); which then would affect just that View instance.

Let me know if you see any potential problems with such a solution please. If it is an acceptable solution, it would possible to add as a non-breaking and completely opt-in feature!

liayn commented 6 years ago

TYPO3 specific bug report: https://forge.typo3.org/issues/86862

lolli42 commented 1 year ago

Hey, the entire whitespace thing seems to be an edge case and it might be a better idea to post-process html if really needed instead. Also, spaceless VH and similar things exist for special cases like plain text mails.

Since this issue did not gain much traction over the years and does not seem to be that pressing, I'll close here for now. Also, any changes in this area will have a rather huge impact on overall parsing and template compilation, so changing this within Fluid itself is a pretty huge effort for probably mostly edge uses.

In case this is still an urgent issue, I ask you to create a fresh issue referencing this one to re-discuss. Also, it would be good to look at the affected places along the way to get a better understanding on what would have to be changed in which way.

TYPO3 / Fluid

RFC: Fine grained whitespace control #154

Request for comments: Add a syntax-element for controlling whitespace trimming

Background

Example solution with Jinja

Needed solutions

ViewHelperNodes

Expression nodes