Skip to content

Commit 53d0a34

Browse files
authored
Fix hover markdown escaping so links are usable (#1159)
* fix hover markdown escaping so links are usable Signed-off-by: Morgan Chang <[email protected]> * fix tests Signed-off-by: Morgan Chang <[email protected]> --------- Signed-off-by: Morgan Chang <[email protected]>
1 parent 23a2e61 commit 53d0a34

File tree

3 files changed

+94
-9
lines changed

3 files changed

+94
-9
lines changed

src/languageservice/services/yamlHover.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -211,7 +211,7 @@ export class YAMLHover {
211211
private toMarkdown(plain: string | undefined): string | undefined {
212212
if (plain) {
213213
let escaped = plain.replace(/([^\n\r])(\r?\n)([^\n\r])/gm, '$1\n\n$3'); // single new lines to \n\n (Markdown paragraph)
214-
escaped = escaped.replace(/[\\`*_{}[\]()#+\-.!]/g, '\\$&'); // escape markdown syntax tokens: http://daringfireball.net/projects/markdown/syntax#backslash
214+
escaped = escaped.replace(/[\\`*_{}[\]#+\-!]/g, '\\$&'); // escape some of the markdown syntax tokens http://daringfireball.net/projects/markdown/syntax#backslash to avoid unintended formatting
215215
if (this.indentation !== undefined) {
216216
// escape indentation whitespace to prevent it from being converted to markdown code blocks.
217217
const indentationMatchRegex = new RegExp(` {${this.indentation.length}}`, 'g');

test/hover.test.ts

Lines changed: 92 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -86,7 +86,7 @@ describe('Hover Tests', () => {
8686
assert.strictEqual((hover.contents as MarkupContent).kind, 'markdown');
8787
assert.strictEqual(
8888
(hover.contents as MarkupContent).value,
89-
`The directory from which bower should run\\. All relative paths will be calculated according to this setting\\.\n\nSource: [${SCHEMA_ID}](file:///${SCHEMA_ID})`
89+
`The directory from which bower should run. All relative paths will be calculated according to this setting.\n\nSource: [${SCHEMA_ID}](file:///${SCHEMA_ID})`
9090
);
9191
});
9292

@@ -108,7 +108,7 @@ describe('Hover Tests', () => {
108108
assert.strictEqual((result.contents as MarkupContent).kind, 'markdown');
109109
assert.strictEqual(
110110
(result.contents as MarkupContent).value,
111-
`The directory from which bower should run\\. All relative paths will be calculated according to this setting\\.\n\nSource: [${SCHEMA_ID}](file:///${SCHEMA_ID})`
111+
`The directory from which bower should run. All relative paths will be calculated according to this setting.\n\nSource: [${SCHEMA_ID}](file:///${SCHEMA_ID})`
112112
);
113113
});
114114

@@ -303,7 +303,7 @@ describe('Hover Tests', () => {
303303
assert.strictEqual(MarkupContent.is(result.contents), true);
304304
assert.strictEqual(
305305
(result.contents as MarkupContent).value,
306-
`Full name of the author\\.\n\nSource: [${SCHEMA_ID}](file:///${SCHEMA_ID})`
306+
`Full name of the author.\n\nSource: [${SCHEMA_ID}](file:///${SCHEMA_ID})`
307307
);
308308
});
309309

@@ -335,7 +335,7 @@ describe('Hover Tests', () => {
335335
assert.strictEqual(MarkupContent.is(result.contents), true);
336336
assert.strictEqual(
337337
(result.contents as MarkupContent).value,
338-
`Email address of the author\\.\n\nSource: [${SCHEMA_ID}](file:///${SCHEMA_ID})`
338+
`Email address of the author.\n\nSource: [${SCHEMA_ID}](file:///${SCHEMA_ID})`
339339
);
340340
});
341341

@@ -425,7 +425,7 @@ storage:
425425
assert.strictEqual(MarkupContent.is(result.contents), true);
426426
assert.strictEqual(
427427
(result.contents as MarkupContent).value,
428-
`#### no\\_proxy \\(list of strings\\):\n\nSource: [${SCHEMA_ID}](file:///${SCHEMA_ID})`
428+
`#### no\\_proxy (list of strings):\n\nSource: [${SCHEMA_ID}](file:///${SCHEMA_ID})`
429429
);
430430

431431
const content2 = `ignition:
@@ -444,7 +444,7 @@ storage:
444444
assert.strictEqual(MarkupContent.is(result.contents), true);
445445
assert.strictEqual(
446446
(result.contents as MarkupContent).value,
447-
`#### devices \\(list of strings\\):\n\nSource: [${SCHEMA_ID}](file:///${SCHEMA_ID})`
447+
`#### devices (list of strings):\n\nSource: [${SCHEMA_ID}](file:///${SCHEMA_ID})`
448448
);
449449
});
450450

@@ -552,7 +552,7 @@ users:
552552
assert.strictEqual(MarkupContent.is(result.contents), true);
553553
assert.strictEqual(
554554
(result.contents as MarkupContent).value,
555-
`#### Person\n\nAt the top level my\\_var is shown properly\\.\n\n&emsp;&emsp;Issue with my\\_var2\n\n&emsp;&emsp;&emsp;here my\\_var3\n\nSource: [${SCHEMA_ID}](file:///${SCHEMA_ID})`
555+
`#### Person\n\nAt the top level my\\_var is shown properly.\n\n&emsp;&emsp;Issue with my\\_var2\n\n&emsp;&emsp;&emsp;here my\\_var3\n\nSource: [${SCHEMA_ID}](file:///${SCHEMA_ID})`
556556
);
557557
});
558558

@@ -860,6 +860,91 @@ Source: [${SCHEMA_ID}](file:///${SCHEMA_ID})`
860860

861861
expect(result).to.be.null;
862862
});
863+
it('Hover preserves literal parentheses', async () => {
864+
schemaProvider.addSchema(SCHEMA_ID, {
865+
type: 'object',
866+
properties: {
867+
parentheses: {
868+
type: 'string',
869+
description: 'Parentheses should be literal: (abc)(1).',
870+
},
871+
},
872+
});
873+
874+
const content = 'parenth|e|ses: x';
875+
const hover = await parseSetup(content);
876+
877+
assert.strictEqual(MarkupContent.is(hover.contents), true);
878+
assert.strictEqual((hover.contents as MarkupContent).kind, 'markdown');
879+
assert.strictEqual(
880+
(hover.contents as MarkupContent).value,
881+
`Parentheses should be literal: (abc)(1).\n\nSource: [${SCHEMA_ID}](file:///${SCHEMA_ID})`
882+
);
883+
});
884+
it('Hover preserves literal dots', async () => {
885+
schemaProvider.addSchema(SCHEMA_ID, {
886+
type: 'object',
887+
properties: {
888+
dotInText: {
889+
type: 'string',
890+
description: 'Dots should be literal in normal text: v1.2.3, 3.14159, example.com.',
891+
},
892+
},
893+
});
894+
895+
const content = 'dot|I|nText: x';
896+
const hover = await parseSetup(content);
897+
898+
assert.strictEqual(MarkupContent.is(hover.contents), true);
899+
assert.strictEqual((hover.contents as MarkupContent).kind, 'markdown');
900+
assert.strictEqual(
901+
(hover.contents as MarkupContent).value,
902+
`Dots should be literal in normal text: v1.2.3, 3.14159, example.com.\n\nSource: [${SCHEMA_ID}](file:///${SCHEMA_ID})`
903+
);
904+
});
905+
it('Hover preserves bare URL in description (no escaping)', async () => {
906+
schemaProvider.addSchema(SCHEMA_ID, {
907+
type: 'object',
908+
properties: {
909+
repo: {
910+
type: 'string',
911+
description: 'The YAML Language Server at https://github.com/redhat-developer/yaml-language-server',
912+
},
913+
},
914+
});
915+
916+
const content = 'rep|o|: x';
917+
const hover = await parseSetup(content);
918+
919+
assert.strictEqual(MarkupContent.is(hover.contents), true);
920+
assert.strictEqual((hover.contents as MarkupContent).kind, 'markdown');
921+
assert.strictEqual(
922+
(hover.contents as MarkupContent).value,
923+
`The YAML Language Server at https://github.com/redhat\\-developer/yaml\\-language\\-server\n\nSource: [${SCHEMA_ID}](file:///${SCHEMA_ID})`
924+
);
925+
});
926+
927+
it('Hover preserves URL wrapped in parentheses in description (no escaping)', async () => {
928+
schemaProvider.addSchema(SCHEMA_ID, {
929+
type: 'object',
930+
properties: {
931+
repo: {
932+
type: 'string',
933+
description: 'Yaml Language Server (https://github.com/redhat-developer/yaml-language-server)',
934+
},
935+
},
936+
});
937+
938+
const content = 'rep|o|: x';
939+
const hover = await parseSetup(content);
940+
941+
assert.strictEqual(MarkupContent.is(hover.contents), true);
942+
assert.strictEqual((hover.contents as MarkupContent).kind, 'markdown');
943+
assert.strictEqual(
944+
(hover.contents as MarkupContent).value,
945+
`Yaml Language Server (https://github.com/redhat\\-developer/yaml\\-language\\-server)\n\nSource: [${SCHEMA_ID}](file:///${SCHEMA_ID})`
946+
);
947+
});
863948
});
864949

865950
describe('Hover on anyOf', () => {

test/yamlLanguageService.test.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -79,7 +79,7 @@ describe('getLanguageService()', () => {
7979
assert.deepEqual(result, {
8080
contents: {
8181
kind: 'markdown',
82-
value: "The person's first name\\.\n\nSource: [my.schema.uri](my.schema.uri)",
82+
value: "The person's first name.\n\nSource: [my.schema.uri](my.schema.uri)",
8383
},
8484
range: {
8585
start: {

0 commit comments

Comments
 (0)