From 1b222afc94c9316eb64b0176548cef5345a6809f Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Tue, 18 Aug 2020 09:54:49 -0600 Subject: [PATCH 1/8] Add a basic env watcher. --- src/client/pythonEnvironments/base/watcher.ts | 98 +++++++++++ .../base/watcher.unit.test.ts | 163 ++++++++++++++++++ 2 files changed, 261 insertions(+) create mode 100644 src/client/pythonEnvironments/base/watcher.ts create mode 100644 src/test/pythonEnvironments/base/watcher.unit.test.ts diff --git a/src/client/pythonEnvironments/base/watcher.ts b/src/client/pythonEnvironments/base/watcher.ts new file mode 100644 index 000000000000..57711464af5e --- /dev/null +++ b/src/client/pythonEnvironments/base/watcher.ts @@ -0,0 +1,98 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +// tslint:disable:max-classes-per-file + +import { Event, EventEmitter, Uri } from 'vscode'; +import { PythonEnvKind } from './info'; + +/** + * The most basic info for a Python environments event. + * + * @prop kind - the env kind, if any, affected by the event + */ +export type BasicPythonEnvsChangedEvent = { + kind?: PythonEnvKind; +}; + +/** + * The full set of possible info for a Python environments event. + * + * @prop searchLocation - the location, if any, affected by the event + */ +export type PythonEnvsChangedEvent = BasicPythonEnvsChangedEvent & { + searchLocation?: Uri; +}; + +/** + * A "watcher" for events related to changes to Python environemts. + * + * The watcher will notify listeners (callbacks registered through + * `onChanged`) of events at undetermined times. The actual emitted + * events, their source, and the timing is entirely up to the watcher + * implementation. + */ +export interface IPythonEnvsWatcher { + /** + * The hook for registering event listeners (callbacks). + */ + readonly onChanged: Event; +} + +/** + * This provides the fundamental functionality of a watcher for any event type. + * + * Consumers register listeners (callbacks) using `onChanged`. Each + * listener is invoked when `fire()` is called. + * + * Note that in most cases classes will not inherit from this classes, + * but instead keep a private watcher property. The rule of thumb + * is to follow whether or not consumers of *that* class should be able + * to trigger events (via `fire()`). + */ +class WatcherBase { + /** + * The hook for registering event listeners (callbacks). + */ + public readonly onChanged: Event; + private readonly didChange = new EventEmitter(); + + constructor() { + this.onChanged = this.didChange.event; + } + + /** + * Send the event to all registered listeners. + */ + public fire(event: T) { + this.didChange.fire(event); + } +} + +/** + * A watcher for the basic Python environments events. + * + * The same rule-of-thumb for subclassing `WatcherBase` applies here. + */ +export class BasicPythonEnvsWatcher extends WatcherBase { + /** + * Fire an event based on the given info. + */ + public trigger(kind?: PythonEnvKind) { + this.fire({ kind }); + } +} + +/** + * A general-use watcher for Python environments events. + * + * The same rule-of-thumb for subclassing `WatcherBase` applies here. + */ +export class PythonEnvsWatcher extends WatcherBase { + /** + * Fire an event based on the given info. + */ + public trigger(kind?: PythonEnvKind, searchLocation?: Uri) { + this.fire({ kind, searchLocation }); + } +} diff --git a/src/test/pythonEnvironments/base/watcher.unit.test.ts b/src/test/pythonEnvironments/base/watcher.unit.test.ts new file mode 100644 index 000000000000..90d1b3959a9d --- /dev/null +++ b/src/test/pythonEnvironments/base/watcher.unit.test.ts @@ -0,0 +1,163 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +import * as assert from 'assert'; +import { Uri } from 'vscode'; +import { PythonEnvKind } from '../../../client/pythonEnvironments/base/info'; +import { + BasicPythonEnvsChangedEvent, + BasicPythonEnvsWatcher, + PythonEnvsChangedEvent, + PythonEnvsWatcher +} from '../../../client/pythonEnvironments/base/watcher'; + +const KINDS_TO_TEST = [ + PythonEnvKind.Unknown, + PythonEnvKind.System, + PythonEnvKind.Custom, + PythonEnvKind.OtherGlobal, + PythonEnvKind.Venv, + PythonEnvKind.Conda, + PythonEnvKind.OtherVirtual +]; + +suite('pyenvs watcher - BasicPythonEnvsWatcher', () => { + suite('fire()', () => { + test('empty event', () => { + const expected: BasicPythonEnvsChangedEvent = {}; + const watcher = new BasicPythonEnvsWatcher(); + let event: BasicPythonEnvsChangedEvent | undefined; + watcher.onChanged((e) => { + event = e; + }); + + watcher.fire(expected); + + assert.equal(event, expected); + }); + + KINDS_TO_TEST.forEach((kind) => { + test(`non-empty event ("${kind}")`, () => { + const expected: BasicPythonEnvsChangedEvent = { + kind: kind + }; + const watcher = new BasicPythonEnvsWatcher(); + let event: BasicPythonEnvsChangedEvent | undefined; + watcher.onChanged((e) => { + event = e; + }); + + watcher.fire(expected); + + assert.equal(event, expected); + }); + }); + }); + + suite('trigger()', () => { + test('empty event', () => { + const expected: BasicPythonEnvsChangedEvent = { + kind: undefined + }; + const watcher = new BasicPythonEnvsWatcher(); + let event: BasicPythonEnvsChangedEvent | undefined; + watcher.onChanged((e) => { + event = e; + }); + + watcher.trigger(); + + assert.deepEqual(event, expected); + }); + + KINDS_TO_TEST.forEach((kind) => { + test(`non-empty event ("${kind}")`, () => { + const expected: BasicPythonEnvsChangedEvent = { + kind: kind + }; + const watcher = new BasicPythonEnvsWatcher(); + let event: BasicPythonEnvsChangedEvent | undefined; + watcher.onChanged((e) => { + event = e; + }); + + watcher.trigger(kind); + + assert.deepEqual(event, expected); + }); + }); + }); +}); + +suite('pyenvs watcher - PythonEnvsWatcher', () => { + const location = Uri.file('some-dir'); + + suite('fire()', () => { + test('empty event', () => { + const expected: PythonEnvsChangedEvent = {}; + const watcher = new PythonEnvsWatcher(); + let event: PythonEnvsChangedEvent | undefined; + watcher.onChanged((e) => { + event = e; + }); + + watcher.fire(expected); + + assert.equal(event, expected); + }); + + KINDS_TO_TEST.forEach((kind) => { + test(`non-empty event ("${kind}")`, () => { + const expected: PythonEnvsChangedEvent = { + kind: kind, + searchLocation: location + }; + const watcher = new PythonEnvsWatcher(); + let event: PythonEnvsChangedEvent | undefined; + watcher.onChanged((e) => { + event = e; + }); + + watcher.fire(expected); + + assert.equal(event, expected); + }); + }); + }); + + suite('trigger()', () => { + test('empty event', () => { + const expected: PythonEnvsChangedEvent = { + kind: undefined, + searchLocation: undefined + }; + const watcher = new PythonEnvsWatcher(); + let event: PythonEnvsChangedEvent | undefined; + watcher.onChanged((e) => { + event = e; + }); + + watcher.trigger(); + + assert.deepEqual(event, expected); + }); + + KINDS_TO_TEST.forEach((kind) => { + test(`non-empty event ("${kind}")`, () => { + const expected: PythonEnvsChangedEvent = { + kind: kind, + searchLocation: location + }; + const watcher = new PythonEnvsWatcher(); + let event: PythonEnvsChangedEvent | undefined; + watcher.onChanged((e) => { + event = e; + }); + + watcher.trigger(kind, location); + + assert.deepEqual(event, expected); + }); + }); + }); +}); From 4a9c7a69dec275805c20b081e326828b5fa02f87 Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Wed, 26 Aug 2020 11:45:22 -0600 Subject: [PATCH 2/8] Add a basic collection of env watchers. --- .../pythonEnvironments/base/watchers.ts | 23 ++++++++ .../base/watchers.unit.test.ts | 55 +++++++++++++++++++ 2 files changed, 78 insertions(+) create mode 100644 src/client/pythonEnvironments/base/watchers.ts create mode 100644 src/test/pythonEnvironments/base/watchers.unit.test.ts diff --git a/src/client/pythonEnvironments/base/watchers.ts b/src/client/pythonEnvironments/base/watchers.ts new file mode 100644 index 000000000000..3f96e52eea21 --- /dev/null +++ b/src/client/pythonEnvironments/base/watchers.ts @@ -0,0 +1,23 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +import { Event } from 'vscode'; +import { IPythonEnvsWatcher, PythonEnvsChangedEvent, PythonEnvsWatcher } from './watcher'; + +/** + * A wrapper around a set of watchers. + * + * If any of the wrapped watchers emits an event then this wrapper + * emits that event. + */ +export class PythonEnvsWatchers { + public readonly onChanged: Event; + private watcher = new PythonEnvsWatcher(); + + constructor(watchers: ReadonlyArray) { + this.onChanged = this.watcher.onChanged; + watchers.forEach((w) => { + w.onChanged((e) => this.watcher.fire(e)); + }); + } +} diff --git a/src/test/pythonEnvironments/base/watchers.unit.test.ts b/src/test/pythonEnvironments/base/watchers.unit.test.ts new file mode 100644 index 000000000000..f5338ff296f9 --- /dev/null +++ b/src/test/pythonEnvironments/base/watchers.unit.test.ts @@ -0,0 +1,55 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +import * as assert from 'assert'; +import { Uri } from 'vscode'; +import { PythonEnvKind } from '../../../client/pythonEnvironments/base/info'; +import { PythonEnvsChangedEvent, PythonEnvsWatcher } from '../../../client/pythonEnvironments/base/watcher'; +import { PythonEnvsWatchers } from '../../../client/pythonEnvironments/base/watchers'; + +suite('pyenvs watchers - PythonEnvsWatchers', () => { + suite('onChanged consolidates', () => { + test('empty', () => { + const watcher = new PythonEnvsWatchers([]); + + assert.ok(watcher); + }); + + test('one', () => { + const event1: PythonEnvsChangedEvent = {}; + const expected = [event1]; + const sub1 = new PythonEnvsWatcher(); + const watcher = new PythonEnvsWatchers([sub1]); + + const events: PythonEnvsChangedEvent[] = []; + watcher.onChanged((e) => events.push(e)); + sub1.fire(event1); + + assert.deepEqual(events, expected); + }); + + test('many', () => { + const loc1 = Uri.file('some-dir'); + const event1: PythonEnvsChangedEvent = { kind: PythonEnvKind.Unknown, searchLocation: loc1 }; + const event2: PythonEnvsChangedEvent = { kind: PythonEnvKind.Venv }; + const event3: PythonEnvsChangedEvent = {}; + const event4: PythonEnvsChangedEvent = { searchLocation: loc1 }; + const event5: PythonEnvsChangedEvent = {}; + const expected = [event1, event2, event3, event4, event5]; + const sub1 = new PythonEnvsWatcher(); + const sub2 = new PythonEnvsWatcher(); + const sub3 = new PythonEnvsWatcher(); + const watcher = new PythonEnvsWatchers([sub1, sub2, sub3]); + + const events: PythonEnvsChangedEvent[] = []; + watcher.onChanged((e) => events.push(e)); + sub2.fire(event1); + sub3.fire(event2); + sub1.fire(event3); + sub2.fire(event4); + sub1.fire(event5); + + assert.deepEqual(events, expected); + }); + }); +}); From 7f3c22e3b14d46cc2b5c32cc40bf02c9875f0d75 Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Wed, 26 Aug 2020 14:36:54 -0600 Subject: [PATCH 3/8] Add a basic "disableable" class. --- src/client/common/utils/misc.ts | 28 +++++++++++++ src/test/common/utils/misc.unit.test.ts | 53 +++++++++++++++++++++++++ 2 files changed, 81 insertions(+) create mode 100644 src/test/common/utils/misc.unit.test.ts diff --git a/src/client/common/utils/misc.ts b/src/client/common/utils/misc.ts index 8c6783b78c9f..ceb71be61b26 100644 --- a/src/client/common/utils/misc.ts +++ b/src/client/common/utils/misc.ts @@ -55,6 +55,34 @@ type DeepReadonlyObject = { }; type NonFunctionPropertyNames = { [K in keyof T]: T[K] extends Function ? never : K }[keyof T]; +/** + * An object that can be disabled. + */ +export class Disableable { + private enabled = true; + + /** + * True if the watcher is currently enabled. + */ + public get isEnabled(): boolean { + return this.enabled; + } + + /** + * Ensure that the watcher is enabled. + */ + public enable() { + this.enabled = true; + } + + /** + * Ensure that the watcher is disabled. + */ + public disable() { + this.enabled = false; + } +} + // Information about a traced function/method call. export type TraceInfo = { elapsed: number; // milliseconds diff --git a/src/test/common/utils/misc.unit.test.ts b/src/test/common/utils/misc.unit.test.ts new file mode 100644 index 000000000000..cf13b448578e --- /dev/null +++ b/src/test/common/utils/misc.unit.test.ts @@ -0,0 +1,53 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +import * as assert from 'assert'; +import { Disableable } from '../../../client/common/utils/misc'; + +suite('common utils - Disableable', () => { + test('initial state', () => { + const obj = new Disableable(); + + const current = obj.isEnabled; + + assert.ok(current); + }); + + test('enable - currently disabled', () => { + const obj = new Disableable(); + obj.disable(); + + obj.enable(); + const current = obj.isEnabled; + + assert.ok(current); + }); + + test('enable - currently enabled', () => { + const obj = new Disableable(); + + obj.enable(); + const current = obj.isEnabled; + + assert.ok(current); + }); + + test('disable - currently enabled', () => { + const obj = new Disableable(); + + obj.disable(); + const current = obj.isEnabled; + + assert.equal(current, false); + }); + + test('disable - currently disabled', () => { + const obj = new Disableable(); + obj.disable(); + + obj.disable(); + const current = obj.isEnabled; + + assert.equal(current, false); + }); +}); From 0a10ea5c1f45f8a610e9baa6a229ad807b5eb625 Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Mon, 24 Aug 2020 15:24:56 -0600 Subject: [PATCH 4/8] Add a disableable watcher wrapper. --- .../pythonEnvironments/base/watchers.ts | 33 +++++++++- .../base/watchers.unit.test.ts | 65 ++++++++++++++++++- 2 files changed, 96 insertions(+), 2 deletions(-) diff --git a/src/client/pythonEnvironments/base/watchers.ts b/src/client/pythonEnvironments/base/watchers.ts index 3f96e52eea21..b4ea95112ddc 100644 --- a/src/client/pythonEnvironments/base/watchers.ts +++ b/src/client/pythonEnvironments/base/watchers.ts @@ -1,7 +1,8 @@ // Copyright (c) Microsoft Corporation. All rights reserved. // Licensed under the MIT License. -import { Event } from 'vscode'; +import { Disposable, Event } from 'vscode'; +import { Disableable } from '../../common/utils/misc'; import { IPythonEnvsWatcher, PythonEnvsChangedEvent, PythonEnvsWatcher } from './watcher'; /** @@ -21,3 +22,33 @@ export class PythonEnvsWatchers { }); } } + +// tslint:disable-next-line:no-any +type EnvsEventListener = (e: PythonEnvsChangedEvent) => any; + +/** + * A watcher wrapper that can be disabled. + * + * If disabled, events emitted by the wrapped watcher are discarded. + */ +export class DisableableEnvsWatcher extends Disableable { + constructor( + // To wrap more than one use `PythonEnvWatchers`. + private readonly wrapped: IPythonEnvsWatcher + ) { + super(); + } + + // tslint:disable-next-line:no-any + public onChanged(listener: EnvsEventListener, thisArgs?: any, disposables?: Disposable[]): Disposable { + return this.wrapped.onChanged( + (e: PythonEnvsChangedEvent) => { + if (this.isEnabled) { + listener(e); + } + }, + thisArgs, + disposables + ); + } +} diff --git a/src/test/pythonEnvironments/base/watchers.unit.test.ts b/src/test/pythonEnvironments/base/watchers.unit.test.ts index f5338ff296f9..c39d61d31e28 100644 --- a/src/test/pythonEnvironments/base/watchers.unit.test.ts +++ b/src/test/pythonEnvironments/base/watchers.unit.test.ts @@ -5,7 +5,7 @@ import * as assert from 'assert'; import { Uri } from 'vscode'; import { PythonEnvKind } from '../../../client/pythonEnvironments/base/info'; import { PythonEnvsChangedEvent, PythonEnvsWatcher } from '../../../client/pythonEnvironments/base/watcher'; -import { PythonEnvsWatchers } from '../../../client/pythonEnvironments/base/watchers'; +import { DisableableEnvsWatcher, PythonEnvsWatchers } from '../../../client/pythonEnvironments/base/watchers'; suite('pyenvs watchers - PythonEnvsWatchers', () => { suite('onChanged consolidates', () => { @@ -53,3 +53,66 @@ suite('pyenvs watchers - PythonEnvsWatchers', () => { }); }); }); + +suite('pyenvs watchers - DisableableEnvsWatcher', () => { + test('enabled by default', () => { + const sub = new PythonEnvsWatcher(); + const watcher = new DisableableEnvsWatcher(sub); + + const enabled = watcher.isEnabled; + + assert.ok(enabled); + }); + + suite('onChanged', () => { + test('fires if enabled', () => { + const event1: PythonEnvsChangedEvent = {}; + const event2: PythonEnvsChangedEvent = {}; + const expected = [event1, event2]; + const sub = new PythonEnvsWatcher(); + const watcher = new DisableableEnvsWatcher(sub); + const events: PythonEnvsChangedEvent[] = []; + watcher.onChanged((e) => events.push(e)); + + sub.fire(event1); + sub.fire(event2); + + assert.deepEqual(events, expected); + }); + + test('does not fire if disabled', () => { + const event1: PythonEnvsChangedEvent = {}; + const event2: PythonEnvsChangedEvent = {}; + const expected: PythonEnvsChangedEvent[] = []; + const sub = new PythonEnvsWatcher(); + const watcher = new DisableableEnvsWatcher(sub); + const events: PythonEnvsChangedEvent[] = []; + watcher.onChanged((e) => events.push(e)); + + watcher.disable(); + sub.fire(event1); + sub.fire(event2); + + assert.deepEqual(events, expected); + }); + + test('follows enabled state', () => { + const event1: PythonEnvsChangedEvent = {}; + const event2: PythonEnvsChangedEvent = { kind: PythonEnvKind.Unknown }; + const event3: PythonEnvsChangedEvent = { kind: PythonEnvKind.Venv }; + const expected = [event1, event3]; + const sub = new PythonEnvsWatcher(); + const watcher = new DisableableEnvsWatcher(sub); + const events: PythonEnvsChangedEvent[] = []; + watcher.onChanged((e) => events.push(e)); + + sub.fire(event1); + watcher.disable(); + sub.fire(event2); + watcher.enable(); + sub.fire(event3); + + assert.deepEqual(events, expected); + }); + }); +}); From b5d60b76e05ebd9ed24c12e93b629f5a2bb1b4d1 Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Wed, 2 Sep 2020 10:18:19 -0600 Subject: [PATCH 5/8] Drop Disableable. --- src/client/common/utils/misc.ts | 28 ---------- .../pythonEnvironments/base/watchers.ts | 22 ++++++-- src/test/common/utils/misc.unit.test.ts | 53 ------------------- .../base/watchers.unit.test.ts | 10 +++- 4 files changed, 25 insertions(+), 88 deletions(-) delete mode 100644 src/test/common/utils/misc.unit.test.ts diff --git a/src/client/common/utils/misc.ts b/src/client/common/utils/misc.ts index ceb71be61b26..8c6783b78c9f 100644 --- a/src/client/common/utils/misc.ts +++ b/src/client/common/utils/misc.ts @@ -55,34 +55,6 @@ type DeepReadonlyObject = { }; type NonFunctionPropertyNames = { [K in keyof T]: T[K] extends Function ? never : K }[keyof T]; -/** - * An object that can be disabled. - */ -export class Disableable { - private enabled = true; - - /** - * True if the watcher is currently enabled. - */ - public get isEnabled(): boolean { - return this.enabled; - } - - /** - * Ensure that the watcher is enabled. - */ - public enable() { - this.enabled = true; - } - - /** - * Ensure that the watcher is disabled. - */ - public disable() { - this.enabled = false; - } -} - // Information about a traced function/method call. export type TraceInfo = { elapsed: number; // milliseconds diff --git a/src/client/pythonEnvironments/base/watchers.ts b/src/client/pythonEnvironments/base/watchers.ts index b4ea95112ddc..f1b1d8d4b1ee 100644 --- a/src/client/pythonEnvironments/base/watchers.ts +++ b/src/client/pythonEnvironments/base/watchers.ts @@ -2,7 +2,6 @@ // Licensed under the MIT License. import { Disposable, Event } from 'vscode'; -import { Disableable } from '../../common/utils/misc'; import { IPythonEnvsWatcher, PythonEnvsChangedEvent, PythonEnvsWatcher } from './watcher'; /** @@ -31,19 +30,32 @@ type EnvsEventListener = (e: PythonEnvsChangedEvent) => any; * * If disabled, events emitted by the wrapped watcher are discarded. */ -export class DisableableEnvsWatcher extends Disableable { +export class DisableableEnvsWatcher { + private enabled = true; constructor( // To wrap more than one use `PythonEnvWatchers`. private readonly wrapped: IPythonEnvsWatcher - ) { - super(); + ) {} + + /** + * Ensure that the watcher is enabled. + */ + public enable() { + this.enabled = true; + } + + /** + * Ensure that the watcher is disabled. + */ + public disable() { + this.enabled = false; } // tslint:disable-next-line:no-any public onChanged(listener: EnvsEventListener, thisArgs?: any, disposables?: Disposable[]): Disposable { return this.wrapped.onChanged( (e: PythonEnvsChangedEvent) => { - if (this.isEnabled) { + if (this.enabled) { listener(e); } }, diff --git a/src/test/common/utils/misc.unit.test.ts b/src/test/common/utils/misc.unit.test.ts deleted file mode 100644 index cf13b448578e..000000000000 --- a/src/test/common/utils/misc.unit.test.ts +++ /dev/null @@ -1,53 +0,0 @@ -// Copyright (c) Microsoft Corporation. All rights reserved. -// Licensed under the MIT License. - -import * as assert from 'assert'; -import { Disableable } from '../../../client/common/utils/misc'; - -suite('common utils - Disableable', () => { - test('initial state', () => { - const obj = new Disableable(); - - const current = obj.isEnabled; - - assert.ok(current); - }); - - test('enable - currently disabled', () => { - const obj = new Disableable(); - obj.disable(); - - obj.enable(); - const current = obj.isEnabled; - - assert.ok(current); - }); - - test('enable - currently enabled', () => { - const obj = new Disableable(); - - obj.enable(); - const current = obj.isEnabled; - - assert.ok(current); - }); - - test('disable - currently enabled', () => { - const obj = new Disableable(); - - obj.disable(); - const current = obj.isEnabled; - - assert.equal(current, false); - }); - - test('disable - currently disabled', () => { - const obj = new Disableable(); - obj.disable(); - - obj.disable(); - const current = obj.isEnabled; - - assert.equal(current, false); - }); -}); diff --git a/src/test/pythonEnvironments/base/watchers.unit.test.ts b/src/test/pythonEnvironments/base/watchers.unit.test.ts index c39d61d31e28..58e5394edcba 100644 --- a/src/test/pythonEnvironments/base/watchers.unit.test.ts +++ b/src/test/pythonEnvironments/base/watchers.unit.test.ts @@ -56,12 +56,16 @@ suite('pyenvs watchers - PythonEnvsWatchers', () => { suite('pyenvs watchers - DisableableEnvsWatcher', () => { test('enabled by default', () => { + const event1: PythonEnvsChangedEvent = {}; + const expected = [event1]; const sub = new PythonEnvsWatcher(); const watcher = new DisableableEnvsWatcher(sub); + const events: PythonEnvsChangedEvent[] = []; + watcher.onChanged((e) => events.push(e)); - const enabled = watcher.isEnabled; + sub.fire(event1); - assert.ok(enabled); + assert.deepEqual(events, expected); }); suite('onChanged', () => { @@ -74,6 +78,7 @@ suite('pyenvs watchers - DisableableEnvsWatcher', () => { const events: PythonEnvsChangedEvent[] = []; watcher.onChanged((e) => events.push(e)); + watcher.enable(); sub.fire(event1); sub.fire(event2); @@ -106,6 +111,7 @@ suite('pyenvs watchers - DisableableEnvsWatcher', () => { const events: PythonEnvsChangedEvent[] = []; watcher.onChanged((e) => events.push(e)); + watcher.enable(); sub.fire(event1); watcher.disable(); sub.fire(event2); From 69d7fe63dba638d97aa3e21d30287b0c81e370a2 Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Wed, 2 Sep 2020 13:22:02 -0600 Subject: [PATCH 6/8] Use unknown instead of any. --- src/client/pythonEnvironments/base/watchers.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/client/pythonEnvironments/base/watchers.ts b/src/client/pythonEnvironments/base/watchers.ts index f1b1d8d4b1ee..0c59c98b7afb 100644 --- a/src/client/pythonEnvironments/base/watchers.ts +++ b/src/client/pythonEnvironments/base/watchers.ts @@ -22,8 +22,8 @@ export class PythonEnvsWatchers { } } -// tslint:disable-next-line:no-any -type EnvsEventListener = (e: PythonEnvsChangedEvent) => any; +// This matches the `vscode.Event` arg. +type EnvsEventListener = (e: PythonEnvsChangedEvent) => unknown; /** * A watcher wrapper that can be disabled. @@ -51,8 +51,8 @@ export class DisableableEnvsWatcher { this.enabled = false; } - // tslint:disable-next-line:no-any - public onChanged(listener: EnvsEventListener, thisArgs?: any, disposables?: Disposable[]): Disposable { + // This matches the signature of `vscode.Event`. + public onChanged(listener: EnvsEventListener, thisArgs?: unknown, disposables?: Disposable[]): Disposable { return this.wrapped.onChanged( (e: PythonEnvsChangedEvent) => { if (this.enabled) { From df16bf16518f7f2e6886b41b5c3e9fbc291a8e16 Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Wed, 2 Sep 2020 13:39:51 -0600 Subject: [PATCH 7/8] Clarify about BasicPythonEnvsWatcher and about implementations of IPythonEnvsWatcher. --- src/client/pythonEnvironments/base/watcher.ts | 28 +++++++++++++++++-- .../pythonEnvironments/base/watchers.ts | 6 +++- 2 files changed, 31 insertions(+), 3 deletions(-) diff --git a/src/client/pythonEnvironments/base/watcher.ts b/src/client/pythonEnvironments/base/watcher.ts index 57711464af5e..128236b45be1 100644 --- a/src/client/pythonEnvironments/base/watcher.ts +++ b/src/client/pythonEnvironments/base/watcher.ts @@ -49,6 +49,8 @@ export interface IPythonEnvsWatcher { /** @@ -69,10 +71,24 @@ class WatcherBase { } } +// The use cases for BasicPythonEnvsWatcher are currently hypothetical. +// However, there's a real chance they may prove useful for the concrete +// locators. Adding BasicPythonEnvsWatcher later will be much harder +// than removing it later, so we're leaving it for now. + /** * A watcher for the basic Python environments events. * - * The same rule-of-thumb for subclassing `WatcherBase` applies here. + * This should be used only in low-level cases, with the most + * rudimentary watchers. Most of the time `PythonEnvsWatcher` + * should be used instead. + * + * Note that in most cases classes will not inherit from this classes, + * but instead keep a private watcher property. The rule of thumb + * is to follow whether or not consumers of *that* class should be able + * to trigger events (via `fire()`). + * + * @implements IPythonEnvsWatcher */ export class BasicPythonEnvsWatcher extends WatcherBase { /** @@ -86,7 +102,15 @@ export class BasicPythonEnvsWatcher extends WatcherBase { /** diff --git a/src/client/pythonEnvironments/base/watchers.ts b/src/client/pythonEnvironments/base/watchers.ts index 0c59c98b7afb..62403facf316 100644 --- a/src/client/pythonEnvironments/base/watchers.ts +++ b/src/client/pythonEnvironments/base/watchers.ts @@ -5,10 +5,12 @@ import { Disposable, Event } from 'vscode'; import { IPythonEnvsWatcher, PythonEnvsChangedEvent, PythonEnvsWatcher } from './watcher'; /** - * A wrapper around a set of watchers. + * A wrapper around a set of watchers, exposing them as a single watcher. * * If any of the wrapped watchers emits an event then this wrapper * emits that event. + * + * @implements IPythonEnvsWatcher */ export class PythonEnvsWatchers { public readonly onChanged: Event; @@ -29,6 +31,8 @@ type EnvsEventListener = (e: PythonEnvsChangedEvent) => unknown; * A watcher wrapper that can be disabled. * * If disabled, events emitted by the wrapped watcher are discarded. + * + * @implements IPythonEnvsWatcher */ export class DisableableEnvsWatcher { private enabled = true; From 077c6e09358e5d10559c4610fa869b97fe4c7b7f Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Wed, 2 Sep 2020 14:19:42 -0600 Subject: [PATCH 8/8] Add "implements IPythonEnvsWatcher" where appropriate (for the sake of code navigation). --- src/client/pythonEnvironments/base/watcher.ts | 8 +------- src/client/pythonEnvironments/base/watchers.ts | 8 ++------ 2 files changed, 3 insertions(+), 13 deletions(-) diff --git a/src/client/pythonEnvironments/base/watcher.ts b/src/client/pythonEnvironments/base/watcher.ts index 128236b45be1..52c7212862d6 100644 --- a/src/client/pythonEnvironments/base/watcher.ts +++ b/src/client/pythonEnvironments/base/watcher.ts @@ -49,10 +49,8 @@ export interface IPythonEnvsWatcher { +class WatcherBase implements IPythonEnvsWatcher { /** * The hook for registering event listeners (callbacks). */ @@ -87,8 +85,6 @@ class WatcherBase { * but instead keep a private watcher property. The rule of thumb * is to follow whether or not consumers of *that* class should be able * to trigger events (via `fire()`). - * - * @implements IPythonEnvsWatcher */ export class BasicPythonEnvsWatcher extends WatcherBase { /** @@ -109,8 +105,6 @@ export class BasicPythonEnvsWatcher extends WatcherBase { /** diff --git a/src/client/pythonEnvironments/base/watchers.ts b/src/client/pythonEnvironments/base/watchers.ts index 62403facf316..9cda8ca4c89c 100644 --- a/src/client/pythonEnvironments/base/watchers.ts +++ b/src/client/pythonEnvironments/base/watchers.ts @@ -9,10 +9,8 @@ import { IPythonEnvsWatcher, PythonEnvsChangedEvent, PythonEnvsWatcher } from '. * * If any of the wrapped watchers emits an event then this wrapper * emits that event. - * - * @implements IPythonEnvsWatcher */ -export class PythonEnvsWatchers { +export class PythonEnvsWatchers implements IPythonEnvsWatcher { public readonly onChanged: Event; private watcher = new PythonEnvsWatcher(); @@ -31,10 +29,8 @@ type EnvsEventListener = (e: PythonEnvsChangedEvent) => unknown; * A watcher wrapper that can be disabled. * * If disabled, events emitted by the wrapped watcher are discarded. - * - * @implements IPythonEnvsWatcher */ -export class DisableableEnvsWatcher { +export class DisableableEnvsWatcher implements IPythonEnvsWatcher { private enabled = true; constructor( // To wrap more than one use `PythonEnvWatchers`.