fix: clean up how to handle file/directory URIs

olafurpg · olafurpg · commit f631b925b5b0 · 2021-09-23T20:41:34.000+02:00
diff --git a/src/commands/SourcegraphQuickPick.ts b/src/commands/SourcegraphQuickPick.ts
@@ -69,19 +69,9 @@ export class SourcegraphQuickPick {
 
     private async resolveFileUri(selection: BrowseQuickPickItem): Promise<SourcegraphUri> {
         let uriString = selection.uri
-        if (selection.unresolvedRepositoryName) {
-            // Update the missing file path if it's missing
-            if (!uriString || !SourcegraphUri.parse(uriString).path) {
-                uriString = (await this.fs.defaultFileUri(selection.unresolvedRepositoryName)).uri
-            }
-
-            // Update the missing revision if it's missing
-            const uri = SourcegraphUri.parse(uriString)
-            if (!uri.revision) {
-                const metadata = await this.fs.repositoryMetadata(uri.repositoryName)
-                const revision = metadata?.defaultBranch || 'HEAD'
-                uriString = uri.withRevision(revision).uri
-            }
+        if (selection.unresolvedRepositoryName && (!uriString || !SourcegraphUri.parse(uriString).path)) {
+            // Update the missing file path
+            uriString = (await this.fs.defaultFileUri(selection.unresolvedRepositoryName)).uri
         }
         return SourcegraphUri.parse(uriString)
     }
diff --git a/src/commands/goToFileCommand.ts b/src/commands/goToFileCommand.ts
@@ -35,5 +35,5 @@ export async function goToFileCommand(fs: SourcegraphFileSystemProvider): Promis
     )
     const uri = await quick.showQuickPickAndGetUserInput()
     await recentlyOpenFilesSetting.update(uri.uri)
-    await openSourcegraphUriCommand(uri)
+    await openSourcegraphUriCommand(fs, uri)
 }
diff --git a/src/commands/goToRepositoryCommand.ts b/src/commands/goToRepositoryCommand.ts
@@ -50,5 +50,5 @@ export async function goToRepositoryCommand(fs: SourcegraphFileSystemProvider):
     })
     const uri = await quick.showQuickPickAndGetUserInput()
     await recentlyOpenRepositoriesSetting.update({ label: uri.repositoryName, uri: uri.uri })
-    await openSourcegraphUriCommand(uri)
+    await openSourcegraphUriCommand(fs, uri)
 }
diff --git a/src/commands/openSourcegraphUriCommand.ts b/src/commands/openSourcegraphUriCommand.ts
@@ -1,7 +1,12 @@
 import * as vscode from 'vscode'
+import { SourcegraphFileSystemProvider } from '../file-system/SourcegraphFileSystemProvider'
 import { SourcegraphUri } from '../file-system/SourcegraphUri'
 
