Allow using {% (end)raw %} tags #1220
Conversation
| /* | ||
| Simple patch here - abort if the delimiters wrap around % % | ||
| */ | ||
| const isCharAfterStartPercent = str.charAt(start + 1) === '%'; |
There was a problem hiding this comment.
*the only functional changes in this file are these 5 lines
There was a problem hiding this comment.
Just to confirm, this won't break the other nunjucks templating syntax that use {% ...%} right?
There was a problem hiding this comment.
yup, if anything it should 'fix' it!
e.g.
{% for x in y %}
{{ x }}
{% endfor %}
this previously works (and still does) because nunjucks rendering is the first step and long precedes markdown rendering, so the {% for/endfor %} is 'erased' long before markdown-it-attrs touches it.
It dosen't work in the case of {% raw/endraw %} because it is still present at the time of markdown rendering though
So this should give more flexibility in future restructuring of the rendering order (if ever needed)
There was a problem hiding this comment.
That makes sense, thanks for clarifying!
There was a problem hiding this comment.
Mostly looks good to me 🙂 This is definitely a very welcome change that allows us to stop using dirty hacks in order to use variables in code blocks. Let's get the netlify build to work so that we can poke around at it to make sure that everything renders correctly.
.travis.yml
Outdated
| @@ -1,6 +1,6 @@ | |||
| language: node_js | |||
| node_js: | |||
| - '10' | |||
| - 'lts/*' | |||
There was a problem hiding this comment.
What happens if we leave this as 10?
I wonder if we should leave it as a fixed version, rather than automatically updating to the latest. That would allow us to choose specifically which versions of node we develop on and support.
| function preEscapeRawTags(pageData) { | ||
| return pageData.replace(REGEX, `${START_ESCAPE_STR}$&${END_ESCAPE_STR}`); | ||
| const tagMatches = pageData.matchAll(RAW_TAG_REGEX); |
There was a problem hiding this comment.
The netlify build failed because it doesn't recognise matchAll as a function. matchAll was added only in Node 12, but we support Node 10. Can we get around using matchAll?
There was a problem hiding this comment.
ahh I didn't notice the supported versions. In fact node v10 is still listed under 'maintainence' here.
Will update shortly with a more verbose but compatible solution
There was a problem hiding this comment.
updated with the solution suggested here https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/RegExp/exec, reverted the node version changes in travis.yml as well
| /* | ||
| Simple patch here - abort if the delimiters wrap around % % | ||
| */ | ||
| const isCharAfterStartPercent = str.charAt(start + 1) === '%'; |
There was a problem hiding this comment.
Just to confirm, this won't break the other nunjucks templating syntax that use {% ...%} right?
Nunjuck's {% raw %} tags don’t work as expected due to a soft syntax
conflict with markdown-it-attrs.
In addition, variable interpolation syntax cannot be output in
code elements due to vue's variable interpolation.
Let's patch markdown-it-attrs to allow the slight syntax conflicts,
and automatically assign v-pre to code elements to skip vue compilation.
|
Thanks for looking through this! @marvinchin |
|
@marvinchin @ang-zeyu we might have to do a release because the previous release version is no longer compatible with our own docs after this PR was merged. i.e., I can't build docs in the master branch using the previous released version of MarkBind. |
@damithc I think that it is reasonable for the previous release to be incompatible with the docs in master branch. Any new feature that is introduced to master that involves any syntax changes to the docs would not be compatible with the previous release. It should be sufficient that the docs on master is compatible with the markbind build on master. That being said, it has been some time since we have done a release. Perhaps we can do one this week 🙂 @ang-zeyu shall we find some time to do the next release together? |
I agree @marvinchin , it's the normal case. Just that I want to do a doc PR and I don't have the dev setup. |
|
But not very urgent. I can PR based on the last commit that is backward compatible. Release at your convenience. |
Might want to hold off a little bit, let's discuss #1236 first which potentially breaks a lot of things
Sure! If its fine we could merge in #1189 and #1208 before doing so as well |
|
I'm thinking of getting MarkBind/vue-strap#147 merged as well for now, so the release process is a tiny bit easier. Not too sure if the repo merging process I mentioned there is the right/best way to do it though (aim being to preserve both repo's histories) |
### *netlify node_version needs to be updated to the current ltsWhat is the purpose of this pull request? (put "X" next to an item, remove the rest)
• [ ] Documentation update
• [x] Bug fix
• [ ] New feature
• [ ] Enhancement to an existing feature
• [ ] Other, please explain:
Resolves #762
Resolves #1206
What is the rationale for this request?
Allow usage of {% raw/endraw %} tags
What changes did you make? (Give an overview)
{% raw/endraw %}to be usedv-preto code fences and inline code and<code>elements for convenience{% raw/endraw %}is still necessary to display{{ content }}- hence variables can still be used inside code fences / etc.<div/span/table/etc>), one would need to addv-preattribute on top of{% raw/endraw %}to disable vue compilation for that elementIs there anything you'd like reviewers to focus on?
na
Testing instructions:
Proposed commit message: (wrap lines at 72 characters)
Allow using {% (end)raw %} tags
Nunjuck's {% raw %} tags don’t work as expected due to a soft syntax
conflict with markdown-it-attrs.
In addition, variable interpolation syntax cannot be output in
code elements due to vue's variable interpolation.
Let's patch markdown-it-attrs to allow the slight syntax conflicts,
and automatically assign v-pre to code elements to skip vue compilation.