-
Notifications
You must be signed in to change notification settings - Fork 1.4k
feat: Add external folder context for monorepos and full-stack workflows #1693
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from 6 commits
fe54902
cd70f75
5a42ad1
e25cdd8
5679597
cbf07b4
85aad2c
74782fb
70de36a
7026caa
b4a374a
6f75f9e
c4e4796
d84a49f
9a1d9a9
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,122 @@ | ||||||||||||||||||
| /*--------------------------------------------------------------------------------------------- | ||||||||||||||||||
| * Copyright (c) Microsoft Corporation. All rights reserved. | ||||||||||||||||||
| * Licensed under the MIT License. See License.txt in the project root for license information. | ||||||||||||||||||
| *--------------------------------------------------------------------------------------------*/ | ||||||||||||||||||
|
|
||||||||||||||||||
| import * as npath from 'path'; | ||||||||||||||||||
| import { createServiceIdentifier } from '../../../util/common/services'; | ||||||||||||||||||
| import { isEqual } from '../../../util/vs/base/common/resources'; | ||||||||||||||||||
| import { URI } from '../../../util/vs/base/common/uri'; | ||||||||||||||||||
| import { Emitter, Event } from '../../../util/vs/base/common/event'; | ||||||||||||||||||
| import { Disposable } from '../../../util/vs/base/common/lifecycle'; | ||||||||||||||||||
| import { isWindows } from '../../../util/vs/base/common/platform'; | ||||||||||||||||||
|
|
||||||||||||||||||
| const MAX_EXTERNAL_PATHS = 3; | ||||||||||||||||||
|
|
||||||||||||||||||
| export const IExternalContextService = createServiceIdentifier<IExternalContextService>('IExternalContextService'); | ||||||||||||||||||
|
|
||||||||||||||||||
| export interface IExternalContextService { | ||||||||||||||||||
| readonly _serviceBrand: undefined; | ||||||||||||||||||
| readonly onDidChangeExternalContext: Event<void>; | ||||||||||||||||||
| readonly maxExternalPaths: number; | ||||||||||||||||||
| getExternalPaths(): readonly URI[]; | ||||||||||||||||||
| addExternalPaths(paths: readonly URI[]): readonly URI[]; | ||||||||||||||||||
| replaceExternalPaths(paths: readonly URI[]): void; | ||||||||||||||||||
| removeExternalPath(path: URI): void; | ||||||||||||||||||
| clear(): void; | ||||||||||||||||||
| isExternalPath(uri: URI): boolean; | ||||||||||||||||||
| } | ||||||||||||||||||
|
|
||||||||||||||||||
| export class ExternalContextService extends Disposable implements IExternalContextService { | ||||||||||||||||||
| declare readonly _serviceBrand: undefined; | ||||||||||||||||||
|
|
||||||||||||||||||
| private readonly _onDidChangeExternalContext = this._register(new Emitter<void>()); | ||||||||||||||||||
| readonly onDidChangeExternalContext: Event<void> = this._onDidChangeExternalContext.event; | ||||||||||||||||||
|
|
||||||||||||||||||
| readonly maxExternalPaths = MAX_EXTERNAL_PATHS; | ||||||||||||||||||
|
|
||||||||||||||||||
| private readonly _paths = new Map<string, URI>(); | ||||||||||||||||||
|
|
||||||||||||||||||
| getExternalPaths(): readonly URI[] { | ||||||||||||||||||
| return [...this._paths.values()]; | ||||||||||||||||||
| } | ||||||||||||||||||
|
|
||||||||||||||||||
| addExternalPaths(paths: readonly URI[]): readonly URI[] { | ||||||||||||||||||
| const added: URI[] = []; | ||||||||||||||||||
| if (!paths.length) { | ||||||||||||||||||
| return added; | ||||||||||||||||||
| } | ||||||||||||||||||
|
|
||||||||||||||||||
| for (const path of paths) { | ||||||||||||||||||
| if (this._paths.size >= MAX_EXTERNAL_PATHS) { | ||||||||||||||||||
| break; | ||||||||||||||||||
| } | ||||||||||||||||||
| const key = path.toString(); | ||||||||||||||||||
| if (!this._paths.has(key)) { | ||||||||||||||||||
| this._paths.set(key, path); | ||||||||||||||||||
| added.push(path); | ||||||||||||||||||
| } | ||||||||||||||||||
| } | ||||||||||||||||||
| if (added.length) { | ||||||||||||||||||
| this._onDidChangeExternalContext.fire(); | ||||||||||||||||||
| } | ||||||||||||||||||
|
|
||||||||||||||||||
| return added; | ||||||||||||||||||
| } | ||||||||||||||||||
|
|
||||||||||||||||||
| replaceExternalPaths(paths: readonly URI[]): void { | ||||||||||||||||||
| this._paths.clear(); | ||||||||||||||||||
| for (const path of paths) { | ||||||||||||||||||
| this._paths.set(path.toString(), path); | ||||||||||||||||||
|
||||||||||||||||||
| this._paths.set(path.toString(), path); | |
| if (this._paths.size >= MAX_EXTERNAL_PATHS) { | |
| break; | |
| } | |
| const key = path.toString(); | |
| if (!this._paths.has(key)) { | |
| this._paths.set(key, path); | |
| } |
Copilot
AI
Oct 29, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The toComparablePath method is called repeatedly inside the loop, converting the same stored paths on every iteration. Consider caching the comparable paths in a Map alongside the URIs to avoid redundant path normalization operations.
Outdated
Copilot
AI
Oct 29, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The equality check at lines 115-117 is redundant since the same comparison is already performed by the caller at line 98 (candidateComparable === storedComparable). Remove this check to avoid duplicated logic.
| if (potentialParent === child) { | |
| return true; | |
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,53 @@ | ||
| /*--------------------------------------------------------------------------------------------- | ||
| * Copyright (c) Microsoft Corporation. All rights reserved. | ||
| * Licensed under the MIT License. See License.txt in the project root for license information. | ||
| *--------------------------------------------------------------------------------------------*/ | ||
|
|
||
| import * as path from 'path'; | ||
| import { describe, expect, it } from 'vitest'; | ||
| import { URI } from '../../../../util/vs/base/common/uri'; | ||
| import { ExternalContextService } from '../externalContextService'; | ||
|
|
||
| function createUri(name: string): URI { | ||
| return URI.file(path.join(process.cwd(), 'external-context-tests', name)); | ||
| } | ||
|
|
||
| describe('ExternalContextService', () => { | ||
| it('caps at max external paths', () => { | ||
| const service = new ExternalContextService(); | ||
|
|
||
| service.addExternalPaths([ | ||
| createUri('one'), | ||
| createUri('two'), | ||
| createUri('three'), | ||
| createUri('four') | ||
| ]); | ||
|
|
||
| expect(service.getExternalPaths()).toHaveLength(service.maxExternalPaths); | ||
| }); | ||
|
|
||
| it('fires change event when paths are added', () => { | ||
| const service = new ExternalContextService(); | ||
| let fired = 0; | ||
|
|
||
| service.onDidChangeExternalContext(() => fired++); | ||
|
|
||
| service.addExternalPaths([createUri('one')]); | ||
|
|
||
| expect(fired).toBe(1); | ||
| }); | ||
|
|
||
| it('removes paths and fires event', () => { | ||
| const service = new ExternalContextService(); | ||
| const [added] = service.addExternalPaths([createUri('one')]); | ||
| let fired = 0; | ||
|
|
||
| service.onDidChangeExternalContext(() => fired++); | ||
|
|
||
| service.removeExternalPath(added); | ||
|
|
||
| expect(service.getExternalPaths()).toHaveLength(0); | ||
| expect(fired).toBe(1); | ||
| }); | ||
| }); | ||
|
|
Uh oh!
There was an error while loading. Please reload this page.