Skip to content

Commit e3034ac

Browse files
heiskrCopilot
andauthored
Trim excessive comments in the rest of src/content-render (#63278)
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 17588eeb-788a-4f82-9d36-b5079fb36521
1 parent 20e9ba9 commit e3034ac

21 files changed

Lines changed: 7 additions & 149 deletions

‎src/content-render/liquid/data.ts‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -91,7 +91,6 @@ function handleIndent(tagToken: TagToken, text: string): string {
9191
// keep the blockquote character on every successive line.
9292
const blockquoteRegexp = /^\n?([ \t]*>[ \t]?)/
9393
function handleBlockquote(tagToken: TagToken, text: string): string {
94-
// If the text isn't multiline, skip
9594
if (text.split('\n').length <= 1) return text
9695

9796
// If the line with the liquid tag starts with a blockquote...
@@ -100,7 +99,6 @@ function handleBlockquote(tagToken: TagToken, text: string): string {
10099
const inputLine = input.split('\n').find((line) => line.includes(content))
101100
if (!inputLine || !blockquoteRegexp.test(inputLine)) return text
102101

103-
// Keep the character on successive lines
104102
const match = inputLine.match(blockquoteRegexp)
105103
if (!match) return text
106104
const start = match[0]

‎src/content-render/liquid/engine.ts‎

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -77,9 +77,6 @@ engine.registerFilter('render_liquid', function (this: FilterScope, input: unkno
7777
return engine.parseAndRender(input, this.context.environments)
7878
})
7979

80-
/**
81-
* Convert the input to a slug
82-
*/
8380
engine.registerFilter('slugify', (input: string): string => {
8481
const slugger = new GithubSlugger()
8582
return slugger.slug(input)

‎src/content-render/liquid/ifversion.ts‎

Lines changed: 3 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -95,21 +95,19 @@ export default class Ifversion extends Tag {
9595
for (const branch of this.branches) {
9696
let resolvedBranchCond = branch.cond
9797

98-
// Resolve "not" keywords in the conditional, if any.
9998
resolvedBranchCond = this.handleNots(resolvedBranchCond)
10099

101100
// Resolve special operators in the conditional, if any.
102101
// This will replace syntax like `fpt or ghes < 3.0` with `fpt or true` or `fpt or false`.
103102
resolvedBranchCond = this.handleOperators(resolvedBranchCond)
104103

105-
// Resolve version names to boolean values for Markdown API context.
106-
// This will replace syntax like `fpt or ghec` with `true or false` based on current version.
107-
// Only apply this transformation in Markdown API context to avoid breaking existing functionality.
104+
// Replace syntax like `fpt or ghec` with `true or false` based on the current
105+
// version. Only done for the Markdown API, where the version names would
106+
// otherwise be undefined.
108107
if ((ctx.environments as IfversionEnvironments).markdownRequested) {
109108
resolvedBranchCond = this.handleVersionNames(resolvedBranchCond)
110109
}
111110

112-
// Use Liquid's native function for the final evaluation.
113111
const cond = yield new Value(resolvedBranchCond, this.liquid).value(ctx, ctx.opts.lenientIf)
114112

115113
if (isTruthy(cond, ctx)) {
@@ -125,7 +123,6 @@ export default class Ifversion extends Tag {
125123

126124
const condArray = resolvedBranchCond.split(' ')
127125

128-
// Find the first index in the array that contains "not".
129126
const notIndex = condArray.findIndex((el: string) => el === 'not')
130127

131128
// E.g., ['not', 'fpt']
@@ -156,15 +153,13 @@ export default class Ifversion extends Tag {
156153
// If this conditional contains multiple parts using `or` or `and`, get only the conditional with operators.
157154
const condArray = resolvedBranchCond.split(' ')
158155

159-
// Find the first index in the array that contains an operator.
160156
const operatorIndex = condArray.findIndex((el: string) =>
161157
supportedOperators.find((op: string) => el === op),
162158
)
163159

164160
// E.g., ['ghes', '<', '3.1']
165161
const condParts = condArray.slice(operatorIndex - 1, operatorIndex + 2)
166162

167-
// Assign to vars.
168163
const [versionShortName, operator, releaseToEvaluate] = condParts
169164

170165
// Make sure the operator is supported and the release number matches `\d\d?\.\d\d?`
@@ -219,17 +214,12 @@ export default class Ifversion extends Tag {
219214
return resolvedBranchCond
220215
}
221216

222-
// Split the condition into tokens for processing
223217
const tokens = resolvedBranchCond.split(/\s+/)
224218
const processedTokens = tokens.map((token: string) => {
225-
// Check if the token is a version short name (fpt, ghec, ghes, ghae)
226219
const versionShortNames = ['fpt', 'ghec', 'ghes', 'ghae']
227220
if (versionShortNames.includes(token)) {
228-
// Transform version names to boolean values for Markdown API
229-
// This fixes the original issue where version names were undefined in API context
230221
return token === this.currentVersionObj!.shortName ? 'true' : 'false'
231222
}
232-
// Return the token unchanged if it's not a version name
233223
return token
234224
})
235225

‎src/content-render/liquid/indented-data-reference.ts‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,6 @@ const IndentedDataReference = {
4747

4848
assert(parseInt(numSpaces) || numSpaces === '0', '"spaces=NUMBER" must include a number')
4949

50-
// Get the referenced value from the context
5150
const text: string | undefined = getDataByLanguage(
5251
dataReference,
5352
scope.environments.currentLanguage,
@@ -63,7 +62,6 @@ const IndentedDataReference = {
6362
return
6463
}
6564

66-
// add spaces to each line
6765
const renderedReferenceWithIndent: string = text.replace(/^/gm, ' '.repeat(parseInt(numSpaces)))
6866

6967
return this.liquid.parseAndRender(renderedReferenceWithIndent, scope.environments)

‎src/content-render/liquid/octicon.ts‎

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,6 @@ const Octicon = {
2525
throw new TokenizationError(SyntaxHelp, tagToken)
2626
}
2727

28-
// Memoize the icon
2928
this.icon = match.groups.icon
3029
// Breaking change in octicons 12
3130
// https://github.com/primer/octicons/releases/tag/v12.0.0
@@ -36,29 +35,24 @@ const Octicon = {
3635

3736
this.options = {}
3837

39-
// Memoize any options passed
4038
if (match.groups.options) {
4139
let optionsMatch: RegExpExecArray | null
4240

43-
// Loop through each option matching the OptionsSyntax regex
4441
while ((optionsMatch = OptionsSyntax.exec(match.groups.options))) {
4542
// Pull out the key/value ([0] is the whole input)
4643
const [, key, value] = optionsMatch
4744
this.options[key] = value
4845

49-
// Alias label to aria-label
5046
if (key === 'label') this.options['aria-label'] = value
5147
}
5248
}
5349
},
5450

5551
async render(): Promise<string> {
56-
// Throw an error if the requested octicon does not exist.
5752
if (!Object.prototype.hasOwnProperty.call(octicons, this.icon)) {
5853
throw new Error(`Octicon ${this.icon} does not exist`)
5954
}
6055

61-
// Auto-generate aria-label if not provided
6256
// Replace non-alphanumeric characters with spaces and append " icon"
6357
if (!this.options['aria-label']) {
6458
const defaultLabel = `${this.icon.toLowerCase().replace(/[^a-z0-9]+/gi, ' ')} icon`

‎src/content-render/liquid/prompt.ts‎

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,3 @@
1-
// src/content-render/liquid/prompt.ts
21
// Defines {% prompt %}…{% endprompt %} to wrap its content in <code> and append the Copilot icon.
32

43
import octicons from '@primer/octicons'
@@ -15,7 +14,6 @@ interface LiquidTag {
1514
export const Prompt: LiquidTag = {
1615
type: 'block',
1716

18-
// Collect everything until {% endprompt %}
1917
parse(tagToken: TagToken, remainTokens: TopLevelToken[]): void {
2018
this.templates = []
2119
const stream = this.liquid.parser.parseStream(remainTokens)
@@ -28,12 +26,10 @@ export const Prompt: LiquidTag = {
2826
stream.start()
2927
},
3028

31-
// Render the inner Markdown, wrap in <code>, then append the SVG
3229
*render(scope: unknown): Generator<unknown, string, unknown> {
3330
const content = yield this.liquid.renderer.renderTemplates(this.templates, scope)
3431
const contentString = String(content)
3532

36-
// build a URL with the prompt text encoded as query parameter
3733
const promptParam: string = encodeURIComponent(contentString)
3834
const href: string = `https://github.com/copilot?prompt=${promptParam}`
3935
// Use murmur hash for deterministic ID (avoids hydration mismatch)

‎src/content-render/tests/annotate.ts‎

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -22,41 +22,34 @@ describe('annotate', () => {
2222
const res = await renderContent(example)
2323
const $ = load(res)
2424

25-
// Check that the annotation structure is rendered correctly
2625
const annotation = $('.annotate')
2726
expect(annotation.length).toBe(1)
2827
expect(annotation.hasClass('beside')).toBe(true)
2928

30-
// Check annotation header exists
3129
const header = $('.annotate-header')
3230
expect(header.length).toBe(1)
3331

34-
// Check both beside and inline modes are rendered
3532
const beside = $('.annotate-beside')
3633
const inline = $('.annotate-inline')
3734
expect(beside.length).toBe(1)
3835
expect(inline.length).toBe(1)
3936

40-
// Check that we have the correct number of annotation rows
4137
const rows = $('.annotate-row')
4238
expect(rows.length).toBe(2)
4339

44-
// Check that each row has both code and note sections
4540
rows.each((i, row) => {
4641
const $row = $(row)
4742
expect($row.find('.annotate-code').length).toBe(1)
4843
expect($row.find('.annotate-note').length).toBe(1)
4944
})
5045

51-
// Check specific content of the annotations
5246
const notes = $('.annotate-note p')
5347
const noteTexts = notes.map((i, el) => $(el).text()).get()
5448
expect(noteTexts).toEqual([
5549
'The name of the workflow as it will appear in the "Actions" tab of the GitHub repository.',
5650
'Add the pull_request event, so that the workflow runs automatically\nevery time a pull request is created.',
5751
])
5852

59-
// Check code content
6053
const codes = $('.annotate-code pre')
6154
const codeTexts = codes.map((i, el) => $(el).text()).get()
6255
expect(codeTexts).toEqual([
@@ -157,7 +150,6 @@ on: [push]
157150
const rows = $('.annotate-row')
158151
const notes = $('.annotate-note', rows)
159152

160-
// Check that AUTOTITLE links were resolved to actual titles
161153
const firstNote = notes.eq(0).html()
162154
const secondNote = notes.eq(1).html()
163155

‎src/content-render/tests/copilot-code-blocks.ts‎

Lines changed: 0 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -18,11 +18,8 @@ describe('code-header plugin', () => {
1818

1919
const html = await renderContent(markdown)
2020

21-
// Should keep copilot as the language (not convert to text without copy meta)
2221
expect(html).toContain('language-copilot')
23-
// Should NOT wrap in code-example div since no copy meta
2422
expect(html).not.toContain('code-example')
25-
// Should NOT have header since no copy meta
2623
expect(html).not.toContain('<header')
2724
})
2825

@@ -31,13 +28,10 @@ describe('code-header plugin', () => {
3128

3229
const html = await renderContent(markdown)
3330

34-
// Should be wrapped in code-example div
3531
expect(html).toContain('code-example')
36-
// Should have header with copy button
3732
expect(html).toContain('<header')
3833
expect(html).toContain('js-btn-copy')
3934
expect(html).toContain('language-copilot')
40-
// Should NOT have prompt button (no prompt meta)
4135
expect(html).not.toContain('https://github.com/copilot?prompt=')
4236
})
4337

@@ -46,14 +40,10 @@ describe('code-header plugin', () => {
4640

4741
const html = await renderContent(markdown)
4842

49-
// Should be wrapped in code-example div
5043
expect(html).toContain('code-example')
51-
// Should have header
5244
expect(html).toContain('<header')
53-
// Should have prompt button
5445
expect(html).toContain('https://github.com/copilot?prompt=')
5546
expect(html).toContain('language-copilot')
56-
// Should NOT have copy button
5747
expect(html).not.toContain('js-btn-copy')
5848
})
5949

@@ -62,15 +52,11 @@ describe('code-header plugin', () => {
6252

6353
const html = await renderContent(markdown)
6454

65-
// Should be wrapped in code-example div
6655
expect(html).toContain('code-example')
67-
// Should have header with copy button
6856
expect(html).toContain('<header')
6957
expect(html).toContain('js-btn-copy')
70-
// Should have prompt button with encoded URL
7158
expect(html).toContain('https://github.com/copilot?prompt=')
7259
expect(html).toContain('Improve%20the%20variable%20names%20in%20this%20function')
73-
// Should have Copilot icon button
7460
expect(html).toContain('aria-label="Run this prompt in Copilot Chat"')
7561
expect(html).toContain('language-copilot')
7662
})
@@ -94,12 +80,9 @@ Improve the variable names in this function
9480

9581
const html = await renderContent(markdown)
9682

97-
// Should have prompt button with both code blocks in URL
9883
expect(html).toContain('https://github.com/copilot?prompt=')
99-
// Should contain encoded content from both the referenced code and the prompt
10084
expect(html).toContain('function%20logPersonsAge')
10185
expect(html).toContain('Improve%20the%20variable%20names')
102-
// Should have different aria-label indicating context
10386
expect(html).toContain('aria-label="Run this prompt with context in Copilot Chat"')
10487
})
10588

@@ -122,14 +105,10 @@ Improve the variable names in this function
122105

123106
const html = await renderContent(markdown)
124107

125-
// Should have prompt button with both code blocks in URL
126108
expect(html).toContain('https://github.com/copilot?prompt=')
127-
// Should contain encoded content from both the referenced code and the prompt
128109
expect(html).toContain('function%20logPersonsAge')
129110
expect(html).toContain('Improve%20the%20variable%20names')
130-
// Should have different aria-label indicating context
131111
expect(html).toContain('aria-label="Run this prompt with context in Copilot Chat"')
132-
// Should NOT have copy button
133112
expect(html).not.toContain('js-btn-copy')
134113
})
135114
})
@@ -143,19 +122,14 @@ Improve the variable names in this function
143122

144123
const html = await renderContent(markdown)
145124

146-
// Should warn about missing reference via structured logger
147125
expect(mockWarn).toHaveBeenCalledWith('Cannot find referenced code block', {
148126
ref: 'nonexistent-id',
149127
})
150128

151-
// Should still render with prompt button using current code only
152129
expect(html).toContain('https://github.com/copilot?prompt=')
153130
expect(html).toContain('Improve%20the%20variable%20names%20in%20this%20function')
154-
// Should NOT contain any referenced code since none was found
155131
expect(html).not.toContain('function%20logPersonsAge')
156-
// Should have standard aria-label (not context version)
157132
expect(html).toContain('aria-label="Run this prompt in Copilot Chat"')
158-
// Should not crash or fail
159133
expect(html).toContain('code-example')
160134

161135
mockWarn.mockClear()
@@ -169,7 +143,6 @@ function test() {}
169143

170144
const html = await renderContent(markdown)
171145

172-
// Should NOT wrap in code-example div (annotated blocks are excluded)
173146
expect(html).not.toContain('code-example')
174147
})
175148

@@ -178,7 +151,6 @@ function test() {}
178151

179152
const html = await renderContent(markdown)
180153

181-
// Should render with copy button
182154
expect(html).toContain('code-example')
183155
expect(html).toContain('js-btn-copy')
184156
expect(html).toContain('language-javascript')
@@ -191,7 +163,6 @@ function test() {}
191163

192164
const html = await renderContent(markdown)
193165

194-
// Should encode quotes and ampersands properly
195166
expect(html).toContain('%22quotes%22')
196167
expect(html).toContain('%26%20symbols')
197168
})
@@ -204,7 +175,6 @@ This is line 2
204175

205176
const html = await renderContent(markdown)
206177

207-
// Should encode newlines properly
208178
expect(html).toContain('This%20is%20line%201%0AThis%20is%20line%202')
209179
})
210180
})

0 commit comments

Comments
 (0)