From e6518ff9c3d9fad8a92be1007909fa27d2c08153 Mon Sep 17 00:00:00 2001 From: "James M. Greene" Date: Wed, 9 Dec 2020 18:58:00 -0600 Subject: [PATCH] Fix unresolved data lookups on GitHub Glossary page (#16815) * Support [...] in data tag path * Use forloop.index0 to get data tag path * Commit whitespace change to not keep getting it * Recursively render glossary terms as well * Add tests to verify Liquid data tag can handle bracketed access * Remove superfluous 'set' for context in tests * Avoid the mystery guest anti-pattern... move the test data closer to the test * Escape the `-` character in the data regex Co-authored-by: Jason Etcovitch Co-authored-by: Chiedo John <2156688+chiedo@users.noreply.github.com> --- .../github-glossary.md | 4 +- lib/liquid-tags/data.js | 2 +- tests/unit/liquid-helpers.js | 55 +++++++++++++++++-- 3 files changed, 53 insertions(+), 8 deletions(-) diff --git a/content/github/getting-started-with-github/github-glossary.md b/content/github/getting-started-with-github/github-glossary.md index a949c7a0eb35..b3c87dcf68a0 100644 --- a/content/github/getting-started-with-github/github-glossary.md +++ b/content/github/getting-started-with-github/github-glossary.md @@ -10,8 +10,8 @@ versions: --- {% for term in site.data.glossaries.external %} - ### {{term.term}} - {{term.description}} + ### {% data glossaries.external[forloop.index0].term %} + {% data glossaries.external[forloop.index0].description %} --- {% endfor %} diff --git a/lib/liquid-tags/data.js b/lib/liquid-tags/data.js index 23abc89352ef..af500dc6e8fa 100644 --- a/lib/liquid-tags/data.js +++ b/lib/liquid-tags/data.js @@ -1,6 +1,6 @@ const Liquid = require('liquid') -const Syntax = /([a-z0-9/\\_.-]+)/i +const Syntax = /([a-z0-9/\\_.\-[\]]+)/i const SyntaxHelp = "Syntax Error in 'data' - Valid syntax: data [path]" module.exports = class Data extends Liquid.Tag { diff --git a/tests/unit/liquid-helpers.js b/tests/unit/liquid-helpers.js index 41c54a2e3d27..7a66b64a4836 100644 --- a/tests/unit/liquid-helpers.js +++ b/tests/unit/liquid-helpers.js @@ -1,7 +1,6 @@ const { liquid } = require('../../lib/render-content') const { loadPageMap } = require('../../lib/pages') const entities = new (require('html-entities').XmlEntities)() -const { set } = require('lodash') const nonEnterpriseDefaultVersion = require('../../lib/non-enterprise-default-version') describe('liquid helper tags', () => { @@ -15,11 +14,16 @@ describe('liquid helper tags', () => { context.currentVersion = nonEnterpriseDefaultVersion context.pages = pageMap context.redirects = [] - context.site = {} + context.site = { + data: { + reusables: { + example: 'a rose by any other name\nwould smell as sweet' + } + } + } context.page = { relativePath: 'desktop/index.md' } - set(context.site, 'data.reusables.example', 'a rose by any other name\nwould smell as sweet') done() }) @@ -83,8 +87,6 @@ describe('liquid helper tags', () => { }) describe('indented_data_reference tag', () => { - set(context.site, 'data.reusables.example', 'a rose by any other name\nwould smell as sweet') - test('without any number of spaces specified', async () => { const template = '{% indented_data_reference site.data.reusables.example %}' const expected = ` a rose by any other name @@ -117,4 +119,47 @@ would smell as sweet` expect(output).toBe(expected) }) }) + + describe('data tag', () => { + test( + 'handles bracketed array access within for-in loop', + async () => { + const template = ` +{% for term in site.data.glossaries.external %} +### {% data glossaries.external[forloop.index0].term %} +{% data glossaries.external[forloop.index0].description %} +--- +{% endfor %}` + + const localContext = { ...context } + localContext.site = { + data: { + variables: { + fire_emoji: ':fire:' + }, + glossaries: { + external: [ + { term: 'lit', description: 'Awesome things. {% data variables.fire_emoji %}' }, + { term: 'Zhu Li', description: '_"Zhu Li, do the thing!"_ :point_up:' } + ] + } + } + } + + const expected = ` + +### lit +Awesome things. :fire: +--- + +### Zhu Li +_"Zhu Li, do the thing!"_ :point_up: +--- +` + + const output = await liquid.parseAndRender(template, localContext) + expect(output).toBe(expected) + } + ) + }) })