Skip to content

Commit

Permalink
fix: improve handling of untitled files (#3553)
Browse files Browse the repository at this point in the history
  • Loading branch information
@Jason3S
Jason3S authored Aug 20, 2024
1 parent 032c148 commit bf7e21d
Show file tree
Hide file tree
Showing 4 changed files with 106 additions and 27 deletions.
19 changes: 19 additions & 0 deletions .vscode/launch.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,25 @@
"<node_internals>/**"
]
},
{
"name": "Client: Launch Extension - single file",
"type": "extensionHost",
"request": "launch",
"runtimeExecutable": "${execPath}",
"args": [
"--disable-extensions",
"--extensionDevelopmentPath=${workspaceRoot}",
"${workspaceRoot}/fixtures/workspaces/single/README.md"
],
"sourceMaps": true,
"outFiles": [
"${workspaceRoot}/packages/client/dist/**"
],
"smartStep": true,
"skipFiles": [
"<node_internals>/**"
]
},
{
"name": "Client: Launch Extension - notebook",
"type": "extensionHost",
Expand Down
26 changes: 18 additions & 8 deletions packages/_server/src/config/documentSettings.mts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ import type { CSpellUserSettings } from './cspellConfig/index.mjs';
import { canAddWordsToDictionary } from './customDictionaries.mjs';
import { handleSpecialUri } from './docUriHelper.mjs';
import { applyEnabledFileTypes, applyEnabledSchemes, extractEnabledFileTypes, extractEnabledSchemes } from './extractEnabledFileTypes.mjs';
import { filterUrl, toDirURL, tryJoinURL, uriToGlobPath, uriToGlobRoot, urlToFilepath } from './urlUtil.mjs';
import { filterUrl, toDirURL, toPathURL, tryJoinURL, uriToGlobPath, uriToGlobRoot, urlToFilepath } from './urlUtil.mjs';
import type { TextDocumentUri } from './vscode.config.mjs';
import { getConfiguration, getWorkspaceFolders } from './vscode.config.mjs';
import { createWorkspaceNamesResolver, resolveSettings } from './WorkspacePathResolver.mjs';
Expand Down Expand Up @@ -136,6 +136,7 @@ export class DocumentSettings {
private isTrusted: boolean | undefined;
private pIsTrusted: Promise<boolean> | undefined;
private emitterOnDidUpdateConfiguration = createEmitter<ExtSettings>();
#rootHref: string | undefined;

constructor(
readonly connection: Connection,
Expand Down Expand Up @@ -261,9 +262,9 @@ export class DocumentSettings {

async getRootUri(): Promise<Uri> {
const folders = await this.folders;
if (!folders.length) return Uri.parse(defaultRootUri);
const urls = folders.map((f) => new URL(f.uri)).sort((a, b) => a.pathname.length - b.pathname.length);
return Uri.parse(urls[0].href);
this.#rootHref = urls[0]?.href || defaultRootUri;
return Uri.parse(this.#rootHref);
}

private async _importSettings() {
Expand Down Expand Up @@ -327,8 +328,17 @@ export class DocumentSettings {
* @returns
*/
private rootSchemaAndDomainForUri(docUri: string | Uri | undefined) {
const uri = Uri.isUri(docUri) ? docUri : Uri.parse(docUri || defaultRootUri);
return uri.with({ path: '', query: null, fragment: null });
let url = toPathURL(docUri || this.#rootHref || defaultRootUri);
// Need to investigate if we should map untitled to the root.
// if (url.protocol === 'untitled:') {
// url = toPathURL(this.#rootHref || defaultRootUri);
// }
if (url.protocol === 'file:') {
url = pathToFileURL('/');
} else {
url = new URL('/', url);
}
return Uri.parse(url.href);
}

private rootSchemaAndDomainFolderForUri(docUri: string | Uri | undefined): WorkspaceFolder {
Expand Down Expand Up @@ -415,8 +425,8 @@ export class DocumentSettings {
try {
await this.determineIsTrusted();
return await this.__fetchSettingsForUri(docUri);
} catch {
// console.error('fetchSettingsForUri: %s %s', docUri, e);
} catch (_e) {
console.error('fetchSettingsForUri: %s %s', docUri, _e);
return {
uri: docUri || '',
vscodeSettings: { cSpell: {} },
Expand Down Expand Up @@ -459,9 +469,9 @@ export class DocumentSettings {
);
const settings = vscodeCSpellConfigSettingsForDocument.noConfigSearch ? undefined : await searchForConfig(useURLForConfig);
const rootFolder = this.rootSchemaAndDomainFolderForUri(docUri);
// console.log('__fetchSettingsForUri Root Folder: %o', { rootFolder, docUri });
const folder = await this.findMatchingFolder(docUri, folders[0] || rootFolder);
const globRootFolder = folder !== rootFolder ? folder : folders[0] || folder;
// console.log('__fetchSettingsForUri Folder: %o', { folder, folders, docUri, globRootFolder });

const mergedSettingsFromVSCode = mergeSettings(await this.importedSettings(), vscodeCSpellSettings);
const mergedSettings = mergeSettings(
Expand Down
58 changes: 41 additions & 17 deletions packages/_server/src/config/urlUtil.mts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,9 @@ import { URI as Uri } from 'vscode-uri';

import { stat } from '../vfs/vfs.mjs';

export async function filterUrl(uri: Uri | string | URL): Promise<Uri | undefined> {
type UrlLike = Readonly<Uri | URL | string>;

export async function filterUrl(uri: UrlLike): Promise<Uri | undefined> {
const url = uriToUrl(uri);
try {
const stats = await stat(url);
Expand All @@ -18,7 +20,7 @@ export async function filterUrl(uri: Uri | string | URL): Promise<Uri | undefine

const regExpIsUriLike = /^[a-z][\w-]+:/;

export function isUriLike(uri: string | URL | Uri): boolean {
export function isUriLike(uri: UrlLike): boolean {
if (typeof uri !== 'string') return true;
return _isUrlLike(uri) || regExpIsUriLike.test(uri);
}
Expand All @@ -32,25 +34,46 @@ export const isUrlLike = isUriLike;
* @param base - base URL
* @returns the joined path or undefined if it is not possible.
*/
export function tryJoinURL(rel: string, base: URL | string): URL | undefined {
export function tryJoinURL(rel: string, base: Readonly<URL | string>): URL | undefined {
try {
return new URL(rel, base);
} catch {
return undefined;
}
}

export function toDirURL(url: string | URL | Uri): URL {
url = uriToUrl(url);
/**
* Some URL protocols (like `untitled:`) used by VSCode do not have a path that starts with a `/`.
* This causes issues with the URL class when trying to resolve relative paths.
* This function will fix the URL so that it has a path that starts with a `/`.
* @param urlIn
* @returns URL
*/
function fixUrlRoot(urlIn: UrlLike): Readonly<URL> {
const url = uriToUrl(urlIn);
if (url.pathname.startsWith('/')) return url;
return new URL(url.protocol + '/' + url.pathname);
}

/**
* Remove the base name from a URL if one exists.
* @param url - url or url like.
* @returns URL
*/
export function toPathURL(url: UrlLike): Readonly<URL> {
url = fixUrlRoot(url);
if (url.pathname.endsWith('/')) {
return url;
}
url = url.href;
url = new URL(url);
if (!url.pathname.endsWith('/')) {
url.pathname += '/';
return new URL('.', url);
}

export function toDirURL(url: UrlLike): Readonly<URL> {
url = fixUrlRoot(url);
if (url.pathname.endsWith('/')) {
return url;
}
return url;
return new URL(url.pathname + '/', url);
}

/**
Expand All @@ -59,7 +82,7 @@ export function toDirURL(url: string | URL | Uri): URL {
* @param url - url
* @returns path
*/
export function urlToFilepath(url: string | URL | Uri): string {
export function urlToFilepath(url: UrlLike): string {
const u = uriToUrl(url);
return u.protocol === 'file:' ? normalizeWindowsRoot(fileURLToPath(u)) : u.pathname;
}
Expand All @@ -70,7 +93,7 @@ export function urlToFilepath(url: string | URL | Uri): string {
* @param url - url
* @returns path or href
*/
export function urlToFilePathOrHref(url: string | URL | Uri): string {
export function urlToFilePathOrHref(url: UrlLike): string {
const u = uriToUrl(url);
return u.protocol === 'file:' ? normalizeWindowsRoot(fileURLToPath(u)) : u.href;
}
Expand All @@ -79,7 +102,7 @@ export function normalizeWindowsRoot(path: string): string {
return path.replace(/^[a-z]:[/\\]/i, (p) => p.toUpperCase());
}

export function uriToGlobPath(uri: string | URL | Uri): string {
export function uriToGlobPath(uri: UrlLike): string {
if (typeof uri === 'string' && !isUriLike(uri)) {
// console.log(`uriToGlobPath: uri is not a URL: %s`, uri);
return uri;
Expand All @@ -89,13 +112,14 @@ export function uriToGlobPath(uri: string | URL | Uri): string {
return url.href;
}

export function uriToGlobRoot(uri: string | URL | Uri): string {
const url = toDirURL(uriToUrl(uri));
export function uriToGlobRoot(uri: UrlLike): string {
const url = uriToUrl(uri);
const urlDir = toDirURL(url);
// console.log('uriToGlobRoot:\n\t%s ->\n\t%s', uri.toString(), url.href);
return url.href;
return urlDir.href;
}

export function uriToUrl(uri: string | URL | Uri): URL {
export function uriToUrl(uri: UrlLike): Readonly<URL> {
if (uri instanceof URL) return uri;
uri = typeof uri === 'string' && !isUriLike(uri) ? pathToFileURL(uri) : uri;
const href = typeof uri === 'string' ? uri : uri.toString();
Expand Down
30 changes: 28 additions & 2 deletions packages/_server/src/config/urlUtil.test.mts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,15 @@ import { fileURLToPath } from 'url';
import { describe, expect, test } from 'vitest';
import { URI } from 'vscode-uri';

import { normalizeWindowsRoot, normalizeWindowsUrl, toDirURL, uriToUrl, urlToFilepath, urlToFilePathOrHref } from './urlUtil.mjs';
import {
normalizeWindowsRoot,
normalizeWindowsUrl,
toDirURL,
toPathURL,
uriToUrl,
urlToFilepath,
urlToFilePathOrHref,
} from './urlUtil.mjs';

describe('urlUtil', () => {
test.each`
Expand All @@ -11,6 +19,7 @@ describe('urlUtil', () => {
${URI.parse('file:///')} | ${'file:///'}
${URI.parse('file:///C:/users/home')} | ${'file:///c:/users/home'}
${'file:///C%3a/users/home/'} | ${'file:///c:/users/home/'}
${'untitled:Untitled-1'} | ${'untitled:Untitled-1'}
`('uriToUrl $uri', ({ uri, expected }) => {
const r = uriToUrl(uri);
expect(r).toBeInstanceOf(URL);
Expand All @@ -23,20 +32,37 @@ describe('urlUtil', () => {
${'file:///'} | ${'file:///'}
${'file:///C:/users/home'} | ${'file:///c:/users/home/'}
${'file:///C%3a/users/home/'} | ${'file:///c:/users/home/'}
${'output:my_output'} | ${'output:my_output'}
${'output:my_output'} | ${'output:/my_output/'}
${'untitled:Untitled-1'} | ${'untitled:/Untitled-1/'}
${'vscode-vfs://github/vitest-dev/vitest'} | ${'vscode-vfs://github/vitest-dev/vitest/'}
`('toDirURL $uri', ({ uri, expected }) => {
const r = toDirURL(uri);
expect(r).toBeInstanceOf(URL);
expect(r.href).toBe(expected);
});

test.each`
uri | expected
${'http://example.com/files'} | ${'http://example.com/'}
${'file:///'} | ${'file:///'}
${'file:///C:/users/home'} | ${'file:///c:/users/'}
${'file:///C%3a/users/home/'} | ${'file:///c:/users/home/'}
${'output:my_output'} | ${'output:/'}
${'untitled:Untitled-1'} | ${'untitled:/'}
${'vscode-vfs://github/vitest-dev/vitest'} | ${'vscode-vfs://github/vitest-dev/'}
`('toPathURL $uri', ({ uri, expected }) => {
const r = toPathURL(uri);
expect(r).toBeInstanceOf(URL);
expect(r.href).toBe(expected);
});

test.each`
uri | expected
${'http://example.com/files/'} | ${'http://example.com/files/'}
${import.meta.url} | ${fileURLToPath(import.meta.url)}
${'output:my_output'} | ${'output:my_output'}
${'vscode-vfs://github/vitest-dev/vitest'} | ${'vscode-vfs://github/vitest-dev/vitest'}
${'untitled:Untitled-1'} | ${'untitled:Untitled-1'}
`('urlToFilePathOrHref $uri', ({ uri, expected }) => {
const r = urlToFilePathOrHref(uri);
expect(r).toBe(expected);
Expand Down

0 comments on commit bf7e21d

Please sign in to comment.