lazd / DOMly

The fast template system that creates and clones DOM nodes
MIT License
53 stars 9 forks source link

Allow escaping of {{ #7

Closed lazd closed 9 years ago

lazd commented 10 years ago

\{\{ should not be processed by the parser implemented in #2

cif commented 9 years ago

Escapes are rendering the escape character in addition to the brace.

\{\{data.attributes.name\}\}   // renders same string instead of {{data.attributes.name}}

Is there a quick solution to this or should I dive in? I'm currently using the nodejs precompile() method

lazd commented 9 years ago

Looks like the grammar will have to be modified to allow this, currently the CONTENT token is matching escaped curly braces and just passing them along. I suppose we need another token that handles escaped braces and returns them as content without the backslash.

cif commented 9 years ago

Makes sense. Dustjs uses {~rb} and {~lb}.

Where is that implemented, part of handlebars/mustache?

lazd commented 9 years ago

The Mustache-like grammar is implemented here: https://github.com/lazd/DOMly/blob/master/lib/grammar-text.jison

lazd commented 9 years ago

@cif, I added escaped mustaches to the grammar, I think we should be good now.

My BISON skills are rusty, so I hope I've done it correctly :)

cif commented 9 years ago

Tried quite a few things within that file, unfortunately I'm in the same boat with the BISON. The escape characters are still being rendered with the updated grammar rules

One thought I had was to enable the use of } and { (entities in general), however during the precompile function those are converted into their respective {} at some point

lazd commented 9 years ago

Hmmm, I added a test and it's working nicely... Can you create a gist with the template you're trying to compile?

I've filed a question issue #20 to track the entities behavior you noticed. I don't think it's a problem, though.

cif commented 9 years ago

Pared it down to be simple as possible.

https://gist.github.com/cif/564137bfba495aad1d4e

Is there anything else I would have to update other than the rules here? https://github.com/lazd/DOMly/blob/master/lib/grammar-text.jison I installed via NPM last last week.

lazd commented 9 years ago

@cif, I see the error you're mentioning. DOMly isn't processing that string as it doesn't use any variables, I can put together a fix shortly.

lazd commented 9 years ago

@cif, the latest change should fix it!

cif commented 9 years ago

Still having trouble unfortunately, pulled master to be sure I had the absolute latest.

\{\{nodata\}\}

Results in error "nodata" is undefined

\{\{Outer with data\}\}

Results in a compile time error:

[swell-domly] error compiling /Users/benipsen/Development/node/swell/client/dom/examples/router.dom: 
Error: Parse error on line 1:
Outer with data
------^
Expecting 'EOF', '(', ')', ',', '.', got 'VAR'
lazd commented 9 years ago

Did you get the changes to index.js as well as the grammar changes?

cif commented 9 years ago

Removed

if (!usesVariables(string)) {
    return safe(string);
  }

From compiler.makeVariableStatement(), was there anything else?

console.log(string); result just before error: making statement from: {{Outer with data}}

lazd commented 9 years ago

Nope, that should have been the only change... You can install DOMly from master by doing this:

npm install git+https://git@github.com/lazd/domly.git

That way, you don't have to worry about updating files in your installed version.

Try that, if you're still having trouble, please post a gist of the template and I'll investigate.

cif commented 9 years ago

Reinstalled with git and removed all but one template from my directory, still getting the same results:

https://gist.github.com/cif/c574523d71dbf30d9da3

Any reason you could see that the pre-compiler might be causing an issue? I included it in the result. I have tried with the markdown pre-processor disabled as well

lazd commented 9 years ago

Thanks for being thorough in your error reporting, it's really appreciated.

I checked out your gist, and I even tried compiling those templates you provided, and everything is working fine here with the latest from master. See my comment.

Are you certain you're picking up the changes? Try doing this:

rm -rf node_modules/domly
npm install git+https://git@github.com/lazd/domly.git
cif commented 9 years ago

My pleasure, if I were better able to fix I'd be stoked to contribute. I'm working with domly in the context of a backbone framework and trying to get two-way binding baked in, gotten the farthest with this lib so far.

100% sure I have the latest, removed via both rm and npm remove to be sure.

Is there somewhere I can take a look at your complied results and the compiler itself you're using? Seems to be the only variations in the simple tests we're doing

lazd commented 9 years ago

@cif, I created a quick CLI you can use to debug, DOMly-cli. Right now, to help you out, it uses the master version of DOMly. Obviously we'll switch it to a stable version before it's published :)

You can install it like so:

npm install -g git+https://git@github.com/lazd/domly-cli.git

Then you can use it like this:

domly someTemplate.html

You can pass -d to enable debug mode, try a -h to see all options.

That should help you see the compiled results.

cif commented 9 years ago

Awesome, thanks.

Sure enough the output from your CLI tool is different than within my node project processing the same exact input. Your tool is using createTextNode() as it should.

If it's not too much trouble, can you include the node_modules/domly folder as part of that repo so I can make sure it's exactly the same as mine? I'm completely at a loss as to what the difference is, I've tried re-installing again with no luck

lazd commented 9 years ago

Try this:

find . -type d -name domly

I imagine you're going to find multiple copies of DOMly, probably under one of your dependencies! You should only see one entry:

./node_modules/domly

Anyway, this problem will go away once I do a release, but if you can test your use case against master, that would make me a bit more confident in doing a release :)

cif commented 9 years ago
Bens-MacBook-Pro:swell benipsen$ find . -type d -name domly
./node_modules/domly

index.js looks as it should, grammar rules are there : /

The fact that the CLI tool you wrote works on my machine gives me confidence enough. Let me know when you npm publish

lazd commented 9 years ago

Can you push an example repo that includes exactly how you're using DOMly? I can help you troubleshoot further. I'm hesitant to publish this until I'm confident it's working for you.

cif commented 9 years ago
$ git clone git@github.com:cif/swell.git
$ cd swell
$ npm install
$ npm install git+https://git@github.com/lazd/domly.git
$ node node/swell.js --watch --server

I've moved all but the one testing template client/dom/examples/list.dom out of the target directory, the functions are compiled into public/tmpl.js

cif commented 9 years ago

Also worth adding, the compiler is located in compilers/domly.js

lazd commented 9 years ago

@cif, the problem lies in the output of converter.makeHtml(data) in compilers/domly.js:25:

var html = converter.makeHtml(data); // This is stripping the backslashes!
var template = dom.precompile(html, {
  stripWhitespace: true // Strip whitespace for better performance
});

This is what DOMly is actually compiling:

<p>testing template:</p>

<p>{{test}}</p>

I found this out by passing debug: true compiler option to DOMly. I got the following output:


Source file contents:
<p>testing template:</p>

<p>{{test}}</p>

Parsed tree:
   p 
     text testing template:
   text 

   p 
     text {{test}}
Parsed statement: { type: 'path', path: [ 'test' ] }

Compiled function:
console.log("Data:", data);
  var frag = document.createDocumentFragment();
  var data = data_0;
  var el0 = document.createElement("p");
  el0.textContent = "testing template:";
  frag.appendChild(el0);
  var el2 = document.createElement("p");
  el2.textContent = test;
  frag.appendChild(el2);
  return frag;

Result:
(function anonymous(data_0) {
console.log("Data:", data);
  var frag = document.createDocumentFragment();
  var data = data_0;
  var el0 = document.createElement("p");
  el0.textContent = "testing template:";
  frag.appendChild(el0);
  var el2 = document.createElement("p");
  el2.textContent = test;
  frag.appendChild(el2);
  return frag;
})

I double checked this by doing a converter.makeHtml(data) and saw the output I pasted above.

cif commented 9 years ago

Doh. Go figure! I had that bypassed (just passing data as arg) but then I stashed : /

debug:true is helpful for sure. I will see what I can do to avoid the loss of escape characters in the converter.

Thanks for the fix and all the help, sometimes just takes someone else to look!

lazd commented 9 years ago

Most welcome! Sometimes you can't see the problem when it's dancing on your nose, you need someone else to take a look :)