Skip to content

Commit 9c536a8

Browse files
committed
Wrap optional parameters name in [] in hover tooltip
----- Behaviour diverges slightly from JSDoc here. JSDoc does not indicate that it's optional, rather it implicitly adds `| undefined` to the types
1 parent 299b434 commit 9c536a8

File tree

4 files changed

+33
-15
lines changed

4 files changed

+33
-15
lines changed

packages/theme-language-server-common/src/hover/providers/RenderSnippetHoverProvider.spec.ts

+9-7
Original file line numberDiff line numberDiff line change
@@ -15,11 +15,13 @@ describe('Module: RenderSnippetHoverProvider', async () => {
1515
name: 'title',
1616
description: 'The title of the product',
1717
type: 'string',
18+
required: true,
1819
},
1920
{
2021
name: 'border-radius',
2122
description: 'The border radius in px',
2223
type: 'number',
24+
required: false,
2325
},
2426
],
2527
},
@@ -47,13 +49,6 @@ describe('Module: RenderSnippetHoverProvider', async () => {
4749
});
4850

4951
describe('hover', () => {
50-
it('should return snippet definition with all parameters', async () => {
51-
await expect(provider).to.hover(
52-
`{% render 'product-car█d' %}`,
53-
'### product-card\n\n**Parameters:**\n- `title`: string - The title of the product\n- `border-radius`: number - The border radius in px',
54-
);
55-
});
56-
5752
it('should return null if no LiquidDocDefinition found', async () => {
5853
getSnippetDefinition = async () => ({ name: 'unknown-snippet', liquidDoc: undefined });
5954
provider = createProvider(getSnippetDefinition);
@@ -70,5 +65,12 @@ describe('Module: RenderSnippetHoverProvider', async () => {
7065
await expect(provider).to.hover(`{% assign asdf = 'snip█pet' %}`, null);
7166
await expect(provider).to.hover(`{{ 'snip█pet' }}`, null);
7267
});
68+
69+
it('should return snippet definition with all parameters', async () => {
70+
await expect(provider).to.hover(
71+
`{% render 'product-car█d' %}`,
72+
'### product-card\n\n**Parameters:**\n- `title`: string - The title of the product\n- `[border-radius]`: number - The border radius in px',
73+
);
74+
});
7375
});
7476
});

packages/theme-language-server-common/src/hover/providers/RenderSnippetHoverProvider.ts

+10-7
Original file line numberDiff line numberDiff line change
@@ -40,13 +40,7 @@ export class RenderSnippetHoverProvider implements BaseHoverProvider {
4040
const parts = [`### ${snippetDefinition.name}`];
4141

4242
if (liquidDoc.parameters?.length) {
43-
const parameters = liquidDoc.parameters
44-
?.map(
45-
({ name, type, description }: LiquidDocParameter) =>
46-
`- \`${name}\`${type ? `: ${type}` : ''} ${description ? `- ${description}` : ''}`,
47-
)
48-
.join('\n');
49-
43+
const parameters = this.buildParameters(liquidDoc.parameters);
5044
parts.push('', '**Parameters:**', parameters);
5145
}
5246

@@ -57,4 +51,13 @@ export class RenderSnippetHoverProvider implements BaseHoverProvider {
5751
},
5852
};
5953
}
54+
55+
private buildParameters(parameters: LiquidDocParameter[]) {
56+
return parameters.map(({ name, type, description, required }: LiquidDocParameter) => {
57+
const nameStr = required ? `\`${name}\`` : `\`[${name}]\``;
58+
const typeStr = type ? `: ${type}` : '';
59+
const descStr = description ? ` - ${description}` : '';
60+
return `- ${nameStr}${typeStr}${descStr}`;
61+
}).join('\n');
62+
}
6063
}

packages/theme-language-server-common/src/liquidDoc.spec.ts

+12-1
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ import { toSourceCode } from '@shopify/theme-check-common';
44
import { describe } from 'vitest';
55
import { getSnippetDefinition } from './liquidDoc';
66

7-
describe('Unit: makeGetLiquidDocDefinitions', () => {
7+
describe('Unit: getSnippetDefinition', () => {
88
function toAST(code: string) {
99
return toSourceCode('/tmp/foo.liquid', code).ast as LiquidHtmlNode;
1010
}
@@ -26,6 +26,7 @@ describe('Unit: makeGetLiquidDocDefinitions', () => {
2626
{% doc %}
2727
@param {String} firstParam - The first param
2828
@param {Number} secondParam - The second param
29+
@param {String} [optionalParam] - The optional param
2930
@param paramWithNoType - param with no type
3031
@param paramWithOnlyName
3132
{% enddoc %}
@@ -40,21 +41,31 @@ describe('Unit: makeGetLiquidDocDefinitions', () => {
4041
name: 'firstParam',
4142
description: 'The first param',
4243
type: 'String',
44+
required: true,
4345
},
4446
{
4547
name: 'secondParam',
4648
description: 'The second param',
4749
type: 'Number',
50+
required: true,
51+
},
52+
{
53+
name: 'optionalParam',
54+
description: 'The optional param',
55+
type: 'String',
56+
required: false,
4857
},
4958
{
5059
name: 'paramWithNoType',
5160
description: 'param with no type',
5261
type: null,
62+
required: true,
5363
},
5464
{
5565
name: 'paramWithOnlyName',
5666
description: '',
5767
type: null,
68+
required: true,
5869
},
5970
],
6071
},

packages/theme-language-server-common/src/liquidDoc.ts

+2
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ export type LiquidDocParameter = {
2222
name: string;
2323
description: string | null;
2424
type: string | null;
25+
required: boolean;
2526
};
2627

2728
export function getSnippetDefinition(
@@ -36,6 +37,7 @@ export function getSnippetDefinition(
3637
name: node.paramName.value,
3738
description: node.paramDescription?.value ?? null,
3839
type: node.paramType?.value ?? null,
40+
required: node.required,
3941
};
4042
},
4143
},

0 commit comments

Comments
 (0)