bake-escape feature request about grunt-bake HOT 20 CLOSED

I've just release 1.7.2 on NPM. Thanks @david-zacharias and @k-funk for testing.

@bosh Glad to hear that you find grunt-bake useful. I hope this fix helps you out!

from grunt-bake.

Hey and sorry for the late response.

I have not thought about escaping yet, maybe @MathiasPaumgarten. Recently we introduced the option removeUndefined. When set to false placeholders which could not be resolved (e.g. no matching variable found in content) stay untouchend in the output code. As far as I know, with current bake version you should not get undefined in the output, depending on removeUndefined these are removed or stay. Can you please provide some tests that describe your usecase?

from grunt-bake.

Included a test with what I'd like to see.
You'll notice when you run the test that

1. <Integer> isn't escaped (browser will think it's html)
2. The multi-line {{ \n ... \n }} is getting interpolated, and leaving behind undefined.

from grunt-bake.

Hey guys,
I see your point @k-funk. Maybe a first quick solution to work around the double braces would be setting the options.parsePattern to something else like /\{\%\s?([\.\-\w]*)\s?\%\}/g which would change the parser to look for variables like this:

<div>{% foo %}</div>

Obviously you can change the pattern to what ever you think looks pretty in code.
I do find it a temporary solution. I think escaping and not escaping could be very powerful. Jade allows you to do that like so:

div= foo //- escaped
div!= foo  //- not espaced

So I'm very open for discussion here. I think an inline flag like _escaped="true" could be fitting for this. We'd have to iron out how the recursion is treated, as you can repeatedly overwrite the value as you go down the parsing tree. But that can be figured out.

from grunt-bake.

This would probably have to deviate from the current implementation since the comment <!--(bake ...)--> would also be turned into html entities, basically making any usage of bake-escape/_escaped="true" be a leaf node.

from grunt-bake.

htmlencode may be useful for the encoding.

from grunt-bake.

Should we close this for now, as _process allows you to skip any encoding of any sort?

from grunt-bake.

_assign and _process met my needs.

from grunt-bake.

@david-zacharias Just tried implementing your solution with _assign and _process, which I've got nearly working. However, when I use _process="false", {{ code_here }} is still being processed into an empty string. You can reproduce this by changing /fixtures/includes/first.html to <div>{{code}}</div> and see that the {{code}} is gone in the inline_no_process test.

Is this expected behavior for _process?

from grunt-bake.

It really shouldn't be, right? @david-zacharias do you have time to look into it, or do you want me to?

from grunt-bake.

Oh sorry. @k-funk is right, I have missed that the processing of the placeholders takes place in the parse function in line 617 which is out of control of the replaceString function where inline attributes are evaluated. I have created a PR that fixes it and also added a test that contains placeholders on first level.

In general it is probably good to clean up and refactor some parts. e.g passing options from one method to another instead of using the global var. That would hopefully allow cleaner solutions than my current PR. But I guess none of us has the time for doing it…

from grunt-bake.

Yeah ... I've been wanting to largely refactor grunt-bake for a while, since it's grown so much since it's original creation. It's outgrown it's state of one file and global vars for a while now. But it's quite the endeavour to rewrite it. Thanks for fix though!! 👍

from grunt-bake.

@k-funk I just release his fix under 1.7.1. If you give it a shot it should finally take care of your issues.

from grunt-bake.

@david-zacharias Still doesn't work as expected. Only seems to work at a top level bake. Not for recursive bakes.

Try running the unit test for inline_no_process with this:
https://github.com/k-funk/grunt-bake/tree/inline-no-process-broken-recursive
You'll see that the result has no {{ code }} in it.

(the below file/code samples are the same as in my repo)
inline_no_process.html

<!DOCTYPE html>
<html>
    <head></head>
    <body>
        <!--(bake includes/first.html)-->
    </body>
</html>

first.html

<div><!--(bake second.html _process="false")--></div>

second.html

{{ code }}

from grunt-bake.

Bump! We're using many levels of bake to build our static documentation site, and the process directives tend to live in the second-to-deepest of 4-8 levels of nesting of files to bake together, so it needs to work for us recursively.

from grunt-bake.

Sorry for the delay, I have a quite heavy workload at the moment...

With my last commit I have included a test that simulates the disabling of processing in an inlcuded file. The commit also includes a fix which let all tests run green.

@k-funk and @bosh can you please add the current bake master to your projects, give it a try, and report. My commit it is kind of hotfix as I had not the time to inspect everything down to the last detail.

from grunt-bake.

@david-zacharias . _process is now working with with sub-includes, but _assign on the same line broke when 57e53fd was made. _assign on the same line does work in 1.7.1.

The code i've been trying to get work this whole time is something like (this is a sub-include):

<!--(bake curl-sample.sh _assign="code" _process="false" _if="curl")-->
<!--(bake c-sharp-sample.cs _assign="code" _process="false" _if="csharp")-->
<!--(bake java-sample.java _assign="code" _process="false" _if="java")-->
<!--(bake php-sample.php _assign="code" _process="false" _if="php")-->
<!--(bake python-sample.py _assign="code" _process="false" _if="python")-->
<!--(bake ruby-sample.rb _assign="code" _process="false" _if="ruby")-->
<pre><code>
{{ code | escape }}
</code></pre>

from grunt-bake.

Well, I have taken a deeper look into the processing and found that placeholders have been evaluated to early so they were resolved when the correct replacement was not ready. So the assign worked internally, but the placeholder was aready removed at the point where the script wanted to replace it...

from grunt-bake.

Looks good. Tested it out with my project and it's now working as expected.
@MathiasPaumgarten version bump to 1.7.2 ?

from grunt-bake.

👍

from grunt-bake.