From bff00c64b828bbb126667b8d4f2709ef8934d477 Mon Sep 17 00:00:00 2001 From: Ethan Reesor <ethan.reesor@gmail.com> Date: Tue, 24 Aug 2021 13:34:00 -0500 Subject: [PATCH 1/2] feat: show markdown help for missing token --- src/command_names.ts | 1 + src/commands/open_repository.test.ts | 4 +- src/constants.test.ts | 25 +++++++ src/constants.ts | 5 ++ src/errors/help_error.ts | 7 ++ src/extension.js | 3 + src/gitlab_service.ts | 5 +- src/log.ts | 17 ++++- src/remotefs/gitlab_remote_file_system.ts | 81 ++++++++++++++--------- src/test_utils/uri.ts | 11 ++- src/utils/context_utils.ts | 17 +++++ src/utils/help.test.ts | 30 +++++++++ src/utils/help.ts | 41 ++++++++++++ 13 files changed, 205 insertions(+), 42 deletions(-) create mode 100644 src/constants.test.ts create mode 100644 src/errors/help_error.ts create mode 100644 src/utils/context_utils.ts create mode 100644 src/utils/help.test.ts create mode 100644 src/utils/help.ts diff --git a/src/command_names.ts b/src/command_names.ts index 3f3ed4da8..f19a27d6b 100644 --- a/src/command_names.ts +++ b/src/command_names.ts @@ -54,4 +54,5 @@ export const VS_COMMANDS = { OPEN: 'vscode.open', GIT_SHOW_OUTPUT: 'git.showOutput', GIT_CLONE: 'git.clone', + MARKDOWN_SHOW_PREVIEW: 'markdown.showPreview', }; diff --git a/src/commands/open_repository.test.ts b/src/commands/open_repository.test.ts index 1aaf9a9be..8276d1082 100644 --- a/src/commands/open_repository.test.ts +++ b/src/commands/open_repository.test.ts @@ -1,5 +1,6 @@ import * as vscode from 'vscode'; import { REMOTE_URI_SCHEME } from '../constants'; +import { HelpError } from '../errors/help_error'; import { tokenService } from '../services/token_service'; import { OpenAction, openRepository } from './open_repository'; @@ -49,8 +50,7 @@ describe('openRepository', () => { const uri = `not-${REMOTE_URI_SCHEME}://gitlab.com/GitLab?project=gitlab-org/gitlab&ref=main`; alwaysPick('new-window'); alwaysInput(uri); - await openRepository(); - expect(vscode.window.showErrorMessage).toHaveBeenCalled(); + await expect(openRepository()).rejects.toThrow(HelpError); expect(vscode.commands.executeCommand).not.toHaveBeenCalled(); }); }); diff --git a/src/constants.test.ts b/src/constants.test.ts new file mode 100644 index 000000000..69ec6af32 --- /dev/null +++ b/src/constants.test.ts @@ -0,0 +1,25 @@ +import { promises as fs } from 'fs'; +import * as path from 'path'; +import { README_SECTIONS } from './constants'; + +describe('readme sections', () => { + const headings: string[] = []; + + beforeAll(async () => { + const readme = await fs.readFile(path.join(__dirname, '..', 'README.md'), 'utf-8'); + + readme.replace(/^#+(.*)$/gm, (s, heading) => { + headings.push( + (heading as string) + .trim() + .toLowerCase() + .replace(/\W/g, '-'), + ); + return s; + }); + }); + + it.each(Object.values(README_SECTIONS))('Readme contains "%s" section', (section: string) => { + expect(headings).toContain(section); + }); +}); diff --git a/src/constants.ts b/src/constants.ts index 17254ba25..d9de902b8 100644 --- a/src/constants.ts +++ b/src/constants.ts @@ -20,3 +20,8 @@ export const PATCH_FILE_SUFFIX = '.patch'; export const SYNCED_COMMENT_CONTEXT = 'synced-comment'; /** Failed comment is only stored in the extension, it failed to be created in GitLab */ export const FAILED_COMMENT_CONTEXT = 'failed-comment'; + +export const README_SECTIONS = { + SETUP: 'setup', + REMOTEFS: 'browse-a-repository-without-cloning', +}; diff --git a/src/errors/help_error.ts b/src/errors/help_error.ts new file mode 100644 index 000000000..80ca9752a --- /dev/null +++ b/src/errors/help_error.ts @@ -0,0 +1,7 @@ +import { HelpOptions } from '../utils/help'; + +export class HelpError extends Error { + constructor(message: string, public readonly options?: HelpOptions) { + super(message); + } +} diff --git a/src/extension.js b/src/extension.js index 227492b52..ce57270b5 100644 --- a/src/extension.js +++ b/src/extension.js @@ -38,6 +38,7 @@ const { applySnippetPatch } = require('./commands/apply_snippet_patch'); const { openMrFile } = require('./commands/open_mr_file'); const { GitLabRemoteFileSystem } = require('./remotefs/gitlab_remote_file_system'); const { openRepository } = require('./commands/open_repository'); +const { contextUtils } = require('./utils/context_utils'); const wrapWithCatch = command => async (...args) => { try { @@ -119,6 +120,8 @@ const registerCiCompletion = context => { * @param {vscode.ExtensionContext} context */ const activate = context => { + contextUtils.init(context); + const outputChannel = vscode.window.createOutputChannel('GitLab Workflow'); initializeLogging(line => outputChannel.appendLine(line)); vscode.workspace.registerTextDocumentContentProvider(REVIEW_URI_SCHEME, new GitContentProvider()); diff --git a/src/gitlab_service.ts b/src/gitlab_service.ts index f22f8e9b8..c001b5262 100644 --- a/src/gitlab_service.ts +++ b/src/gitlab_service.ts @@ -14,6 +14,8 @@ import { getInstanceUrl } from './utils/get_instance_url'; import { GitLabProject } from './gitlab/gitlab_project'; import { gitExtensionWrapper } from './git/git_extension_wrapper'; import { getExtensionConfiguration } from './utils/get_extension_configuration'; +import { README_SECTIONS } from './constants'; +import { HelpError } from './errors/help_error'; const normalizeAvatarUrl = (instanceUrl: string) => (issuable: RestIssuable): RestIssuable => { const { author } = issuable; @@ -53,8 +55,7 @@ async function fetch( err = `${err} You have configured tokens for ${tokens}.`; } - await vscode.window.showInformationMessage(err); - throw new Error(err); + throw new HelpError(err, { section: README_SECTIONS.SETUP }); } const config: request.RequestPromiseOptions = { diff --git a/src/log.ts b/src/log.ts index 115b5a0e7..7dc2e0fe8 100644 --- a/src/log.ts +++ b/src/log.ts @@ -1,11 +1,17 @@ import * as vscode from 'vscode'; import { USER_COMMANDS } from './command_names'; import { IDetailedError } from './errors/common'; +import { HelpError } from './errors/help_error'; +import { Help, HelpMessageSeverity } from './utils/help'; function isDetailedError(object: any): object is IDetailedError { return Boolean(object.details); } +function isHelpError(object: any): object is HelpError { + return object instanceof HelpError; +} + type logFunction = (line: string) => void; let globalLog: logFunction = console.error; @@ -20,15 +26,20 @@ export const logError = (e: Error | IDetailedError): void => isDetailedError(e) ? globalLog(e.details) : globalLog(`${e.message}\n${e.stack}`); export const handleError = (e: Error | IDetailedError): { onlyForTesting: Promise<void> } => { + // This is probably the only place where we want to ignore a floating promise. + // We don't want to block the app and wait for user click on the "Show Logs" + // button or close the message However, for testing this method, we need to + // keep the promise. + logError(e); + if (isHelpError(e)) { + return { onlyForTesting: Help.showError(e, HelpMessageSeverity.Error) }; + } const showErrorMessage = async () => { const choice = await vscode.window.showErrorMessage(e.message, 'Show Logs'); if (choice === 'Show Logs') { await vscode.commands.executeCommand(USER_COMMANDS.SHOW_OUTPUT); } }; - // This is probably the only place where we want to ignore a floating promise. - // We don't want to block the app and wait for user click on the "Show Logs" button or close the message - // However, for testing this method, we need to keep the promise return { onlyForTesting: showErrorMessage() }; }; diff --git a/src/remotefs/gitlab_remote_file_system.ts b/src/remotefs/gitlab_remote_file_system.ts index 4159b34af..8289f49f5 100644 --- a/src/remotefs/gitlab_remote_file_system.ts +++ b/src/remotefs/gitlab_remote_file_system.ts @@ -1,10 +1,10 @@ -import * as assert from 'assert'; import * as vscode from 'vscode'; -import { REMOTE_URI_SCHEME } from '../constants'; +import { README_SECTIONS, REMOTE_URI_SCHEME } from '../constants'; import { FetchError } from '../errors/fetch_error'; import { GitLabNewService } from '../gitlab/gitlab_new_service'; -import { logError } from '../log'; +import { handleError } from '../log'; import { tokenService } from '../services/token_service'; +import { HelpError } from '../errors/help_error'; import { removeTrailingSlash } from '../utils/remove_trailing_slash'; import { ReadOnlyFileSystem } from './readonly_file_system'; @@ -39,12 +39,6 @@ async function nullIf40x<T>(p: Promise<T>) { } } -// Check if the error is a FileSystemError - filesystem errors should not be -// logged. -function isFsErr(err: unknown): boolean { - return err instanceof vscode.FileSystemError; -} - interface GitLabRemotePath { instance: vscode.Uri; project: string; @@ -69,20 +63,30 @@ export class GitLabRemoteFileSystem extends ReadOnlyFileSystem { * instances, parseUri will throw an assertion error. * @param uri The URI. * @returns The parsed GitLab remote fs path. + * */ static parseUri(uri: vscode.Uri): GitLabRemotePath { - assert.strictEqual( - uri.scheme, - REMOTE_URI_SCHEME, - `URI is not a GitLab remote, it begins with ${uri.scheme} but it should begin with ${REMOTE_URI_SCHEME}`, - ); + if (uri.scheme !== REMOTE_URI_SCHEME) { + throw new HelpError( + `URI is not a GitLab remote. It begins with ${uri.scheme} but it should begin with ${REMOTE_URI_SCHEME}`, + { section: README_SECTIONS.REMOTEFS }, + ); + } const query = new URLSearchParams(uri.query); const project = query.get('project'); - assert(project, 'URI is not a GitLab remote, URI must contain a project= query parameter'); + if (!project) + throw new HelpError( + 'URI is not a GitLab remote. The URI must contain a project= query parameter', + { section: README_SECTIONS.REMOTEFS }, + ); const ref = query.get('ref'); - assert(ref, 'URI is not a GitLab remote, URI must contain a ref= query parameter'); + if (!ref) + throw new HelpError( + 'URI is not a GitLab remote. The URI must contain a ref= query parameter', + { section: README_SECTIONS.REMOTEFS }, + ); // Find the instance with a matching authority and a subpath that is a // prefix of the URI's path. @@ -90,10 +94,11 @@ export class GitLabRemoteFileSystem extends ReadOnlyFileSystem { .getInstanceUrls() .map(x => vscode.Uri.parse(x)) .find(x => uri.authority === x.authority && uri.path.startsWith(x.path)); - assert( - instance, - `Cannot open ${uri}: missing token for GitLab instance ${uri.authority} - see "Setup" in the README for help`, - ); + if (!instance) + throw new HelpError( + `Cannot open ${uri}: missing token for GitLab instance ${uri.authority}`, + { section: README_SECTIONS.SETUP }, + ); // To get the file path, we first remove the instance subpath, then the // project label. @@ -166,25 +171,37 @@ export class GitLabRemoteFileSystem extends ReadOnlyFileSystem { } /* eslint-disable class-methods-use-this */ - stat(uri: vscode.Uri): Promise<vscode.FileStat> { - return GitLabRemoteFileSystem.stat(uri).catch(e => { - if (!isFsErr(e)) logError(e); + async stat(uri: vscode.Uri): Promise<vscode.FileStat> { + try { + return await GitLabRemoteFileSystem.stat(uri); + } catch (e) { + if (!(e instanceof vscode.FileSystemError)) { + handleError(e); + } throw e; - }); + } } - readDirectory(uri: vscode.Uri): Promise<[string, vscode.FileType][]> { - return GitLabRemoteFileSystem.readDirectory(uri).catch(e => { - if (!isFsErr(e)) logError(e); + async readDirectory(uri: vscode.Uri): Promise<[string, vscode.FileType][]> { + try { + return await GitLabRemoteFileSystem.readDirectory(uri); + } catch (e) { + if (!(e instanceof vscode.FileSystemError)) { + handleError(e); + } throw e; - }); + } } - readFile(uri: vscode.Uri): Promise<Uint8Array> { - return GitLabRemoteFileSystem.readFile(uri).catch(e => { - if (!isFsErr(e)) logError(e); + async readFile(uri: vscode.Uri): Promise<Uint8Array> { + try { + return await GitLabRemoteFileSystem.readFile(uri); + } catch (e) { + if (!(e instanceof vscode.FileSystemError)) { + handleError(e); + } throw e; - }); + } } /* eslint-enable class-methods-use-this */ } diff --git a/src/test_utils/uri.ts b/src/test_utils/uri.ts index 3f82d48c8..5b13f6305 100644 --- a/src/test_utils/uri.ts +++ b/src/test_utils/uri.ts @@ -1,3 +1,4 @@ +import * as path from 'path'; import * as vscode from 'vscode'; interface UriOptions { @@ -63,13 +64,12 @@ export class Uri implements vscode.Uri { static parse(stringUri: string): Uri { const url = new URL(stringUri); - const [query, fragment = ''] = url.search.split('#'); return new Uri({ scheme: url.protocol.replace(/:$/, ''), authority: url.hostname, path: url.pathname, - query: query.replace(/^\?/, ''), - fragment, + query: url.search.replace(/^\?/, ''), + fragment: url.hash.replace(/^#/, ''), }); } @@ -82,4 +82,9 @@ export class Uri implements vscode.Uri { fragment: '', }); } + + static joinPath(base: Uri, ...pathSegments: string[]): Uri { + const { path: p, ...rest } = base; + return new this({ ...rest, path: path.join(p, ...pathSegments) }); + } } diff --git a/src/utils/context_utils.ts b/src/utils/context_utils.ts new file mode 100644 index 000000000..16c5c7d61 --- /dev/null +++ b/src/utils/context_utils.ts @@ -0,0 +1,17 @@ +import * as assert from 'assert'; +import { ExtensionContext, Uri } from 'vscode'; + +class ContextUtils { + private context: ExtensionContext | undefined; + + init(context: ExtensionContext) { + this.context = context; + } + + getEmbededFileUri(...path: string[]) { + assert(this.context, 'Context Utils is not initialized'); + return Uri.joinPath(this.context.extensionUri, ...path); + } +} + +export const contextUtils = new ContextUtils(); diff --git a/src/utils/help.test.ts b/src/utils/help.test.ts new file mode 100644 index 000000000..234e01658 --- /dev/null +++ b/src/utils/help.test.ts @@ -0,0 +1,30 @@ +import * as vscode from 'vscode'; +import { VS_COMMANDS } from '../command_names'; +import { contextUtils } from './context_utils'; +import { Help } from './help'; + +describe('Help', () => { + describe('show', () => { + beforeAll(() => { + contextUtils.init({ + extensionUri: vscode.Uri.parse(`file:///path/to/extension`), + } as vscode.ExtensionContext); + }); + + it('opens the file', async () => { + await Help.show(); + expect(vscode.commands.executeCommand).toHaveBeenCalledWith( + VS_COMMANDS.MARKDOWN_SHOW_PREVIEW, + vscode.Uri.parse(`file:///path/to/extension/README.md`), + ); + }); + + it('opens the file to the correct section', async () => { + await Help.show('foobar'); + expect(vscode.commands.executeCommand).toHaveBeenCalledWith( + VS_COMMANDS.MARKDOWN_SHOW_PREVIEW, + vscode.Uri.parse(`file:///path/to/extension/README.md#foobar`), + ); + }); + }); +}); diff --git a/src/utils/help.ts b/src/utils/help.ts new file mode 100644 index 000000000..9c005ffb3 --- /dev/null +++ b/src/utils/help.ts @@ -0,0 +1,41 @@ +import * as vscode from 'vscode'; +import { VS_COMMANDS } from '../command_names'; +import { HelpError } from '../errors/help_error'; +import { contextUtils } from './context_utils'; + +export type HelpOptions = { section: string }; + +// eslint-disable-next-line no-shadow +export enum HelpMessageSeverity { + Info, + Warning, + Error, +} + +export class Help { + static async show(section?: string): Promise<void> { + const help = contextUtils.getEmbededFileUri('README.md').with({ fragment: section }); + await vscode.commands.executeCommand(VS_COMMANDS.MARKDOWN_SHOW_PREVIEW, help); + } + + static async showError(error: HelpError, severity = HelpMessageSeverity.Info): Promise<void> { + let shouldShow = false; + switch (severity) { + default: + shouldShow = !!(await vscode.window.showInformationMessage(error.message, 'Show Help')); + break; + + case HelpMessageSeverity.Warning: + shouldShow = !!(await vscode.window.showWarningMessage(error.message, 'Show Help')); + break; + + case HelpMessageSeverity.Error: + shouldShow = !!(await vscode.window.showErrorMessage(error.message, 'Show Help')); + break; + } + + if (shouldShow) { + await Help.show(error.options?.section); + } + } +} -- GitLab From 0e3ab94f62ac607e767fe2b17b4ae8c655f70f27 Mon Sep 17 00:00:00 2001 From: Tomas Vik <tvik@gitlab.com> Date: Wed, 8 Sep 2021 15:02:17 +0000 Subject: [PATCH 2/2] Apply 1 suggestion(s) to 1 file(s) --- src/gitlab_service.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/gitlab_service.ts b/src/gitlab_service.ts index c001b5262..20ec2d4cc 100644 --- a/src/gitlab_service.ts +++ b/src/gitlab_service.ts @@ -156,7 +156,7 @@ export async function fetchVersion(repositoryRoot: string) { versionCache = response.version; } } catch (e) { - logError(e); + handleError(e); } return versionCache; -- GitLab