refactor: remove unneeded Component, use visibleTextDocuments directly

This suffices for a while.

BREAKING CHANGE: The environment component has been removed; now you need to specify the visible text documents directly.

Author: sqs

src/client/api/documents.ts

@@ -1,10 +1,8 @@
 import { Observable, Subscription } from 'rxjs'
-import { filter } from 'rxjs/operators'
 import { TextDocument } from 'sourcegraph'
 import { createProxyAndHandleRequests } from '../../common/proxy'
 import { ExtDocumentsAPI } from '../../extension/api/documents'
 import { Connection } from '../../protocol/jsonrpc2/connection'
-import { TextDocumentItem } from '../types/textDocument'
 import { SubscriptionMap } from './common'
 
 /** @internal */
@@ -15,13 +13,13 @@ export class ClientDocuments {
 
     constructor(
         connection: Connection,
-        environmentTextDocument: Observable<Pick<TextDocument, 'uri' | 'languageId'> | null>
+        environmentTextDocuments: Observable<Pick<TextDocument, 'uri' | 'languageId' | 'text'>[] | null>
     ) {
         this.proxy = createProxyAndHandleRequests('documents', connection, this)
 
         this.subscriptions.add(
-            environmentTextDocument.pipe(filter((v): v is TextDocumentItem => v !== null)).subscribe(doc => {
-                this.proxy.$acceptDocumentData(doc)
+            environmentTextDocuments.subscribe(docs => {
+                this.proxy.$acceptDocumentData(docs || [])
             })
         )
 

src/client/api/windows.ts

@@ -1,5 +1,4 @@
 import { Observable, Subscription } from 'rxjs'
-import { filter } from 'rxjs/operators'
 import * as sourcegraph from 'sourcegraph'
 import { createProxyAndHandleRequests } from '../../common/proxy'
 import { ExtWindowsAPI } from '../../extension/api/windows'
@@ -11,7 +10,7 @@ import {
     ShowMessageRequestParams,
 } from '../../protocol'
 import { Connection } from '../../protocol/jsonrpc2/connection'
-import { Component } from '../environment'
+import { TextDocumentItem } from '../types/textDocument'
 import { SubscriptionMap } from './common'
 
 /** @internal */
@@ -29,7 +28,7 @@ export class ClientWindows implements ClientWindowsAPI {
 
     constructor(
         connection: Connection,
-        environmentComponent: Observable<Component | null>,
+        environmentTextDocuments: Observable<TextDocumentItem[] | null>,
         /** Called when the client receives a window/showMessage notification. */
         private showMessage: (params: ShowMessageParams) => void,
         /**
@@ -46,8 +45,10 @@ export class ClientWindows implements ClientWindowsAPI {
         this.proxy = createProxyAndHandleRequests('windows', connection, this)
 
         this.subscriptions.add(
-            environmentComponent.pipe(filter((v): v is Component => v !== null)).subscribe(component => {
-                this.proxy.$acceptWindowData(component ? [{ visibleTextDocument: component.document.uri }] : [])
+            environmentTextDocuments.subscribe(textDocuments => {
+                this.proxy.$acceptWindowData(
+                    textDocuments ? textDocuments.map(textDocument => ({ visibleTextDocument: textDocument.uri })) : []
+                )
             })
         )
 

src/client/context/context.test.ts

@@ -33,13 +33,13 @@ describe('getComputedContextProperty', () => {
     describe('environment with component', () => {
         const env: Environment = {
             ...EMPTY_ENVIRONMENT,
-            component: {
-                document: {
+            visibleTextDocuments: [
+                {
                     uri: 'file:///a/b.c',
                     languageId: 'l',
                     text: 't',
                 },
-            },
+            ],
         }
 
         describe('resource', () => {

src/client/context/context.ts

@@ -1,5 +1,6 @@
 import { basename, dirname, extname } from 'path'
-import { Component, Environment } from '../environment'
+import { Environment } from '../environment'
+import { TextDocumentItem } from '../types/textDocument'
 
 /**
  * Returns a new context created by applying the update context to the base context. It is equivalent to `{...base,
@@ -34,7 +35,7 @@ export const EMPTY_CONTEXT: Context = {}
  * @param key the context property key to look up
  * @param scope the user interface component in whose scope this computation should occur
  */
-export function getComputedContextProperty(environment: Environment, key: string, scope?: Component): any {
+export function getComputedContextProperty(environment: Environment, key: string, scope?: TextDocumentItem): any {
     if (key.startsWith('config.')) {
         const prop = key.slice('config.'.length)
         const value = environment.configuration.merged[prop]
@@ -43,12 +44,13 @@ export function getComputedContextProperty(environment: Environment, key: string
         // which a falsey null default is useful).
         return value === undefined ? null : value
     }
-    const component = scope || environment.component
-    if (key === 'resource' || key === 'component') {
-        return !!component
+    const textDocument: TextDocumentItem | null =
+        scope || (environment.visibleTextDocuments && environment.visibleTextDocuments[0])
+    if (key === 'resource' || key === 'component' /* BACKCOMPAT: allow 'component' */) {
+        return !!textDocument
     }
     if (key.startsWith('resource.')) {
-        if (!component) {
+        if (!textDocument) {
             return undefined
         }
         // TODO(sqs): Define these precisely. If the resource is in a repository, what is the "path"? Is it the
@@ -57,23 +59,23 @@ export function getComputedContextProperty(environment: Environment, key: string
         const prop = key.slice('resource.'.length)
         switch (prop) {
             case 'uri':
-                return component.document.uri
+                return textDocument.uri
             case 'basename':
-                return basename(component.document.uri)
+                return basename(textDocument.uri)
             case 'dirname':
-                return dirname(component.document.uri)
+                return dirname(textDocument.uri)
             case 'extname':
-                return extname(component.document.uri)
+                return extname(textDocument.uri)
             case 'language':
-                return component.document.languageId
+                return textDocument.languageId
             case 'textContent':
-                return component.document.text
+                return textDocument.text
             case 'type':
                 return 'textDocument'
         }
     }
     if (key.startsWith('component.')) {
-        if (!component) {
+        if (!textDocument) {
             return undefined
         }
         const prop = key.slice('component.'.length)

src/client/controller.test.ts

@@ -30,9 +30,7 @@ const create = (environment?: Environment): TestController => {
 }
 
 const FIXTURE_ENVIRONMENT: Environment<any, any> = {
-    component: {
-        document: { uri: 'file:///f', languageId: 'l', text: '' },
-    },
+    visibleTextDocuments: [{ uri: 'file:///f', languageId: 'l', text: '' }],
     extensions: [{ id: 'x' }],
     configuration: {},
     context: {},

src/client/controller.ts

@@ -234,7 +234,7 @@ export class Controller<X extends Extension, C extends ConfigurationCascade> imp
             new ClientWindows(
                 client,
                 this.environment.pipe(
-                    map(({ component }) => component),
+                    map(({ visibleTextDocuments }) => visibleTextDocuments),
                     distinctUntilChanged()
                 ),
                 (params: ShowMessageParams) => this._showMessages.next({ ...params }),
@@ -253,7 +253,7 @@ export class Controller<X extends Extension, C extends ConfigurationCascade> imp
             new ClientDocuments(
                 client,
                 this.environment.pipe(
-                    map(({ component }) => component && component.document),
+                    map(({ visibleTextDocuments }) => visibleTextDocuments),
                     distinctUntilChanged()
                 )
             )

src/client/environment.ts

@@ -14,11 +14,10 @@ import { TextDocumentItem } from './types/textDocument'
  */
 export interface Environment<X extends Extension = Extension, C extends ConfigurationCascade = ConfigurationCascade> {
     /**
-     * The active user interface component (i.e., file view or editor), or null if there is none.
-     *
-     * TODO(sqs): Support multiple components.
+     * The text documents that are currently visible. Each text document is represented to extensions as being
+     * in its own visible CodeEditor.
      */
-    readonly component: Component | null
+    readonly visibleTextDocuments: TextDocumentItem[] | null
 
     /** The active extensions, or null if there are none. */
     readonly extensions: X[] | null
@@ -32,14 +31,8 @@ export interface Environment<X extends Extension = Extension, C extends Configur
 
 /** An empty Sourcegraph extension client environment. */
 export const EMPTY_ENVIRONMENT: Environment<any, any> = {
-    component: null,
+    visibleTextDocuments: null,
     extensions: null,
     configuration: { merged: {} },
     context: EMPTY_CONTEXT,
 }
-
-/** An application component that displays a [TextDocument](#TextDocument). */
-export interface Component {
-    /** The document displayed by the component. */
-    readonly document: TextDocumentItem
-}

src/client/providers/contribution.ts

@@ -12,7 +12,8 @@ import { flatten, isEqual } from '../../util'
 import { getComputedContextProperty } from '../context/context'
 import { ComputedContext, evaluate, evaluateTemplate } from '../context/expr/evaluator'
 import { TEMPLATE_BEGIN } from '../context/expr/lexer'
-import { Component, Environment } from '../environment'
+import { Environment } from '../environment'
+import { TextDocumentItem } from '../types/textDocument'
 
 /** A registered set of contributions from an extension in the registry. */
 export interface ContributionsEntry {
@@ -74,13 +75,13 @@ export class ContributionRegistry {
      * Returns an observable that emits all contributions (merged) evaluated in the current
      * environment (with the optional scope). It emits whenever there is any change.
      */
-    public getContributions(scope?: Component): Observable<Contributions> {
+    public getContributions(scope?: TextDocumentItem): Observable<Contributions> {
         return this.getContributionsFromEntries(this._entries, scope)
     }
 
     protected getContributionsFromEntries(
         entries: Observable<ContributionsEntry[]>,
-        scope?: Component
+        scope?: TextDocumentItem
     ): Observable<Contributions> {
         return combineLatest(
             entries.pipe(

src/extension/api/documents.ts

@@ -4,7 +4,7 @@ import { TextDocumentItem } from '../../client/types/textDocument'
 
 /** @internal */
 export interface ExtDocumentsAPI {
-    $acceptDocumentData(doc: TextDocumentItem): void
+    $acceptDocumentData(doc: TextDocumentItem[]): void
 }
 
 /** @internal */
@@ -54,8 +54,17 @@ export class ExtDocuments implements ExtDocumentsAPI {
     private textDocumentAdds = new Subject<TextDocument>()
     public readonly onDidOpenTextDocument: Observable<TextDocument> = this.textDocumentAdds
 
-    public $acceptDocumentData(doc: TextDocumentItem): void {
-        this.documents.set(doc.uri, doc)
-        this.textDocumentAdds.next(doc)
+    public $acceptDocumentData(docs: TextDocumentItem[] | null): void {
+        if (!docs) {
+            // We don't ever (yet) communicate to the extension when docs are closed.
+            return
+        }
+        for (const doc of docs) {
+            const isNew = !this.documents.has(doc.uri)
+            this.documents.set(doc.uri, doc)
+            if (isNew) {
+                this.textDocumentAdds.next(doc)
+            }
+        }
     }
 }

src/integration-test/documents.test.ts

@@ -19,9 +19,7 @@ describe('Documents (integration)', () => {
             const prevEnvironment = getEnvironment()
             clientController.setEnvironment({
                 ...prevEnvironment,
-                component: {
-                    document: { uri: 'file:///f2', languageId: 'l2', text: 't2' },
-                },
+                visibleTextDocuments: [{ uri: 'file:///f2', languageId: 'l2', text: 't2' }],
             })
 
             await ready
@@ -43,9 +41,7 @@ describe('Documents (integration)', () => {
             const prevEnvironment = getEnvironment()
             clientController.setEnvironment({
                 ...prevEnvironment,
-                component: {
-                    document: { uri: 'file:///f2', languageId: 'l2', text: 't2' },
-                },
+                visibleTextDocuments: [{ uri: 'file:///f2', languageId: 'l2', text: 't2' }],
             })
             await extensionHost.internal.sync()
 

src/integration-test/helpers.test.ts

@@ -6,9 +6,13 @@ import { createExtensionHost } from '../extension/extensionHost'
 import { createMessageTransports } from '../protocol/jsonrpc2/helpers.test'
 
 const FIXTURE_ENVIRONMENT: Environment = {
-    component: {
-        document: { uri: 'file:///f', languageId: 'l', text: 't' },
-    },
+    visibleTextDocuments: [
+        {
+            uri: 'file:///f',
+            languageId: 'l',
+            text: 't',
+        },
+    ],
     extensions: [{ id: 'x' }],
     configuration: { merged: { a: 1 } },
     context: {},

src/integration-test/windows.test.ts

@@ -43,9 +43,7 @@ describe('Windows (integration)', () => {
             const prevEnvironment = getEnvironment()
             clientController.setEnvironment({
                 ...prevEnvironment,
-                component: {
-                    document: { uri: 'file:///f2', languageId: 'l2', text: 't2' },
-                },
+                visibleTextDocuments: [{ uri: 'file:///f2', languageId: 'l2', text: 't2' }],
             })
 
             await ready