From 33cc9e761936dcf949d2c9748f694ebaf43d7f0e Mon Sep 17 00:00:00 2001 From: David Cummings Date: Wed, 5 Mar 2025 09:24:15 -0500 Subject: [PATCH] Improve UX around educational notes The Swift compiler contains educational notes to further describe diagnostics [1]. These educational notes are documented in markdown files that are contained within the toolchain. Sourcekit LSP includes a link to the local markdown file when returning diagnostics that have an associated educational note (as part of the diagnostic code). The default behaviour in VSCode is to present these as a link in the diagnostic hover, and open the editor to the markdown file when the link is clicked. This PR updates the behaviour for educational notes to instead open the link using the markdown preview, which shows nicely rendered content. It also updates the link in the hover to show "More Information" instead of the code. Issue: #1395 [1] https://github.com/swiftlang/swift/tree/main/userdocs/diagnostics --- package.json | 9 ++ src/DiagnosticsManager.ts | 18 +++ src/commands.ts | 4 + src/commands/openEducationalNote.ts | 24 ++++ .../DiagnosticsManager.test.ts | 111 ++++++++++++++++++ 5 files changed, 166 insertions(+) create mode 100644 src/commands/openEducationalNote.ts diff --git a/package.json b/package.json index 9f216849a..99d6dbea1 100644 --- a/package.json +++ b/package.json @@ -111,6 +111,11 @@ "title": "Create New Project...", "category": "Swift" }, + { + "command": "swift.openEducationalNote", + "title": "Open Educational Note...", + "category": "Swift" + }, { "command": "swift.newFile", "title": "Create New Swift File...", @@ -943,6 +948,10 @@ { "command": "swift.coverAllTests", "when": "swift.isActivated" + }, + { + "command": "swift.openEducationalNote", + "when": "false" } ], "editor/context": [ diff --git a/src/DiagnosticsManager.ts b/src/DiagnosticsManager.ts index 7f2ea1578..e2ab4570d 100644 --- a/src/DiagnosticsManager.ts +++ b/src/DiagnosticsManager.ts @@ -142,6 +142,24 @@ export class DiagnosticsManager implements vscode.Disposable { d1 => isSwiftc(d1) && !!removedDiagnostics.find(d2 => isEqual(d1, d2)) ); } + + for (const diagnostic of newDiagnostics) { + if ( + diagnostic.code && + typeof diagnostic.code !== "string" && + typeof diagnostic.code !== "number" + ) { + if (diagnostic.code.target.fsPath.endsWith(".md")) { + diagnostic.code = { + target: vscode.Uri.parse( + `command:swift.openEducationalNote?${encodeURIComponent(JSON.stringify(diagnostic.code.target))}` + ), + value: "More Information...", + }; + } + } + } + // Append the new diagnostics we just received allDiagnostics.push(...newDiagnostics); this.allDiagnostics.set(uri.fsPath, allDiagnostics); diff --git a/src/commands.ts b/src/commands.ts index a28979167..6a6b08a32 100644 --- a/src/commands.ts +++ b/src/commands.ts @@ -31,6 +31,7 @@ import { openInExternalEditor } from "./commands/openInExternalEditor"; import { switchPlatform } from "./commands/switchPlatform"; import { insertFunctionComment } from "./commands/insertFunctionComment"; import { createNewProject } from "./commands/createNewProject"; +import { openEducationalNote } from "./commands/openEducationalNote"; import { openPackage } from "./commands/openPackage"; import { resolveDependencies } from "./commands/dependencies/resolve"; import { resetPackage } from "./commands/resetPackage"; @@ -199,6 +200,9 @@ export function register(ctx: WorkspaceContext): vscode.Disposable[] { vscode.commands.registerCommand(Commands.SHOW_NESTED_DEPENDENCIES_LIST, () => updateDependenciesViewList(ctx, false) ), + vscode.commands.registerCommand("swift.openEducationalNote", uri => + openEducationalNote(uri) + ), ]; } diff --git a/src/commands/openEducationalNote.ts b/src/commands/openEducationalNote.ts new file mode 100644 index 000000000..ea3dbe677 --- /dev/null +++ b/src/commands/openEducationalNote.ts @@ -0,0 +1,24 @@ +//===----------------------------------------------------------------------===// +// +// This source file is part of the VS Code Swift open source project +// +// Copyright (c) 2021-2025 the VS Code Swift project authors +// Licensed under Apache License v2.0 +// +// See LICENSE.txt for license information +// See CONTRIBUTORS.txt for the list of VS Code Swift project authors +// +// SPDX-License-Identifier: Apache-2.0 +// +//===----------------------------------------------------------------------===// + +import * as vscode from "vscode"; + +/** + * Handle the user requesting to show an educational note. + * + * The default behaviour is to open it in a markdown preview to the side. + */ +export async function openEducationalNote(markdownFile: vscode.Uri | undefined): Promise { + await vscode.commands.executeCommand("markdown.showPreviewToSide", markdownFile); +} diff --git a/test/integration-tests/DiagnosticsManager.test.ts b/test/integration-tests/DiagnosticsManager.test.ts index b0a783090..5ac97f374 100644 --- a/test/integration-tests/DiagnosticsManager.test.ts +++ b/test/integration-tests/DiagnosticsManager.test.ts @@ -24,6 +24,7 @@ import { FolderContext } from "../../src/FolderContext"; import { Version } from "../../src/utilities/version"; import { Workbench } from "../../src/utilities/commands"; import { activateExtensionForSuite, folderInRootWorkspace } from "./utilities/testutilities"; +import { expect } from "chai"; const isEqual = (d1: vscode.Diagnostic, d2: vscode.Diagnostic) => { return ( @@ -555,6 +556,116 @@ suite("DiagnosticsManager Test Suite", async function () { await swiftConfig.update("diagnosticsCollection", undefined); }); + suite("markdownLinks", () => { + let diagnostic: vscode.Diagnostic; + + setup(async () => { + workspaceContext.diagnostics.clear(); + diagnostic = new vscode.Diagnostic( + new vscode.Range(new vscode.Position(1, 8), new vscode.Position(1, 8)), // Note swiftc provides empty range + "Cannot assign to value: 'bar' is a 'let' constant", + vscode.DiagnosticSeverity.Error + ); + diagnostic.source = "SourceKit"; + }); + + test("ignore strings", async () => { + diagnostic.code = "string"; + + // Now provide identical SourceKit diagnostic + workspaceContext.diagnostics.handleDiagnostics( + mainUri, + DiagnosticsManager.isSourcekit, + [diagnostic] + ); + + // check diagnostic hasn't changed + assertHasDiagnostic(mainUri, diagnostic); + + const diagnostics = vscode.languages.getDiagnostics(mainUri); + const matchingDiagnostic = diagnostics.find(findDiagnostic(diagnostic)); + + expect(matchingDiagnostic).to.have.property("code", "string"); + }); + + test("ignore numbers", async () => { + diagnostic.code = 1; + + // Now provide identical SourceKit diagnostic + workspaceContext.diagnostics.handleDiagnostics( + mainUri, + DiagnosticsManager.isSourcekit, + [diagnostic] + ); + + // check diagnostic hasn't changed + assertHasDiagnostic(mainUri, diagnostic); + + const diagnostics = vscode.languages.getDiagnostics(mainUri); + const matchingDiagnostic = diagnostics.find(findDiagnostic(diagnostic)); + + expect(matchingDiagnostic).to.have.property("code", 1); + }); + + test("target without markdown link", async () => { + const diagnosticCode = { + value: "string", + target: vscode.Uri.file("/some/path/md/readme.txt"), + }; + diagnostic.code = diagnosticCode; + + // Now provide identical SourceKit diagnostic + workspaceContext.diagnostics.handleDiagnostics( + mainUri, + DiagnosticsManager.isSourcekit, + [diagnostic] + ); + + // check diagnostic hasn't changed + assertHasDiagnostic(mainUri, diagnostic); + + const diagnostics = vscode.languages.getDiagnostics(mainUri); + const matchingDiagnostic = diagnostics.find(findDiagnostic(diagnostic)); + + expect(matchingDiagnostic).to.have.property("code", diagnostic.code); + }); + + test("target with markdown link", async () => { + const pathToMd = "/some/path/md/readme.md"; + diagnostic.code = { + value: "string", + target: vscode.Uri.file(pathToMd), + }; + + workspaceContext.diagnostics.handleDiagnostics( + mainUri, + DiagnosticsManager.isSourcekit, + [diagnostic] + ); + + const diagnostics = vscode.languages.getDiagnostics(mainUri); + const matchingDiagnostic = diagnostics.find(findDiagnostic(diagnostic)); + + expect(matchingDiagnostic).to.have.property("code"); + expect(matchingDiagnostic?.code).to.have.property("value", "More Information..."); + + if ( + matchingDiagnostic && + matchingDiagnostic.code && + typeof matchingDiagnostic.code !== "string" && + typeof matchingDiagnostic.code !== "number" + ) { + expect(matchingDiagnostic.code.target.scheme).to.equal("command"); + expect(matchingDiagnostic.code.target.path).to.equal( + "swift.openEducationalNote" + ); + expect(matchingDiagnostic.code.target.query).to.contain(pathToMd); + } else { + assert.fail("Diagnostic target not replaced with markdown command"); + } + }); + }); + suite("keepAll", () => { setup(async () => { await swiftConfig.update("diagnosticsCollection", "keepAll");