-export async function openSourcegraphUriCommand(uri: SourcegraphUri): Promise<void> {
+export async function openSourcegraphUriCommand(fs: SourcegraphFileSystemProvider, uri: SourcegraphUri): Promise<void> {
+    if (!uri.revision) {
+        const metadata = await fs.repositoryMetadata(uri.repositoryName)
+        uri = uri.withRevision(metadata?.defaultBranch || 'HEAD')
+    }
     const textDocument = await vscode.workspace.openTextDocument(vscode.Uri.parse(uri.uri))
     const selection = getSelection(uri, textDocument)
     await vscode.window.showTextDocument(textDocument, {
diff --git a/src/extension.ts b/src/extension.ts
@@ -107,7 +107,7 @@ export function activate(context: vscode.ExtensionContext): void {
     context.subscriptions.push(
         vscode.commands.registerCommand('extension.openFile', async uri => {
             if (typeof uri === 'string') {
-                await openSourcegraphUriCommand(SourcegraphUri.parse(uri))
+                await openSourcegraphUriCommand(fs, SourcegraphUri.parse(uri))
             } else {
                 // eslint-disable-next-line @typescript-eslint/restrict-template-expressions
                 log.error(`extension.openFile(${uri}) argument is not a string`)
diff --git a/src/file-system/FileTree.ts b/src/file-system/FileTree.ts
@@ -28,13 +28,15 @@ export class FileTree {
                 continue
             }
             if (file.startsWith(directory)) {
-                const revision = this.uri.revision ? `@${this.uri.revision}` : ''
                 const fileDepth = this.depth(file)
-                const isDirect = isRoot ? fileDepth === 0 : fileDepth === depth + 1
-                const path = isDirect ? file : file.slice(0, file.indexOf('/', directory.length))
-                const kind = isDirect ? 'blob' : 'tree'
-                const uri = `sourcegraph://${this.uri.host}/${this.uri.repositoryName}${revision}/-/${kind}/${path}`
-                if (isDirect) {
+                const isFile = isRoot ? fileDepth === 0 : fileDepth === depth + 1
+                const path = isFile ? file : file.slice(0, file.indexOf('/', directory.length))
+                const uri = SourcegraphUri.fromParts(this.uri.host, this.uri.repositoryName, {
+                    revision: this.uri.revision,
+                    path,
+                    isDirectory: !isFile,
+                }).uri
+                if (isFile) {
                     directFiles.add(uri)
                 } else {
                     directDirectories.add(uri)
diff --git a/src/file-system/SourcegraphFileSystemProvider.ts b/src/file-system/SourcegraphFileSystemProvider.ts
@@ -32,26 +32,34 @@ export class SourcegraphFileSystemProvider implements vscode.FileSystemProvider
     private fileNamesByRepository: Map<string, Promise<string[]>> = new Map()
     private metadata: Map<string, RepositoryMetadata> = new Map()
     private didDownloadFilenames = new vscode.EventEmitter<string>()
-    private readonly cache = new Map<string, Blob>()
 
     // ======================
     // FileSystemProvider API
     // ======================
     private didChangeFile = new vscode.EventEmitter<vscode.FileChangeEvent[]>() // Never used.
     public readonly onDidChangeFile: vscode.Event<vscode.FileChangeEvent[]> = this.didChangeFile.event
-    public async stat(uri: vscode.Uri): Promise<vscode.FileStat> {
-        const blob = await this.fetchBlob(this.sourcegraphUri(uri))
+    public async stat(vscodeUri: vscode.Uri): Promise<vscode.FileStat> {
+        const uri = this.sourcegraphUri(vscodeUri)
+        const files = await this.downloadedFiles(uri)
+        const isFile = uri.path && files.includes(uri.path)
+        const type = isFile ? vscode.FileType.File : vscode.FileType.Directory
+        const now = Date.now()
         return {
-            mtime: blob.time,
-            ctime: blob.time,
-            size: blob.byteSize,
-            type: blob.type,
+            // It seems to be OK to return hardcoded values for the timestamps
+            // and the byte size.  If it turns out the byte size needs to be
+            // correct for some reason, then we can use
+            // `this.fetchBlob(uri).byteSize` to get the value for files.
+            mtime: now,
+            ctime: now,
+            size: 1337,
+            type,
         }
     }
 
     public async readFile(vscodeUri: vscode.Uri): Promise<Uint8Array> {
         const uri = this.sourcegraphUri(vscodeUri)
-        return (await this.fetchBlob(uri)).content
+        const blob = await this.fetchBlob(uri)
+        return blob.content
     }
 
     public async readDirectory(vscodeUri: vscode.Uri): Promise<[string, vscode.FileType][]> {
@@ -60,13 +68,10 @@ export class SourcegraphFileSystemProvider implements vscode.FileSystemProvider
             return []
         }
         const tree = await this.getFileTree(uri)
-        if (!tree) {
-            return []
-        }
         const children = tree.directChildren(uri.path || '')
         return children.map(childUri => {
             const child = SourcegraphUri.parse(childUri)
-            const type = child.isDirectory ? vscode.FileType.Directory : vscode.FileType.File
+            const type = child.isDirectory() ? vscode.FileType.Directory : vscode.FileType.File
             return [child.basename(), type]
         })
     }
@@ -113,7 +118,7 @@ export class SourcegraphFileSystemProvider implements vscode.FileSystemProvider
                     fileNames,
                 })
             } catch {
-                log.appendLine(`ERROR: failed to download repo files ${repositoryUri}`)
+                log.error(`failed to download files for repository '${repositoryUri}'`)
             }
         }
         return promises
@@ -148,12 +153,12 @@ export class SourcegraphFileSystemProvider implements vscode.FileSystemProvider
         const token = emptyCancelationToken()
         const defaultBranch = (await this.repositoryMetadata(repositoryName, token))?.defaultBranch
         if (!defaultBranch) {
-            const message = `ERROR defaultBranch no repository '${repositoryName}'`
-            log.appendLine(message)
+            const message = `repository '${repositoryName}' has no default branch`
+            log.error(message)
             throw new Error(message)
         }
         const uri = SourcegraphUri.fromParts(endpointHostnameSetting(), repositoryName, { revision: defaultBranch })
-        const files = await this.downloadFiles(uri, defaultBranch)
+        const files = await this.downloadFiles(uri)
         const readmes = files.filter(name => name.match(/readme/i))
         const candidates = readmes.length > 0 ? readmes : files
         let readme: string | undefined
@@ -176,21 +181,18 @@ export class SourcegraphFileSystemProvider implements vscode.FileSystemProvider
     }
 
     public async fetchBlob(uri: SourcegraphUri): Promise<Blob> {
-        const result = this.cache.get(uri.uri)
-        if (result) {
-            return result
-        }
         await this.repositoryMetadata(uri.repositoryName)
         const token = emptyCancelationToken()
-        const revision = uri.revision || (await this.repositoryMetadata(uri.repositoryName, token))?.defaultBranch
-        if (!revision) {
-            throw new Error(`no uri.revision from uri ${uri.uri}`)
+        if (!uri.revision) {
+            const error = `missing revision for URI '${uri.uri}'`
+            log.error(error)
+            throw new Error(error)
         }
         const path = uri.path || ''
         const content = await contents(
             {
                 repository: uri.repositoryName,
-                revision,
+                revision: uri.revision,
                 path,
             },
             token
@@ -200,18 +202,17 @@ export class SourcegraphFileSystemProvider implements vscode.FileSystemProvider
             const toCacheResult: Blob = {
                 uri: uri.uri,
                 repositoryName: uri.repositoryName,
-                revision,
+                revision: uri.revision,
                 content: content.content,
                 isBinaryFile: content.isBinary,
                 byteSize: content.byteSize,
                 path,
                 time: new Date().getMilliseconds(),
                 type: vscode.FileType.File,
             }
-            this.cache.set(uri.uri, toCacheResult)
 
             // Start downloading the repository files in the background.
-            this.downloadFiles(uri, revision).then(
+            this.downloadFiles(uri).then(
                 () => {},
                 () => {}
             )
@@ -235,12 +236,19 @@ export class SourcegraphFileSystemProvider implements vscode.FileSystemProvider
         return metadata
     }
 
-    public downloadFiles(uri: SourcegraphUri, revision: string): Promise<string[]> {
+    public async downloadedFiles(uri: SourcegraphUri): Promise<string[]> {
+        return this.fileNamesByRepository.get(uri.repositoryUri()) || []
+    }
+
+    public downloadFiles(uri: SourcegraphUri): Promise<string[]> {
         const key = uri.repositoryUri()
         const fileNamesByRepository = this.fileNamesByRepository
         let downloadingFiles = this.fileNamesByRepository.get(key)
         if (!downloadingFiles) {
-            downloadingFiles = filesQuery({ repository: uri.repositoryName, revision }, emptyCancelationToken())
+            downloadingFiles = filesQuery(
+                { repository: uri.repositoryName, revision: uri.revision },
+                emptyCancelationToken()
+            )
             downloadingFiles.then(
                 () => this.didDownloadFilenames.fire(key),
                 () => fileNamesByRepository.delete(key)
@@ -254,22 +262,8 @@ export class SourcegraphFileSystemProvider implements vscode.FileSystemProvider
         return SourcegraphUri.parse(uri.toString(true))
     }
 
-    public async getFileTree(uri: SourcegraphUri): Promise<FileTree | undefined> {
-        if (!uri.revision) {
-            uri = uri.withRevision(this.metadata.get(uri.repositoryName)?.defaultBranch)
-        }
-        const key = uri.repositoryUri()
-        const downloading = this.fileNamesByRepository.get(key)
-        if (!downloading) {
-            const keys = JSON.stringify([...this.fileNamesByRepository.keys()])
-            log.error(`getFileTree(${uri.uri}) - empty downloading key=${key} keys=${keys}`)
-            return Promise.resolve(undefined)
-        }
-        const files = await downloading
-        if (!files) {
-            log.error('getFileTree - empty files')
-            return Promise.resolve(undefined)
-        }
+    public async getFileTree(uri: SourcegraphUri): Promise<FileTree> {
+        const files = await this.downloadedFiles(uri)
         return new FileTree(uri, files)
     }
 }
diff --git a/src/file-system/SourcegraphTreeDataProvider.ts b/src/file-system/SourcegraphTreeDataProvider.ts
@@ -52,8 +52,7 @@ export class SourcegraphTreeDataProvider implements vscode.TreeDataProvider<stri
             }
             const uri = SourcegraphUri.parse(uriString)
             const tree = await this.fs.getFileTree(uri)
-            const result = tree?.directChildren(uri.path || '')
-            return result
+            return tree.directChildren(uri.path || '')
         } catch (error) {
             log.error(`getChildren(${uriString || ''})`, error)
             return Promise.resolve(undefined)
@@ -71,19 +70,17 @@ export class SourcegraphTreeDataProvider implements vscode.TreeDataProvider<stri
         this.activeUri = vscodeUri
         if (vscodeUri && vscodeUri.scheme === 'sourcegraph' && this.treeView && this.isTreeViewVisible) {
             const uri = this.fs.sourcegraphUri(vscodeUri)
-            await this.fs.downloadFiles(uri, uri.revision || '')
+            await this.fs.downloadFiles(uri)
             await this.didFocusString(uri, true, this.didFocusToken.token)
         }
     }
 
     public async getTreeItem(uriString: string): Promise<vscode.TreeItem> {
-        const uri = SourcegraphUri.parse(uriString)
         try {
+            const uri = SourcegraphUri.parse(uriString)
             const label = await this.treeItemLabel(uri)
-            const isFile = uri.uri.includes('/-/blob/')
-            const isDirectory = !isFile
-            const collapsibleState = await this.getCollapsibleState(uri, isDirectory)
-            const command = isFile
+            const collapsibleState = await this.getCollapsibleState(uri)
+            const command = uri.isFile()
                 ? {
                       command: 'extension.openFile',
                       title: 'Open file',
@@ -99,7 +96,7 @@ export class SourcegraphTreeDataProvider implements vscode.TreeDataProvider<stri
                 resourceUri: vscode.Uri.parse(uri.uri),
             }
         } catch (error) {
-            log.error(`getTreeItem(${uri.uri})`, error)
+            log.error(`getTreeItem(${uriString})`, error)
             return Promise.resolve({})
         }
     }
@@ -141,21 +138,21 @@ export class SourcegraphTreeDataProvider implements vscode.TreeDataProvider<stri
         return `${uri.repositoryName}@${revision || ''}`
     }
 
-    private async getCollapsibleState(
-        uri: SourcegraphUri,
-        isDirectory: boolean
-    ): Promise<vscode.TreeItemCollapsibleState> {
-        const parent = uri.parentUri()
-        if (isDirectory && parent) {
-            const parentUri = SourcegraphUri.parse(parent)
-            if (parentUri.path) {
-                const tree = await this.fs.getFileTree(parentUri)
-                const directChildren = tree?.directChildren(parentUri.path)
+    private async getCollapsibleState(uri: SourcegraphUri): Promise<vscode.TreeItemCollapsibleState> {
+        if (uri.isFile()) {
+            return vscode.TreeItemCollapsibleState.None
+        }
+        const parentUri = uri.parentUri()
+        if (parentUri) {
+            const parent = SourcegraphUri.parse(parentUri)
+            if (parent.path) {
+                const tree = await this.fs.getFileTree(parent)
+                const directChildren = tree.directChildren(parent.path)
                 if (directChildren && directChildren.length === 1) {
                     return vscode.TreeItemCollapsibleState.Expanded
                 }
             }
         }
-        return isDirectory ? vscode.TreeItemCollapsibleState.Collapsed : vscode.TreeItemCollapsibleState.None
+        return vscode.TreeItemCollapsibleState.Collapsed
     }
 }
diff --git a/src/file-system/SourcegraphUri.ts b/src/file-system/SourcegraphUri.ts
@@ -9,10 +9,9 @@ export class SourcegraphUri {
         public readonly uri: string,
         public readonly host: string,
         public readonly repositoryName: string,
-        public readonly revision: string | undefined,
+        public readonly revision: string,
         public readonly path: string | undefined,
-        public readonly position: Position | undefined,
-        public readonly isDirectory: boolean
+        public readonly position: Position | undefined
     ) {}
 
     public withRevision(newRevision: string | undefined): SourcegraphUri {
@@ -50,8 +49,21 @@ export class SourcegraphUri {
         return undefined
     }
 
+    public withIsDirectory(isDirectory: boolean): SourcegraphUri {
+        return SourcegraphUri.fromParts(this.host, this.repositoryName, {
+            isDirectory,
+            path: this.path,
+            revision: this.revision,
+            position: this.position,
+        })
+    }
+
+    public isDirectory(): boolean {
+        return !this.isFile()
+    }
+
     public isFile(): boolean {
-        return typeof this.path === 'string' && this.uri.includes('/-/blob/')
+        return this.uri.includes('/-/blob/')
     }
 
     public static fromParts(
@@ -65,15 +77,15 @@ export class SourcegraphUri {
         }
     ): SourcegraphUri {
         const revisionPart = optional?.revision ? `@${optional.revision}` : ''
-        const pathPart = optional?.path ? `/-/blob/${optional?.path}` : ''
+        const directoryPart = optional?.isDirectory ? 'tree' : 'blob'
+        const pathPart = optional?.path ? `/-/${directoryPart}/${optional?.path}` : ''
         return new SourcegraphUri(
             `sourcegraph://${host}/${repositoryName}${revisionPart}${pathPart}`,
             host,
             repositoryName,
-            optional?.revision,
+            optional?.revision || '',
             optional?.path,
-            optional?.position,
-            optional?.isDirectory || false
+            optional?.position
         )
     }
     public repositoryUri(): string {
diff --git a/src/queries/graphqlQuery.ts b/src/queries/graphqlQuery.ts
@@ -15,7 +15,6 @@ export function graphqlQueryWithAccessToken<A, B>(
     token: CancellationToken,
     accessToken: string
 ): Promise<B | undefined> {
-    log.appendLine(`accessToken ${accessToken}`)
     return new Promise<B | undefined>((resolve, reject) => {
         const data = JSON.stringify({
             query,
diff --git a/src/search/SourcegraphNotebookSerializer.ts b/src/search/SourcegraphNotebookSerializer.ts
diff --git a/src/settings/readConfiguration.ts b/src/settings/readConfiguration.ts

Original file line number	Diff line number	Diff line change
`@@ -35,5 +35,5 @@ export async function goToFileCommand(fs: SourcegraphFileSystemProvider): Promis`
`35`	`35`	`)`
`36`	`36`	`const uri = await quick.showQuickPickAndGetUserInput()`
`37`	`37`	`await recentlyOpenFilesSetting.update(uri.uri)`
`38`		`- await openSourcegraphUriCommand(uri)`
	`38`	`+ await openSourcegraphUriCommand(fs, uri)`
`39`	`39`	`}`
Original file line number	Diff line number	Diff line change
`@@ -50,5 +50,5 @@ export async function goToRepositoryCommand(fs: SourcegraphFileSystemProvider):`
`50`	`50`	`})`
`51`	`51`	`const uri = await quick.showQuickPickAndGetUserInput()`
`52`	`52`	`await recentlyOpenRepositoriesSetting.update({ label: uri.repositoryName, uri: uri.uri })`
`53`		`- await openSourcegraphUriCommand(uri)`
	`53`	`+ await openSourcegraphUriCommand(fs, uri)`
`54`	`54`	`}`