1
0
mirror of synced 2026-01-29 03:03:52 -05:00

Merge pull request #19822 from github/support-short-versions

Support short versions
This commit is contained in:
Sarah Schneider
2021-06-14 20:32:57 -04:00
committed by GitHub
11 changed files with 444 additions and 23 deletions

View File

@@ -11,6 +11,7 @@ const plans = [
// See lib/remove-fpt-from-path.js for details.
plan: 'free-pro-team',
planTitle: 'GitHub.com',
shortName: 'fpt',
releases: [latestNonNumberedRelease],
latestRelease: latestNonNumberedRelease,
nonEnterpriseDefault: true, // permanent way to refer to this plan if the name changes
@@ -20,6 +21,7 @@ const plans = [
{
plan: 'enterprise-server',
planTitle: 'Enterprise Server',
shortName: 'ghes',
releases: enterpriseServerReleases.supported,
latestRelease: enterpriseServerReleases.latest,
hasNumberedReleases: true,
@@ -29,6 +31,7 @@ const plans = [
{
plan: 'github-ae',
planTitle: 'GitHub AE',
shortName: 'ghae',
releases: [latestNonNumberedRelease],
latestRelease: latestNonNumberedRelease,
openApiBaseName: 'github.ae',

View File

@@ -0,0 +1,6 @@
module.exports = [
'=',
'<',
'>',
'!='
]

View File

@@ -0,0 +1,151 @@
const { isTruthy, Expression, TokenizationError } = require('liquidjs')
const versionSatisfiesRange = require('../version-satisfies-range')
const supportedOperators = require('./ifversion-supported-operators')
const SyntaxHelp = "Syntax Error in 'ifversion' with range - Valid syntax: ifversion [operator] [releaseNumber]"
const supportedOperatorsRegex = new RegExp(`[${supportedOperators.join('')}]`)
const releaseRegex = /\d\d?\.\d\d?/
const notRegex = /(?:^|\s)not\s/
// This module supports a new tag we can use for docs versioning specifically. It extends the
// native Liquid `if` block tag. It has special handling for statements like {% ifversion ghes < 3.0 %},
// using semver to evaluate release numbers instead of doing standard number comparisons, which
// don't work the way we want because they evaluate 3.2 > 3.10 = true.
module.exports = {
// The following is verbatim from https://github.com/harttle/liquidjs/blob/v9.22.1/src/builtin/tags/if.ts
parse (tagToken, remainTokens) {
this.tagToken = tagToken
this.branches = []
this.elseTemplates = []
let p
const stream = this.liquid.parser.parseStream(remainTokens)
.on('start', () => this.branches.push({
cond: tagToken.args,
templates: (p = [])
}))
.on('tag:elsif', (token) => {
this.branches.push({
cond: token.args,
templates: p = []
})
})
.on('tag:else', () => (p = this.elseTemplates))
.on('tag:endif', () => stream.stop())
.on('template', (tpl) => p.push(tpl))
.on('end', () => {
throw new Error(`tag ${tagToken.getText()} not closed`)
})
stream.start()
},
// The following is _mostly_ verbatim from https://github.com/harttle/liquidjs/blob/v9.22.1/src/builtin/tags/if.ts
// The additions here are the handleNots() and handleOperators() calls.
render: function * (ctx, emitter) {
const r = this.liquid.renderer
const { operators, operatorsTrie } = this.liquid.options
this.currentRelease = ctx.environments.currentRelease
this.currentVersionShortName = ctx.environments.currentVersionShortName
for (const branch of this.branches) {
let resolvedBranchCond = branch.cond
// Resolve "not" keywords in the conditional, if any.
resolvedBranchCond = this.handleNots(resolvedBranchCond)
// Resolve special operators in the conditional, if any.
// This will replace syntax like `fpt or ghes < 3.0` with `fpt or true` or `fpt or false`.
resolvedBranchCond = this.handleOperators(resolvedBranchCond)
// Use Liquid's native function for the final evaluation.
const cond = yield new Expression(resolvedBranchCond, operators, operatorsTrie, ctx.opts.lenientIf).value(ctx)
if (isTruthy(cond, ctx)) {
yield r.renderTemplates(branch.templates, ctx, emitter)
return
}
}
yield r.renderTemplates(this.elseTemplates, ctx, emitter)
},
handleNots (resolvedBranchCond) {
if (!notRegex.test(resolvedBranchCond)) return resolvedBranchCond
const condArray = resolvedBranchCond.split(' ')
// Find the first index in the array that contains "not".
const notIndex = condArray.findIndex(el => el === 'not')
// E.g., ['not', 'fpt']
const condParts = condArray.slice(notIndex, notIndex + 2)
// E.g., 'fpt'
const versionToEvaluate = condParts[1]
// If the current version is the version being evaluated in the conditional,
// that is negated and resolved to false. If it's NOT the version being
// evaluated, that resolves to true.
const resolvedBoolean = !(versionToEvaluate === this.currentVersionShortName)
// Replace syntax like `not fpt` with `true` or `false`.
resolvedBranchCond = resolvedBranchCond.replace(condParts.join(' '), resolvedBoolean)
// Run this function recursively until we've resolved all the nots.
if (notRegex.test(resolvedBranchCond)) {
return this.handleNots(resolvedBranchCond)
}
return resolvedBranchCond
},
handleOperators (resolvedBranchCond) {
if (!supportedOperatorsRegex.test(resolvedBranchCond)) return resolvedBranchCond
// If this conditional contains multiple parts using `or` or `and`, get only the conditional with operators.
const condArray = resolvedBranchCond.split(' ')
// Find the first index in the array that contains an operator.
const operatorIndex = condArray.findIndex(el => supportedOperators.find(op => el === op))
// E.g., ['ghae', '<', '3.1']
const condParts = condArray.slice(operatorIndex - 1, operatorIndex + 2)
// Assign to vars.
const [versionShortName, operator, releaseToEvaluate] = condParts
// Handle syntax errors.
const error = !supportedOperators.includes(operator) || !releaseRegex.test(releaseToEvaluate)
if (error) {
throw new TokenizationError(SyntaxHelp, this.tagToken)
}
let resolvedBoolean
if (operator === '!=') {
// If this is the current version, compare the release numbers. (Our semver package doesn't handle !=.)
// If it's not the current version, it's always true.
resolvedBoolean = versionShortName === this.currentVersionShortName
? releaseToEvaluate !== this.currentRelease
: true
} else {
// If this is the current version, evaluate the operator using semver.
// If it's not the current version, it's always false.
resolvedBoolean = versionShortName === this.currentVersionShortName
? versionSatisfiesRange(this.currentRelease, `${operator}${releaseToEvaluate}`)
: false
}
// Replace syntax like `fpt or ghes < 3.0` with `fpt or true` or `fpt or false`.
resolvedBranchCond = resolvedBranchCond.replace(condParts.join(' '), resolvedBoolean)
// Run this function recursively until we've resolved all the special operators.
if (supportedOperatorsRegex.test(resolvedBranchCond)) {
return this.handleOperators(resolvedBranchCond)
}
return resolvedBranchCond
}
}

View File

@@ -12,6 +12,7 @@ renderContent.liquid.registerTag('indented_data_reference', require('../liquid-t
renderContent.liquid.registerTag('data', require('../liquid-tags/data'))
renderContent.liquid.registerTag('octicon', require('../liquid-tags/octicon'))
renderContent.liquid.registerTag('link_as_article_card', require('../liquid-tags/link-as-article-card'))
renderContent.liquid.registerTag('ifversion', require('../liquid-tags/ifversion'))
for (const tag in tags) {
// Register all the extended markdown tags, like {% note %} and {% warning %}

View File

@@ -1,8 +1,10 @@
const semver = require('semver')
// workaround for Enterprise Server 11.10.340 because we can't use semver to
// compare it to 2.x like we can with 2.0+
// Where "version" is an Enterprise Server release number, like `3.1`,
// and "range" is a semver range operator with another number, like `<=3.2`.
module.exports = function versionSatisfiesRange (version, range) {
// workaround for Enterprise Server 11.10.340 because we can't use semver to
// compare it to 2.x like we can with 2.0+
if (version === '11.10.340') return range.startsWith('<')
return semver.satisfies(semver.coerce(version), range)

View File

@@ -0,0 +1,23 @@
// This module creates shortcuts for version comparisons in Liquid conditional strings.
//
// Supported:
// {% if fpt %}
// {% if ghae %}
// {% if ghes %}
//
// For the custom operator handling in statements like {% if ghes > 3.0 %}, see `lib/liquid-tags/if-ver.js`.
module.exports = async function shortVersions (req, res, next) {
const { allVersions, currentVersion } = req.context
const currentVersionObj = allVersions[currentVersion]
if (!currentVersionObj) return next()
// Add the short name to context.
req.context[currentVersionObj.shortName] = true
// Add convenience props.
req.context.currentRelease = currentVersion.split('@')[1]
req.context.currentVersionShortName = currentVersionObj.shortName
return next()
}

View File

@@ -61,6 +61,7 @@ module.exports = function (app) {
app.use(require('./record-redirect'))
app.use(instrument('./detect-language')) // Must come before context, breadcrumbs, find-page, handle-errors, homepages
app.use(asyncMiddleware(instrument('./context'))) // Must come before early-access-*, handle-redirects
app.use(asyncMiddleware(instrument('./contextualizers/short-versions'))) // Support version shorthands
// *** Redirects, 3xx responses ***
// I ordered these by use frequency

View File

@@ -0,0 +1,17 @@
const renderContent = require('../../lib/render-content')
module.exports = function getIfversionConditionals (str) {
const conditionals = []
renderContent.liquid.parse(str)
.filter(block => block.name === 'ifversion')
// block.impl.branches is the only way to get an array of ifs and elsifs.
.map(block => block.impl.branches.map(branch => branch.cond))
.forEach(block => {
block.forEach(branch => {
conditionals.push(branch)
})
})
return conditionals
}

View File

@@ -6,6 +6,7 @@ const delimiter = '@'
const versionPattern = `${planPattern}${delimiter}${releasePattern}`
module.exports = {
additionalProperties: false,
properties: {
version: {
required: true,
@@ -40,6 +41,11 @@ module.exports = {
description: 'the plan title', // this is the same as the version title, sans numbered release
type: 'string'
},
shortName: {
required: true,
description: 'the short name for the version to be used in Liquid conditionals',
type: 'string'
},
releases: {
required: true,
description: 'an array of all supported releases for the version',

View File

@@ -16,8 +16,12 @@ const learningTracksSchema = require('../helpers/schemas/learning-tracks-schema'
const renderContent = require('../../lib/render-content')
const getApplicableVersions = require('../../lib/get-applicable-versions')
const { execSync } = require('child_process')
const allVersions = Object.keys(require('../../lib/all-versions'))
const enterpriseServerVersions = allVersions.filter(v => v.startsWith('enterprise-server@'))
const allVersions = require('../../lib/all-versions')
const { supported } = require('../../lib/enterprise-server-releases')
const getIfversionConditionals = require('../helpers/get-ifversion-conditionals')
const enterpriseServerVersions = Object.keys(allVersions).filter(v => v.startsWith('enterprise-server@'))
const versionShortNames = Object.values(allVersions).map(v => v.shortName)
const allowedVersionOperators = require('../../lib/liquid-tags/ifversion-supported-operators')
const rootDir = path.join(__dirname, '../..')
const contentDir = path.join(rootDir, 'content')
@@ -30,6 +34,8 @@ const learningTracks = path.join(rootDir, 'data/learning-tracks')
const languageCodes = Object.keys(languages)
const versionShortNameExceptions = ['ghae-next', 'ghae-issue-']
// WARNING: Complicated RegExp below!
//
// Things matched by this RegExp:
@@ -147,6 +153,11 @@ const oldOcticonRegex = /{{\s*?octicon-([a-z-]+)(\s[\w\s\d-]+)?\s*?}}/g
//
const oldExtendedMarkdownRegex = /{{\s*?[#/][a-z-]+\s*?}}/g
// Strings in Liquid will always evaluate true _because_ they are strings; instead use unquoted variables, like {% if foo %}.
// - {% if "foo" %}
// - {% unless "bar" %}
const stringInLiquidRegex = /{% (?:if|ifversion|elseif|unless) (?:"|').+?%}/g
const relativeArticleLinkErrorText = 'Found unexpected relative article links:'
const languageLinkErrorText = 'Found article links with hard-coded language codes:'
const versionLinkErrorText = 'Found article links with hard-coded version numbers:'
@@ -157,6 +168,7 @@ const badEarlyAccessImageErrorText = 'Found article images/links leaking incorre
const oldVariableErrorText = 'Found article uses old {{ site.data... }} syntax. Use {% data example.data.string %} instead!'
const oldOcticonErrorText = 'Found octicon variables with the old {{ octicon-name }} syntax. Use {% octicon "name" %} instead!'
const oldExtendedMarkdownErrorText = 'Found extended markdown tags with the old {{#note}} syntax. Use {% note %}/{% endnote %} instead!'
const stringInLiquidErrorText = 'Found Liquid conditionals that evaluate a string instead of a variable. Remove the quotes around the variable!'
const mdWalkOptions = {
globs: ['**/*.md'],
@@ -273,7 +285,8 @@ describe('lint markdown content', () => {
describe.each(mdToLint)(
'%s',
(markdownRelPath, markdownAbsPath) => {
let content, ast, links, yamlScheduledWorkflows, isHidden, isEarlyAccess, isSitePolicy, frontmatterErrors, frontmatterData
let content, ast, links, yamlScheduledWorkflows, isHidden, isEarlyAccess, isSitePolicy, frontmatterErrors, frontmatterData,
ifversionConditionals
beforeAll(async () => {
const fileContents = await readFileAsync(markdownAbsPath, 'utf8')
@@ -308,6 +321,9 @@ describe('lint markdown content', () => {
})))
.flat()
.map(schedule => schedule.cron)
ifversionConditionals = getIfversionConditionals(data)
.concat(getIfversionConditionals(bodyContent))
})
// We need to support some non-Early Access hidden docs in Site Policy
@@ -317,6 +333,11 @@ describe('lint markdown content', () => {
}
})
test('ifversion conditionals are valid in markdown', async () => {
const errors = validateIfversionConditionals(ifversionConditionals)
expect(errors.length, errors.join('\n')).toBe(0)
})
test('relative URLs must start with "/"', async () => {
const matches = links.filter(link => {
if (
@@ -408,6 +429,12 @@ describe('lint markdown content', () => {
})
})
test('does not contain Liquid that evaluates strings (because they are always true)', async () => {
const matches = (content.match(stringInLiquidRegex) || [])
const errorMessage = formatLinkError(stringInLiquidErrorText, matches)
expect(matches.length, errorMessage).toBe(0)
})
test('URLs must not contain a hard-coded language code', async () => {
const matches = links.filter(link => {
return /\/(?:${languageCodes.join('|')})\//.test(link)
@@ -486,15 +513,22 @@ describe('lint yaml content', () => {
describe.each(ymlToLint)(
'%s',
(yamlRelPath, yamlAbsPath) => {
let dictionary, isEarlyAccess
let dictionary, isEarlyAccess, ifversionConditionals
beforeAll(async () => {
const fileContents = await readFileAsync(yamlAbsPath, 'utf8')
ifversionConditionals = getIfversionConditionals(fileContents)
dictionary = yaml.load(fileContents, { filename: yamlRelPath })
isEarlyAccess = yamlRelPath.split('/').includes('early-access')
})
test('ifversion conditionals are valid in yaml', async () => {
const errors = validateIfversionConditionals(ifversionConditionals)
expect(errors.length, errors.join('\n')).toBe(0)
})
test('relative URLs must start with "/"', async () => {
const matches = []
@@ -667,6 +701,22 @@ describe('lint yaml content', () => {
const errorMessage = formatLinkError(oldExtendedMarkdownErrorText, matches)
expect(matches.length, errorMessage).toBe(0)
})
test('does not contain Liquid that evaluates strings (because they are always true)', async () => {
const matches = []
for (const [key, content] of Object.entries(dictionary)) {
const contentStr = getContent(content)
if (!contentStr) continue
const valMatches = (contentStr.match(stringInLiquidRegex) || [])
if (valMatches.length > 0) {
matches.push(...valMatches.map((match) => `Key "${key}": ${match}`))
}
}
const errorMessage = formatLinkError(stringInLiquidErrorText, matches)
expect(matches.length, errorMessage).toBe(0)
})
}
}
)
@@ -836,3 +886,64 @@ describe('lint learning tracks', () => {
}
)
})
function validateVersion (version) {
return versionShortNames.includes(version) ||
versionShortNameExceptions.some(exception => version.startsWith(exception))
}
function validateIfversionConditionals (conds) {
const errors = []
conds.forEach(cond => {
// This will get us an array of strings, where each string may have these space-separated parts:
// * Length 1: `<version>` (example: `fpt`)
// * Length 2: `not <version>` (example: `not ghae`)
// * Length 3: `<version> <operator> <release>` (example: `ghes > 3.0`)
const condParts = cond
.split(/ (or|and) /)
.filter(part => !(part === 'or' || part === 'and'))
condParts
.forEach(str => {
const strParts = str.split(' ')
// if length = 1, this should be a valid short version name.
if (strParts.length === 1) {
const version = strParts[0]
const isValidVersion = validateVersion(version)
if (!isValidVersion) {
errors.push(`"${version}" is not a valid short version name`)
}
}
// if length = 2, this should be 'not' followed by a valid short version name.
if (strParts.length === 2) {
const [notKeyword, version] = strParts
const isValidVersion = validateVersion(version)
const isValid = notKeyword === 'not' && isValidVersion
if (!isValid) {
errors.push(`"${cond}" is not a valid conditional`)
}
}
// if length = 3, this should be a range in the format: ghes > 3.0
// where the first item is `ghes` (currently the only version with numbered releases),
// the second item is a supported operator, and the third is a supported GHES release.
if (strParts.length === 3) {
const [version, operator, release] = strParts
const isGhes = version === 'ghes'
const isSupportedOperator = allowedVersionOperators.includes(operator)
const isSupportedRelease = supported.includes(release)
const isValid = isGhes && isSupportedOperator && isSupportedRelease
const errorMessage = str === cond
? `"${str}" is not a valid operation`
: `"${str}" is not a valid operation inside "${cond}"`
if (!isValid) {
errors.push(errorMessage)
}
}
})
})
return errors
}

View File

@@ -1,28 +1,42 @@
const { liquid } = require('../../lib/render-content')
const middleware = require('../../middleware/contextualizers/short-versions')
const allVersions = require('../../lib/all-versions')
const enterpriseServerReleases = require('../../lib/enterprise-server-releases')
const template = `
{% if page.version ver_gt "2.13" %}up to date{% endif %}
{% if page.version ver_lt "2.13" %}out of date{% endif %}
{% if currentVersion ver_gt "enterprise-server@2.13" %}up to date{% endif %}
{% if currentVersion ver_lt "enterprise-server@2.13" %}out of date{% endif %}
`
const shortVersionsTemplate = `
{% ifversion fpt %} I am FPT {% endif %}
{% ifversion ghae %} I am GHAE {% endif %}
{% ifversion ghes %} I am GHES {% endif %}
{% ifversion ghes = 3.1 %} I am GHES = 3.1 {% endif %}
{% ifversion ghes > 3.1 %} I am GHES > 3.1 {% endif %}
{% ifversion ghes < 3.1 %} I am GHES < 3.1 {% endif %}
{% ifversion fpt or ghes < 3.0 %} I am FTP or GHES < 3.0 {% endif %}
{% ifversion ghes < 3.1 and ghes > 2.22 %} I am 3.0 only {% endif %}
`
const negativeVersionsTemplate = `
{% ifversion not ghae %} I am not GHAE {% endif %}
{% ifversion not ghes %} I am not GHES {% endif %}
{% ifversion ghes != 3.1 %} I am not GHES 3.1 {% endif %}
`
describe('liquid template parser', () => {
describe('custom operators', () => {
describe('ver_gt', () => {
test('works as expected', async () => {
const context = {
page: {
version: '2.14'
}
}
const context = { currentVersion: 'enterprise-server@2.14' }
const output = await liquid.parseAndRender(template, context)
expect(output.trim()).toBe('up to date')
})
test('returns false when given value is not numeric, like `dotcom`', async () => {
const context = {
page: {
version: 'dotcom'
}
}
const context = { currentVersion: 'free-pro-team@latest' }
const output = await liquid.parseAndRender(template, context)
expect(output.trim()).toBe('')
})
@@ -36,14 +50,100 @@ describe('liquid template parser', () => {
describe('ver_lt', () => {
test('works as expected', async () => {
const context = {
page: {
version: '2.12'
}
}
const context = { currentVersion: 'enterprise-server@2.12' }
const output = await liquid.parseAndRender(template, context)
expect(output.trim()).toBe('out of date')
})
})
})
describe('short versions', () => {
// Create a fake req so we can test the shortVersions middleware
const req = { language: 'en', query: {} }
test('FPT works as expected when it is FPT', async () => {
req.context = {
currentVersion: 'free-pro-team@latest',
page: {},
allVersions,
enterpriseServerReleases
}
await middleware(req, null, () => {})
const output = await liquid.parseAndRender(shortVersionsTemplate, req.context)
// We should have TWO results because we are supporting two shortcuts
expect(output.replace(/\s\s+/g, ' ').trim()).toBe('I am FPT I am FTP or GHES < 3.0')
})
test('GHAE works as expected', async () => {
req.context = {
currentVersion: 'github-ae@latest',
page: {},
allVersions,
enterpriseServerReleases
}
await middleware(req, null, () => {})
const output = await liquid.parseAndRender(shortVersionsTemplate, req.context)
expect(output.trim()).toBe('I am GHAE')
})
test('GHES works as expected', async () => {
req.context = {
currentVersion: 'enterprise-server@2.22',
page: {},
allVersions,
enterpriseServerReleases
}
await middleware(req, null, () => {})
const output = await liquid.parseAndRender(shortVersionsTemplate, req.context)
expect(output.replace(/\s\s+/g, ' ').trim()).toBe('I am GHES I am GHES < 3.1 I am FTP or GHES < 3.0')
})
test('AND statements work as expected', async () => {
req.context = {
currentVersion: 'enterprise-server@3.0',
page: {},
allVersions,
enterpriseServerReleases
}
await middleware(req, null, () => {})
const output = await liquid.parseAndRender(shortVersionsTemplate, req.context)
expect(output.replace(/\s\s+/g, ' ').trim()).toBe('I am GHES I am GHES < 3.1 I am 3.0 only')
})
test('NOT statements work as expected on versions without numbered releases', async () => {
req.context = {
currentVersion: 'github-ae@latest',
page: {},
allVersions,
enterpriseServerReleases
}
await middleware(req, null, () => {})
const output = await liquid.parseAndRender(negativeVersionsTemplate, req.context)
expect(output.replace(/\s\s+/g, ' ').trim()).toBe('I am not GHES I am not GHES 3.1')
})
test('NOT statements work as expected on versions with numbered releases', async () => {
req.context = {
currentVersion: 'enterprise-server@3.0',
page: {},
allVersions,
enterpriseServerReleases
}
await middleware(req, null, () => {})
const output = await liquid.parseAndRender(negativeVersionsTemplate, req.context)
expect(output.replace(/\s\s+/g, ' ').trim()).toBe('I am not GHAE I am not GHES 3.1')
})
test('The != operator works as expected', async () => {
req.context = {
currentVersion: 'enterprise-server@3.1',
page: {},
allVersions,
enterpriseServerReleases
}
await middleware(req, null, () => {})
const output = await liquid.parseAndRender(negativeVersionsTemplate, req.context)
expect(output.replace(/\s\s+/g, ' ').trim()).toBe('I am not GHAE')
})
})
})