Fix unresolved data lookups on GitHub Glossary page (github#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 <[email protected]> Co-authored-by: Chiedo John <[email protected]>
witaliy12 · Dec 10, 2020 · e6518ff · e6518ff
1 parent 9725758
commit e6518ff
Show file tree

Hide file tree

Showing 3 changed files with 53 additions and 8 deletions.
diff --git 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
@@ -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
@@ -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)
+      }
+    )
+  })
 })