-
Notifications
You must be signed in to change notification settings - Fork 36.9k
event handling for custom chat modes #250012
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
Changes from all commits
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 | ||
|---|---|---|---|---|
|
|
@@ -12,19 +12,21 @@ import { PromptsConfig } from '../../../../../../platform/prompts/common/config. | |||
| import { basename, dirname, joinPath } from '../../../../../../base/common/resources.js'; | ||||
| import { IWorkspaceContextService } from '../../../../../../platform/workspace/common/workspace.js'; | ||||
| import { IConfigurationService } from '../../../../../../platform/configuration/common/configuration.js'; | ||||
| import { getPromptFileExtension, getPromptFileType, PromptsType } from '../../../../../../platform/prompts/common/prompts.js'; | ||||
| import { getPromptFileExtension, getPromptFileLocationsConfigKey, getPromptFileType, PromptsType } from '../../../../../../platform/prompts/common/prompts.js'; | ||||
| import { IWorkbenchEnvironmentService } from '../../../../../services/environment/common/environmentService.js'; | ||||
| import { Schemas } from '../../../../../../base/common/network.js'; | ||||
| import { getExcludes, IFileQuery, ISearchConfiguration, ISearchService, QueryType } from '../../../../../services/search/common/search.js'; | ||||
| import { CancellationToken } from '../../../../../../base/common/cancellation.js'; | ||||
| import { isCancellationError } from '../../../../../../base/common/errors.js'; | ||||
| import { TPromptsStorage } from '../service/types.js'; | ||||
| import { IUserDataProfileService } from '../../../../../services/userDataProfile/common/userDataProfile.js'; | ||||
| import { Emitter, Event } from '../../../../../../base/common/event.js'; | ||||
| import { Disposable } from '../../../../../../base/common/lifecycle.js'; | ||||
|
|
||||
| /** | ||||
| * Utility class to locate prompt files. | ||||
| */ | ||||
| export class PromptFilesLocator { | ||||
| export class PromptFilesLocator extends Disposable { | ||||
|
|
||||
| constructor( | ||||
| @IFileService private readonly fileService: IFileService, | ||||
|
|
@@ -33,7 +35,9 @@ export class PromptFilesLocator { | |||
| @IWorkbenchEnvironmentService private readonly environmentService: IWorkbenchEnvironmentService, | ||||
| @ISearchService private readonly searchService: ISearchService, | ||||
| @IUserDataProfileService private readonly userDataService: IUserDataProfileService, | ||||
| ) { } | ||||
| ) { | ||||
| super(); | ||||
| } | ||||
|
|
||||
| /** | ||||
| * List all prompt files from the filesystem. | ||||
|
|
@@ -42,9 +46,7 @@ export class PromptFilesLocator { | |||
| */ | ||||
| public async listFiles(type: PromptsType, storage: TPromptsStorage, token: CancellationToken): Promise<readonly URI[]> { | ||||
| if (storage === 'local') { | ||||
| const configuredLocations = PromptsConfig.promptSourceFolders(this.configService, type); | ||||
| const absoluteLocations = this.toAbsoluteLocations(configuredLocations); | ||||
| return await this.findFilesInLocations(absoluteLocations, type, token); | ||||
| return await this.listFilesInLocal(type, token); | ||||
| } else { | ||||
| return await this.listFilesInUserData(type, token); | ||||
| } | ||||
|
|
@@ -55,6 +57,29 @@ export class PromptFilesLocator { | |||
| return files.filter(file => getPromptFileType(file) === type); | ||||
| } | ||||
|
|
||||
| public getFilesUpdatedEvent(type: PromptsType): Event<void> { | ||||
| const eventEmitter = this._register(new Emitter<void>()); | ||||
| const key = getPromptFileLocationsConfigKey(type); | ||||
| let parentFolders = this.getLocalParentFolders(type).map(folder => folder.parent); | ||||
| this._register(this.configService.onDidChangeConfiguration(e => { | ||||
| if (e.affectsConfiguration(key)) { | ||||
| parentFolders = this.getLocalParentFolders(type).map(folder => folder.parent); | ||||
| eventEmitter.fire(); | ||||
| } | ||||
| })); | ||||
| this._register(this.fileService.onDidFilesChange(e => { | ||||
| if (e.affects(this.userDataService.currentProfile.promptsHome)) { | ||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @aeschli this is problematic I think: the
But I would consider doing an explicit watch here to ensure the watcher is setup (multiple same watchers are folded into 1). |
||||
| eventEmitter.fire(); | ||||
| return; | ||||
| } | ||||
| if (parentFolders.some(folder => e.affects(folder))) { | ||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @aeschli I wonder how this can happen since the watching does not seem to be recursive. |
||||
| eventEmitter.fire(); | ||||
| return; | ||||
| } | ||||
| })); | ||||
| return eventEmitter.event; | ||||
| } | ||||
|
|
||||
| /** | ||||
| * Get all possible unambiguous prompt file source folders based on | ||||
| * the current workspace folder structure. | ||||
|
|
@@ -107,29 +132,19 @@ export class PromptFilesLocator { | |||
| } | ||||
|
|
||||
| /** | ||||
| * Finds all existent prompt files in the provided source folders. | ||||
| * | ||||
| * @throws if any of the provided folder paths is not an `absolute path`. | ||||
| * Finds all existent prompt files in the configured local source folders. | ||||
| * | ||||
| * @param absoluteLocations List of prompt file source folders to search for prompt files in. Must be absolute paths. | ||||
| * @returns List of prompt files found in the provided source folders. | ||||
| * @returns List of prompt files found in the local source folders. | ||||
| */ | ||||
| private async findFilesInLocations( | ||||
| absoluteLocations: readonly URI[], | ||||
| private async listFilesInLocal( | ||||
| type: PromptsType, | ||||
| token: CancellationToken | ||||
| ): Promise<readonly URI[]> { | ||||
| // find all prompt files in the provided locations, then match | ||||
| // the found file paths against (possible) glob patterns | ||||
| const paths = new ResourceSet(); | ||||
| for (const absoluteLocation of absoluteLocations) { | ||||
| assert( | ||||
| isAbsolute(absoluteLocation.path), | ||||
| `Provided location must be an absolute path, got '${absoluteLocation.path}'.`, | ||||
| ); | ||||
|
|
||||
| const { parent, filePattern } = firstNonGlobParentAndPattern(absoluteLocation); | ||||
|
|
||||
| for (const { parent, filePattern } of this.getLocalParentFolders(type)) { | ||||
| const files = (filePattern === undefined) | ||||
| ? await this.resolveFilesAtLocation(parent, token) // if the location does not contain a glob pattern, resolve the location directly | ||||
| : await this.searchFilesInLocation(parent, filePattern, token); | ||||
|
|
@@ -146,6 +161,12 @@ export class PromptFilesLocator { | |||
| return [...paths]; | ||||
| } | ||||
|
|
||||
| private getLocalParentFolders(type: PromptsType): readonly { parent: URI; filePattern?: string }[] { | ||||
| const configuredLocations = PromptsConfig.promptSourceFolders(this.configService, type); | ||||
| const absoluteLocations = this.toAbsoluteLocations(configuredLocations); | ||||
| return absoluteLocations.map(firstNonGlobParentAndPattern); | ||||
| } | ||||
|
|
||||
| /** | ||||
| * Converts locations defined in `settings` to absolute filesystem path URIs. | ||||
| * This conversion is needed because locations in settings can be relative, | ||||
|
|
@@ -330,7 +351,7 @@ export const isValidGlob = (pattern: string): boolean => { | |||
| */ | ||||
| const firstNonGlobParentAndPattern = ( | ||||
| location: URI | ||||
| ): { parent: URI; filePattern: string | undefined } => { | ||||
| ): { parent: URI; filePattern?: string } => { | ||||
| const segments = location.path.split('/'); | ||||
| let i = 0; | ||||
| while (i < segments.length && isValidGlob(segments[i]) === false) { | ||||
|
|
@@ -339,18 +360,16 @@ const firstNonGlobParentAndPattern = ( | |||
| if (i === segments.length) { | ||||
| // the path does not contain a glob pattern, so we can | ||||
| // just find all prompt files in the provided location | ||||
| return { parent: location, filePattern: undefined }; | ||||
| return { parent: location }; | ||||
| } | ||||
| const parent = location.with({ path: segments.slice(0, i).join('/') }); | ||||
| if (i === segments.length - 1 && segments[i] === '*' || segments[i] === ``) { | ||||
| return { | ||||
| parent: location.with({ path: segments.slice(0, i).join('/') }), | ||||
| filePattern: undefined | ||||
| }; | ||||
| return { parent }; | ||||
| } | ||||
|
|
||||
| // the path contains a glob pattern, so we search in last folder that does not contain a glob pattern | ||||
| return { | ||||
| parent: location.with({ path: segments.slice(0, i).join('/') }), | ||||
| parent, | ||||
| filePattern: segments.slice(i).join('/') | ||||
| }; | ||||
| }; | ||||
|
|
||||
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.
@aeschli I find this a bit of a weird pattern to return an event from a method: esp. since you register disposables to the
PromptFilesLocator, the lifecycle seems bound by "the big guy" and not the one calling it. This means you pile up listeners for as often as the method is called, only disposing them when "the big guy" goes away (likely never).I would rather do 1 event that is fixed for the
PromptsTypeto follow what we do in other places.