Skip to content

Commit 1a07fd1

Browse files
authored
Clarify markdown validate settings (microsoft#151997)
Clairify markdown validate settings Fixes microsoft#150949 - Rename headerLinks -> fragmentLinks - Add new `fileLink.markdownFragmentsLinks` to validate the headers on fragment links (inherits the default setting value from `fragmentLinks`)
1 parent af754ef commit 1a07fd1

File tree

4 files changed

+101
-49
lines changed

4 files changed

+101
-49
lines changed

extensions/markdown-language-features/package.json

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -447,10 +447,10 @@
447447
"experimental"
448448
]
449449
},
450-
"markdown.experimental.validate.headerLinks.enabled": {
450+
"markdown.experimental.validate.fragmentLinks.enabled": {
451451
"type": "string",
452452
"scope": "resource",
453-
"markdownDescription": "%configuration.markdown.experimental.validate.headerLinks.enabled.description%",
453+
"markdownDescription": "%configuration.markdown.experimental.validate.fragmentLinks.enabled.description%",
454454
"default": "warning",
455455
"enum": [
456456
"ignore",
@@ -475,6 +475,19 @@
475475
"experimental"
476476
]
477477
},
478+
"markdown.experimental.validate.fileLinks.markdownFragmentLinks": {
479+
"type": "string",
480+
"scope": "resource",
481+
"markdownDescription": "%configuration.markdown.experimental.validate.fileLinks.markdownFragmentLinks.description%",
482+
"enum": [
483+
"ignore",
484+
"warning",
485+
"error"
486+
],
487+
"tags": [
488+
"experimental"
489+
]
490+
},
478491
"markdown.experimental.validate.ignoreLinks": {
479492
"type": "array",
480493
"scope": "resource",

extensions/markdown-language-features/package.nls.json

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,8 +32,9 @@
3232
"configuration.markdown.editor.pasteLinks.enabled": "Enable/disable pasting files into a Markdown editor inserts Markdown links.",
3333
"configuration.markdown.experimental.validate.enabled.description": "Enable/disable all error reporting in Markdown files.",
3434
"configuration.markdown.experimental.validate.referenceLinks.enabled.description": "Validate reference links in Markdown files, e.g. `[link][ref]`. Requires enabling `#markdown.experimental.validate.enabled#`.",
35-
"configuration.markdown.experimental.validate.headerLinks.enabled.description": "Validate links to headers in Markdown files, e.g. `[link](#header)`. Requires enabling `#markdown.experimental.validate.enabled#`.",
35+
"configuration.markdown.experimental.validate.fragmentLinks.enabled.description": "Validate fragment links to headers in the current Markdown file, e.g. `[link](#header)`. Requires enabling `#markdown.experimental.validate.enabled#`.",
3636
"configuration.markdown.experimental.validate.fileLinks.enabled.description": "Validate links to other files in Markdown files, e.g. `[link](/path/to/file.md)`. This checks that the target files exists. Requires enabling `#markdown.experimental.validate.enabled#`.",
37+
"configuration.markdown.experimental.validate.fileLinks.markdownFragmentLinks.description": "Validate the fragment part of links to headers in other files in Markdown files, e.g. `[link](/path/to/file.md#header)`. Inherits the setting value from `#markdown.experimental.validate.fragmentLinks.enabled#` by default.",
3738
"configuration.markdown.experimental.validate.ignoreLinks.description": "Configure links that should not be validated. For example `/about` would not validate the link `[about](/about)`, while the glob `/assets/**/*.svg` would let you skip validation for any link to `.svg` files under the `assets` directory.",
3839
"workspaceTrust": "Required for loading styles configured in the workspace."
3940
}

extensions/markdown-language-features/src/languageFeatures/diagnostics.ts

Lines changed: 23 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -37,17 +37,19 @@ export enum DiagnosticLevel {
3737

3838
export interface DiagnosticOptions {
3939
readonly enabled: boolean;
40-
readonly validateReferences: DiagnosticLevel;
41-
readonly validateOwnHeaders: DiagnosticLevel;
42-
readonly validateFilePaths: DiagnosticLevel;
40+
readonly validateReferences: DiagnosticLevel | undefined;
41+
readonly validateFragmentLinks: DiagnosticLevel | undefined;
42+
readonly validateFileLinks: DiagnosticLevel | undefined;
43+
readonly validateMarkdownFileLinkFragments: DiagnosticLevel | undefined;
4344
readonly ignoreLinks: readonly string[];
4445
}
4546

46-
function toSeverity(level: DiagnosticLevel): vscode.DiagnosticSeverity | undefined {
47+
function toSeverity(level: DiagnosticLevel | undefined): vscode.DiagnosticSeverity | undefined {
4748
switch (level) {
4849
case DiagnosticLevel.error: return vscode.DiagnosticSeverity.Error;
4950
case DiagnosticLevel.warning: return vscode.DiagnosticSeverity.Warning;
5051
case DiagnosticLevel.ignore: return undefined;
52+
case undefined: return undefined;
5153
}
5254
}
5355

@@ -63,8 +65,9 @@ class VSCodeDiagnosticConfiguration extends Disposable implements DiagnosticConf
6365
if (
6466
e.affectsConfiguration('markdown.experimental.validate.enabled')
6567
|| e.affectsConfiguration('markdown.experimental.validate.referenceLinks.enabled')
66-
|| e.affectsConfiguration('markdown.experimental.validate.headerLinks.enabled')
68+
|| e.affectsConfiguration('markdown.experimental.validate.fragmentLinks.enabled')
6769
|| e.affectsConfiguration('markdown.experimental.validate.fileLinks.enabled')
70+
|| e.affectsConfiguration('markdown.experimental.validate.fileLinks.markdownFragmentLinks')
6871
|| e.affectsConfiguration('markdown.experimental.validate.ignoreLinks')
6972
) {
7073
this._onDidChange.fire();
@@ -74,11 +77,13 @@ class VSCodeDiagnosticConfiguration extends Disposable implements DiagnosticConf
7477

7578
public getOptions(resource: vscode.Uri): DiagnosticOptions {
7679
const config = vscode.workspace.getConfiguration('markdown', resource);
80+
const validateFragmentLinks = config.get<DiagnosticLevel>('experimental.validate.fragmentLinks.enabled');
7781
return {
7882
enabled: config.get<boolean>('experimental.validate.enabled', false),
79-
validateReferences: config.get<DiagnosticLevel>('experimental.validate.referenceLinks.enabled', DiagnosticLevel.ignore),
80-
validateOwnHeaders: config.get<DiagnosticLevel>('experimental.validate.headerLinks.enabled', DiagnosticLevel.ignore),
81-
validateFilePaths: config.get<DiagnosticLevel>('experimental.validate.fileLinks.enabled', DiagnosticLevel.ignore),
83+
validateReferences: config.get<DiagnosticLevel>('experimental.validate.referenceLinks.enabled'),
84+
validateFragmentLinks,
85+
validateFileLinks: config.get<DiagnosticLevel>('experimental.validate.fileLinks.enabled'),
86+
validateMarkdownFileLinkFragments: config.get<DiagnosticLevel | undefined>('markdown.experimental.validate.fileLinks.markdownFragmentLinks', validateFragmentLinks),
8287
ignoreLinks: config.get('experimental.validate.ignoreLinks', []),
8388
};
8489
}
@@ -294,7 +299,7 @@ export class DiagnosticManager extends Disposable {
294299
if (doc) {
295300
this.inFlightDiagnostics.trigger(doc.uri, async (token) => {
296301
const state = await this.recomputeDiagnosticState(doc, token);
297-
this.linkWatcher.updateLinksForDocument(doc.uri, state.config.enabled && state.config.validateFilePaths ? state.links : []);
302+
this.linkWatcher.updateLinksForDocument(doc.uri, state.config.enabled && state.config.validateFileLinks ? state.links : []);
298303
this.collection.set(doc.uri, state.diagnostics);
299304
});
300305
}
@@ -395,13 +400,13 @@ export class DiagnosticComputer {
395400
diagnostics: (await Promise.all([
396401
this.validateFileLinks(doc, options, links, token),
397402
Array.from(this.validateReferenceLinks(options, links)),
398-
this.validateOwnHeaderLinks(doc, options, links, token),
403+
this.validateFragmentLinks(doc, options, links, token),
399404
])).flat()
400405
};
401406
}
402407

403-
private async validateOwnHeaderLinks(doc: SkinnyTextDocument, options: DiagnosticOptions, links: readonly MdLink[], token: vscode.CancellationToken): Promise<vscode.Diagnostic[]> {
404-
const severity = toSeverity(options.validateOwnHeaders);
408+
private async validateFragmentLinks(doc: SkinnyTextDocument, options: DiagnosticOptions, links: readonly MdLink[], token: vscode.CancellationToken): Promise<vscode.Diagnostic[]> {
409+
const severity = toSeverity(options.validateFragmentLinks);
405410
if (typeof severity === 'undefined') {
406411
return [];
407412
}
@@ -449,10 +454,11 @@ export class DiagnosticComputer {
449454
}
450455

451456
private async validateFileLinks(doc: SkinnyTextDocument, options: DiagnosticOptions, links: readonly MdLink[], token: vscode.CancellationToken): Promise<vscode.Diagnostic[]> {
452-
const severity = toSeverity(options.validateFilePaths);
453-
if (typeof severity === 'undefined') {
457+
const pathErrorSeverity = toSeverity(options.validateFileLinks);
458+
if (typeof pathErrorSeverity === 'undefined') {
454459
return [];
455460
}
461+
const fragmentErrorSeverity = toSeverity(typeof options.validateMarkdownFileLinkFragments === 'undefined' ? options.validateFragmentLinks : options.validateMarkdownFileLinkFragments);
456462

457463
const linkSet = new FileLinkMap(links);
458464
if (linkSet.size === 0) {
@@ -479,18 +485,18 @@ export class DiagnosticComputer {
479485
const msg = localize('invalidPathLink', 'File does not exist at path: {0}', path.fsPath);
480486
for (const link of links) {
481487
if (!this.isIgnoredLink(options, link.source.pathText)) {
482-
diagnostics.push(new LinkDoesNotExistDiagnostic(link.source.hrefRange, msg, severity, link.source.pathText));
488+
diagnostics.push(new LinkDoesNotExistDiagnostic(link.source.hrefRange, msg, pathErrorSeverity, link.source.pathText));
483489
}
484490
}
485-
} else if (hrefDoc) {
491+
} else if (hrefDoc && typeof fragmentErrorSeverity !== 'undefined') {
486492
// Validate each of the links to headers in the file
487493
const fragmentLinks = links.filter(x => x.fragment);
488494
if (fragmentLinks.length) {
489495
const toc = await TableOfContents.create(this.engine, hrefDoc);
490496
for (const link of fragmentLinks) {
491497
if (!toc.lookup(link.fragment) && !this.isIgnoredLink(options, link.source.pathText) && !this.isIgnoredLink(options, link.source.text)) {
492498
const msg = localize('invalidLinkToHeaderInOtherFile', 'Header does not exist in file: {0}', link.fragment);
493-
diagnostics.push(new LinkDoesNotExistDiagnostic(link.source.hrefRange, msg, severity, link.source.text));
499+
diagnostics.push(new LinkDoesNotExistDiagnostic(link.source.hrefRange, msg, fragmentErrorSeverity, link.source.text));
494500
}
495501
}
496502
}

extensions/markdown-language-features/src/test/diagnostic.test.ts

Lines changed: 61 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -23,15 +23,16 @@ async function getComputedDiagnostics(doc: InMemoryDocument, workspaceContents:
2323
return (
2424
await computer.getDiagnostics(doc, {
2525
enabled: true,
26-
validateFilePaths: DiagnosticLevel.warning,
27-
validateOwnHeaders: DiagnosticLevel.warning,
26+
validateFileLinks: DiagnosticLevel.warning,
27+
validateFragmentLinks: DiagnosticLevel.warning,
28+
validateMarkdownFileLinkFragments: DiagnosticLevel.warning,
2829
validateReferences: DiagnosticLevel.warning,
2930
ignoreLinks: [],
3031
}, noopToken)
3132
).diagnostics;
3233
}
3334

34-
function createDiagnosticsManager(workspaceContents: MdWorkspaceContents, configuration = new MemoryDiagnosticConfiguration()) {
35+
function createDiagnosticsManager(workspaceContents: MdWorkspaceContents, configuration = new MemoryDiagnosticConfiguration({})) {
3536
const engine = createNewMarkdownEngine();
3637
const linkProvider = new MdLinkProvider(engine);
3738
return new DiagnosticManager(new DiagnosticComputer(engine, workspaceContents, linkProvider), configuration);
@@ -45,32 +46,28 @@ function assertDiagnosticsEqual(actual: readonly vscode.Diagnostic[], expectedRa
4546
}
4647
}
4748

49+
const defaultDiagnosticsOptions = Object.freeze<DiagnosticOptions>({
50+
enabled: true,
51+
validateFileLinks: DiagnosticLevel.warning,
52+
validateMarkdownFileLinkFragments: undefined,
53+
validateFragmentLinks: DiagnosticLevel.warning,
54+
validateReferences: DiagnosticLevel.warning,
55+
ignoreLinks: [],
56+
});
57+
4858
class MemoryDiagnosticConfiguration implements DiagnosticConfiguration {
4959

5060
private readonly _onDidChange = new vscode.EventEmitter<void>();
5161
public readonly onDidChange = this._onDidChange.event;
5262

5363
constructor(
54-
private readonly enabled: boolean = true,
55-
private readonly ignoreLinks: string[] = [],
64+
private readonly _options: Partial<DiagnosticOptions>,
5665
) { }
5766

5867
getOptions(_resource: vscode.Uri): DiagnosticOptions {
59-
if (!this.enabled) {
60-
return {
61-
enabled: false,
62-
validateFilePaths: DiagnosticLevel.ignore,
63-
validateOwnHeaders: DiagnosticLevel.ignore,
64-
validateReferences: DiagnosticLevel.ignore,
65-
ignoreLinks: this.ignoreLinks,
66-
};
67-
}
6868
return {
69-
enabled: true,
70-
validateFilePaths: DiagnosticLevel.warning,
71-
validateOwnHeaders: DiagnosticLevel.warning,
72-
validateReferences: DiagnosticLevel.warning,
73-
ignoreLinks: this.ignoreLinks,
69+
...defaultDiagnosticsOptions,
70+
...this._options,
7471
};
7572
}
7673
}
@@ -172,7 +169,7 @@ suite('markdown: Diagnostics', () => {
172169
`[text][no-such-ref]`,
173170
));
174171

175-
const manager = createDiagnosticsManager(new InMemoryWorkspaceMarkdownDocuments([doc1]), new MemoryDiagnosticConfiguration(false));
172+
const manager = createDiagnosticsManager(new InMemoryWorkspaceMarkdownDocuments([doc1]), new MemoryDiagnosticConfiguration({ enabled: false }));
176173
const { diagnostics } = await manager.recomputeDiagnosticState(doc1, noopToken);
177174
assert.deepStrictEqual(diagnostics.length, 0);
178175
});
@@ -203,17 +200,52 @@ suite('markdown: Diagnostics', () => {
203200
`[text]: /no-such-file`,
204201
));
205202

206-
const manager = createDiagnosticsManager(new InMemoryWorkspaceMarkdownDocuments([doc1]), new MemoryDiagnosticConfiguration(true, ['/no-such-file']));
203+
const manager = createDiagnosticsManager(new InMemoryWorkspaceMarkdownDocuments([doc1]), new MemoryDiagnosticConfiguration({ ignoreLinks: ['/no-such-file'] }));
204+
const { diagnostics } = await manager.recomputeDiagnosticState(doc1, noopToken);
205+
assert.deepStrictEqual(diagnostics.length, 0);
206+
});
207+
208+
test('Should be able to disable fragment validation for external files', async () => {
209+
const doc1 = new InMemoryDocument(workspacePath('doc1.md'), joinLines(
210+
`![i](/doc2.md#no-such)`,
211+
));
212+
const doc2 = new InMemoryDocument(workspacePath('doc2.md'), joinLines(''));
213+
214+
const contents = new InMemoryWorkspaceMarkdownDocuments([doc1, doc2]);
215+
216+
const manager = createDiagnosticsManager(contents, new MemoryDiagnosticConfiguration({ validateMarkdownFileLinkFragments: DiagnosticLevel.ignore }));
207217
const { diagnostics } = await manager.recomputeDiagnosticState(doc1, noopToken);
208218
assert.deepStrictEqual(diagnostics.length, 0);
209219
});
210220

221+
test('Disabling own fragment validation should also disable path fragment validation by default', async () => {
222+
const doc1 = new InMemoryDocument(workspacePath('doc1.md'), joinLines(
223+
`[b](#no-head)`,
224+
`![i](/doc2.md#no-such)`,
225+
));
226+
const doc2 = new InMemoryDocument(workspacePath('doc2.md'), joinLines(''));
227+
228+
const contents = new InMemoryWorkspaceMarkdownDocuments([doc1, doc2]);
229+
230+
{
231+
const manager = createDiagnosticsManager(contents, new MemoryDiagnosticConfiguration({ validateFragmentLinks: DiagnosticLevel.ignore }));
232+
const { diagnostics } = await manager.recomputeDiagnosticState(doc1, noopToken);
233+
assert.deepStrictEqual(diagnostics.length, 0);
234+
}
235+
{
236+
// But we should be able to override the default
237+
const manager = createDiagnosticsManager(contents, new MemoryDiagnosticConfiguration({ validateFragmentLinks: DiagnosticLevel.ignore, validateMarkdownFileLinkFragments: DiagnosticLevel.warning }));
238+
const { diagnostics } = await manager.recomputeDiagnosticState(doc1, noopToken);
239+
assert.deepStrictEqual(diagnostics.length, 1);
240+
}
241+
});
242+
211243
test('ignoreLinks should allow skipping link to non-existent file', async () => {
212244
const doc1 = new InMemoryDocument(workspacePath('doc1.md'), joinLines(
213245
`[text](/no-such-file#header)`,
214246
));
215247

216-
const manager = createDiagnosticsManager(new InMemoryWorkspaceMarkdownDocuments([doc1]), new MemoryDiagnosticConfiguration(true, ['/no-such-file']));
248+
const manager = createDiagnosticsManager(new InMemoryWorkspaceMarkdownDocuments([doc1]), new MemoryDiagnosticConfiguration({ ignoreLinks: ['/no-such-file'] }));
217249
const { diagnostics } = await manager.recomputeDiagnosticState(doc1, noopToken);
218250
assert.deepStrictEqual(diagnostics.length, 0);
219251
});
@@ -223,7 +255,7 @@ suite('markdown: Diagnostics', () => {
223255
`[text](/no-such-file#header)`,
224256
));
225257

226-
const manager = createDiagnosticsManager(new InMemoryWorkspaceMarkdownDocuments([doc1]), new MemoryDiagnosticConfiguration(true, ['/no-such-file']));
258+
const manager = createDiagnosticsManager(new InMemoryWorkspaceMarkdownDocuments([doc1]), new MemoryDiagnosticConfiguration({ ignoreLinks: ['/no-such-file'] }));
227259
const { diagnostics } = await manager.recomputeDiagnosticState(doc1, noopToken);
228260
assert.deepStrictEqual(diagnostics.length, 0);
229261
});
@@ -235,7 +267,7 @@ suite('markdown: Diagnostics', () => {
235267
`![i](/images/sub/sub2/ccc.png)`,
236268
));
237269

238-
const manager = createDiagnosticsManager(new InMemoryWorkspaceMarkdownDocuments([doc1]), new MemoryDiagnosticConfiguration(true, ['/images/**/*.png']));
270+
const manager = createDiagnosticsManager(new InMemoryWorkspaceMarkdownDocuments([doc1]), new MemoryDiagnosticConfiguration({ ignoreLinks: ['/images/**/*.png'] }));
239271
const { diagnostics } = await manager.recomputeDiagnosticState(doc1, noopToken);
240272
assert.deepStrictEqual(diagnostics.length, 0);
241273
});
@@ -245,7 +277,7 @@ suite('markdown: Diagnostics', () => {
245277
`![i](#no-such)`,
246278
));
247279

248-
const manager = createDiagnosticsManager(new InMemoryWorkspaceMarkdownDocuments([doc1]), new MemoryDiagnosticConfiguration(true, ['#no-such']));
280+
const manager = createDiagnosticsManager(new InMemoryWorkspaceMarkdownDocuments([doc1]), new MemoryDiagnosticConfiguration({ ignoreLinks: ['#no-such'] }));
249281
const { diagnostics } = await manager.recomputeDiagnosticState(doc1, noopToken);
250282
assert.deepStrictEqual(diagnostics.length, 0);
251283
});
@@ -258,12 +290,12 @@ suite('markdown: Diagnostics', () => {
258290

259291
const contents = new InMemoryWorkspaceMarkdownDocuments([doc1, doc2]);
260292
{
261-
const manager = createDiagnosticsManager(contents, new MemoryDiagnosticConfiguration(true, ['/doc2.md#no-such']));
293+
const manager = createDiagnosticsManager(contents, new MemoryDiagnosticConfiguration({ ignoreLinks: ['/doc2.md#no-such'] }));
262294
const { diagnostics } = await manager.recomputeDiagnosticState(doc1, noopToken);
263295
assert.deepStrictEqual(diagnostics.length, 0);
264296
}
265297
{
266-
const manager = createDiagnosticsManager(contents, new MemoryDiagnosticConfiguration(true, ['/doc2.md#*']));
298+
const manager = createDiagnosticsManager(contents, new MemoryDiagnosticConfiguration({ ignoreLinks: ['/doc2.md#*'] }));
267299
const { diagnostics } = await manager.recomputeDiagnosticState(doc1, noopToken);
268300
assert.deepStrictEqual(diagnostics.length, 0);
269301
}
@@ -276,7 +308,7 @@ suite('markdown: Diagnostics', () => {
276308
const doc2 = new InMemoryDocument(workspacePath('doc2.md'), joinLines(''));
277309

278310
const contents = new InMemoryWorkspaceMarkdownDocuments([doc1, doc2]);
279-
const manager = createDiagnosticsManager(contents, new MemoryDiagnosticConfiguration(true, ['/doc2.md']));
311+
const manager = createDiagnosticsManager(contents, new MemoryDiagnosticConfiguration({ ignoreLinks: ['/doc2.md'] }));
280312
const { diagnostics } = await manager.recomputeDiagnosticState(doc1, noopToken);
281313
assert.deepStrictEqual(diagnostics.length, 0);
282314
});
@@ -289,7 +321,7 @@ suite('markdown: Diagnostics', () => {
289321
));
290322

291323
const contents = new InMemoryWorkspaceMarkdownDocuments([doc1]);
292-
const manager = createDiagnosticsManager(contents, new MemoryDiagnosticConfiguration(true, ['/doc2.md']));
324+
const manager = createDiagnosticsManager(contents, new MemoryDiagnosticConfiguration({ ignoreLinks: ['/doc2.md'] }));
293325
const { diagnostics } = await manager.recomputeDiagnosticState(doc1, noopToken);
294326
assert.deepStrictEqual(diagnostics.length, 0);
295327
});

0 commit comments

Comments
 (0)