Clean up docs & tests for variables#2301
Conversation
tlylt
left a comment
tlylt
left a comment
There was a problem hiding this comment.
Added some comments for ease of reviewing
| {{ njcode('base1 | date(format2, 10)') }} :glyphicon-arrow-right: {{ base1 | date(format2, 10) }}<br/> | ||
|
|
||
| #### Page variables | ||
| Dates can be supplied using [page variables](../reusingContents.html#variables) for convenience. |
There was a problem hiding this comment.
Note: page variables no longer supported via <variable> syntax, see #2283
| <variable name="global_variable_overriding_included_variable">Global Variable Overriding Included Variable</variable> | ||
| <variable name="global_variable">Global Variable</variable> | ||
| <variable name="page_global_variable_overriding_page_variable">Global Variable Overriding Page Variable</variable> | ||
| <variable from="variable.json"></variable> |
There was a problem hiding this comment.
Note: removed as this way of importing does not work in variables.md
|
|
||
| **Test Page Variable and Included Variable Integrations** | ||
|
|
||
| {% set outerNunjucksVariable %} |
There was a problem hiding this comment.
Note: adding this because in testPageVariablesInInclude.md below we will test if this outer variable is leaking into the inner page.
| **Outer Page Variable Should Not Leak Into Inner Pages** | ||
|
|
||
| {{ nested_page_variable or "Outer Page Variable Should Not Leak Into Inner Pages" }} | ||
| {{ outerNunjucksVariable or "Outer Page Variable Should Not Leak Into Inner Pages" }} |
There was a problem hiding this comment.
Note: updating this because it appears to be outdated. we used to have nested_page_variable defined in index.md but that has been deleted for a long time. See yucheng11122017@2ddf00e
| @@ -0,0 +1,4 @@ | |||
| <variable name="example"> | |||
| To inject this HTML segment in your MarkBind files, use {{ example }} where you want to place it. | |||
There was a problem hiding this comment.
Note: for some reason there's no variables.md in this and the other site above, but we should include it to avoid unnecessary warning.
2 participants
What is the purpose of this pull request?
Overview of changes:
Fixes #2283
variables.mdfilevariables.mdfile to remove unnecessary warning about missing file during functional testsAnything you'd like to highlight/discuss:
Was expecting a need to update functional tests but doesn't seem to be required due to how
variables.mdand how nunjucks is processed before generating the HTML.Testing instructions:
markbind initdoes not contain warning of "variable has no name"npm run testno longer shows any warning about "no such file" for ".../variables.md" or the variable has no name warningProposed commit message: (wrap lines at 72 characters)
Clean up docs & tests for variables
Our docs on page variable is outdated due to feature removal. The
incorrect use of variable imports from json files within our variables.md
is problematic as it is no longer supported. A outdated test case still
exists for testing of page variable override. There are two warnings
of missing variables.md files in our functional test suite.
Keeping docs, test cases and implementation up-to-date is important.
Let's fix the docs, update the test cases, remove unnecessary warning
and ensure all variables.md do not contain the imports.
The above will fix the related issues around page variables and
variable imports.
Checklist: ☑️
Breaking change description:
A newly generated site (e.g. via
markbind init) will no longer contain the following erroneous line in_markbind/variables.md:This way of importing variables from an external file within the global variables.md is not supported.
For existing sites migrating to the latest version, please check that you do not have the above line in your
_markbind/variables.mdfile as it will not work as expected and will log a warning. Please remove such a line if found.