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

Add template trim of white space to dust.compile #166

Merged
merged 4 commits into from
Oct 30, 2012
Merged

Add template trim of white space to dust.compile #166

merged 4 commits into from
Oct 30, 2012

Conversation

smfoote
Copy link
Contributor

@smfoote smfoote commented Oct 4, 2012

This change allows dust templates to be compiled in a minified way. Specifically, all leading whitespace is removed, then all newlines are removed. This can be beneficial because Dust currently writes all whitespace, which can result in HTML like this:

My link

This change allows dust templates to be compiled in a minified way. Specifically, all leading whitespace is removed, then all newlines are removed. This can be beneficial because Dust currently writes all whitespace, which can result in HTML like this:

<a href="                 index.html            ">My link</a>
@vybs
Copy link
Contributor

vybs commented Oct 4, 2012

@jairodemorais added as minifier with uglify ( would be good to integrate together )

@smfoote
Copy link
Contributor Author

smfoote commented Oct 4, 2012

@vybs does uglify minify our Dust templates? If not, what is uglify minifying?

@vybs
Copy link
Contributor

vybs commented Oct 5, 2012

@jairodemorais knows more. I forget.

@jairodemorais
Copy link
Contributor

@vybs we are talking about two different things. We are using uglify to minify the dust code, and @smfoote wants to minify the compiled templates.

Maybe we could use uglify to minify the templates too. I think that is a better alternative than just replace some things.

@smfoote
Copy link
Contributor Author

smfoote commented Oct 9, 2012

I think uglify might be overkill for minifying templates on compile, when
all we want to do is remove leading whitespace and new lines.
On Oct 9, 2012 8:28 AM, "Jairo de Morais" notifications@github.com wrote:

@vybs https://github.com/vybs we are talking about two different
things. We are using uglify to minify the dust code, and @smfootehttps://github.com/smfootewants to minify the compiled templates.

Maybe we could use uglify to minify the templates too. I think that is a
better alternative than just replace some things.


Reply to this email directly or view it on GitHubhttps://github.com//pull/166#issuecomment-9265466.

@vybs
Copy link
Contributor

vybs commented Oct 9, 2012

it is not minify perse. the term is pretty misleading

@jairodemorais
Copy link
Contributor

@smfoote
Copy link
Contributor Author

smfoote commented Oct 9, 2012

I agree. It's not minify in the same way that Uglify and Google Closure does minification. But, I still think it is useful. Any suggestions on better names? strip? trim? removeExcessWhitespace?

@jleppert
Copy link
Contributor

jleppert commented Oct 9, 2012

Are we concerned about it removing new lines in templates that are intentional?

@smfoote
Copy link
Contributor Author

smfoote commented Oct 9, 2012

New lines in templates that are intentional should use {~n}, correct?

@vybs
Copy link
Contributor

vybs commented Oct 9, 2012

Can always ensure that is the case. Just be careful we dont break anything in use already

@jleppert
Copy link
Contributor

jleppert commented Oct 9, 2012

It seems okay, since this is triggered with an option anyway. We can easily turn it off if it causes a problem.

@vybs
Copy link
Contributor

vybs commented Oct 9, 2012

i would rather prefer, strip/trim ( newlines/spaces/tabs)

add docs into what this does exactly.

Update testcases and renderTestSpec.js with new name.
@rragan
Copy link
Contributor

rragan commented Oct 10, 2012

While we are talking about whitespace, is there any good way for someone to do the equivalent of writing a multi-line

 block where the alignment and whitespace are preserved? I don't know of one. Granted it is not a common need but it does come up.

@vybs
Copy link
Contributor

vybs commented Oct 10, 2012

code example @rragan

What I want to generate is:

  myvar=1;
  if (a === b) {
    a = b+1;
  }

Adding <br/> will force one line per output line (like pre) but we still lack the indentation.
myvar=1;<br/>
if (a === b) <br/>
a = b+1;<br/>
}<br/>

Changing the leading spaces to &nbsp; would likely suffice to get the whitespace but the whole thing is very unpleasant for common genre's of documentation (e.g. writing about code).

@smfoote
Copy link
Contributor Author

smfoote commented Oct 10, 2012

@rragan Preserving whitespace (but only in some places) definitely complicates this issue. Are you looking for what is offered by the <pre> tag, but in Dust?

@rragan
Copy link
Contributor

rragan commented Oct 10, 2012

I don't need the

 capability often unless I'm writing web pages about code (heaven forbid it's about Python where whitespace is signficant). When I need it, writing the page in dust will be a major pain to write and/or update.

@smfoote
Copy link
Contributor Author

smfoote commented Oct 10, 2012

Is @rragan 's issues blocking this from being merged?

@rragan
Copy link
Contributor

rragan commented Oct 10, 2012

I don't see that my comment has any impact on this proposed change. Any solution for a pre-like ability will have to be a very special case and likely not to interact with this change.

I don't know that there are "backward compatibility" issues with this change but there could be. Browsers generally gobble up extra whitespace but still can do things that affect visual impact when tags have newline/extra blank floating around between them. What, if any, weight should this have?

@smfoote
Copy link
Contributor Author

smfoote commented Oct 12, 2012

There shouldn't be any backwards compatibility issues because you have to opt in to make strip do anything.

jimmyhchan added a commit that referenced this pull request Oct 30, 2012
Add template strip of white space to dust.compile (optional defaults to not strip)
@jimmyhchan jimmyhchan merged commit 930a1ce into linkedin:master Oct 30, 2012
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.

6 participants