From 4c72765b77a32a0354170b0f9d6ebe91f50137d0 Mon Sep 17 00:00:00 2001 From: Eric Amodio Date: Wed, 12 Sep 2018 03:39:51 -0400 Subject: [PATCH] Fixes issues with comparing added/deleted files Fixes path issues with git fs provider --- src/commands/diffLineWithPrevious.ts | 2 +- src/commands/diffWith.ts | 41 ++++++++++++++++++++---------------- src/commands/diffWithPrevious.ts | 6 +++--- src/git/fsProvider.ts | 22 ++++++++++++------- src/git/git.ts | 26 +++++++++++++++++++++-- src/git/gitService.ts | 28 +++++++++++++----------- src/views/explorerCommands.ts | 2 +- src/views/nodes/lineHistoryNode.ts | 20 ++++++++++++++++-- 8 files changed, 100 insertions(+), 47 deletions(-) diff --git a/src/commands/diffLineWithPrevious.ts b/src/commands/diffLineWithPrevious.ts index caedb9f..b22d561 100644 --- a/src/commands/diffLineWithPrevious.ts +++ b/src/commands/diffLineWithPrevious.ts @@ -64,7 +64,7 @@ export class DiffLineWithPreviousCommand extends ActiveEditorCommand { const diffArgs: DiffWithCommandArgs = { repoPath: args.commit.repoPath, lhs: { - sha: args.commit.previousSha !== undefined ? args.commit.previousSha : GitService.deletedSha, + sha: args.commit.previousSha !== undefined ? args.commit.previousSha : GitService.deletedOrMissingSha, uri: args.commit.previousUri }, rhs: { diff --git a/src/commands/diffWith.ts b/src/commands/diffWith.ts index 71e3628..c010ad5 100644 --- a/src/commands/diffWith.ts +++ b/src/commands/diffWith.ts @@ -49,7 +49,10 @@ export class DiffWithCommand extends ActiveEditorCommand { args = { repoPath: commit1.repoPath, lhs: { - sha: commit1.previousSha !== undefined ? commit1.previousSha : GitService.deletedSha, + sha: + commit1.previousSha !== undefined + ? commit1.previousSha + : GitService.deletedOrMissingSha, uri: commit1.previousUri! }, rhs: { @@ -94,13 +97,19 @@ export class DiffWithCommand extends ActiveEditorCommand { if (args.repoPath === undefined || args.lhs === undefined || args.rhs === undefined) return undefined; try { - // If the shas aren't resolved (e.g. a2d24f^), resolve them - if (GitService.isResolveRequired(args.lhs.sha)) { - args.lhs.sha = await Container.git.resolveReference(args.repoPath, args.lhs.sha, args.lhs.uri); - } + let lhsSha = args.lhs.sha; + let rhsSha = args.rhs.sha; + + [args.lhs.sha, args.rhs.sha] = await Promise.all([ + await Container.git.resolveReference(args.repoPath, args.lhs.sha, args.lhs.uri), + await Container.git.resolveReference(args.repoPath, args.rhs.sha, args.rhs.uri) + ]); - if (GitService.isResolveRequired(args.rhs.sha)) { - args.rhs.sha = await Container.git.resolveReference(args.repoPath, args.rhs.sha, args.rhs.uri); + if (args.lhs.sha !== GitService.deletedOrMissingSha) { + lhsSha = args.lhs.sha; + } + if (args.rhs.sha !== GitService.deletedOrMissingSha) { + rhsSha = args.rhs.sha; } const [lhs, rhs] = await Promise.all([ @@ -112,7 +121,7 @@ export class DiffWithCommand extends ActiveEditorCommand { if (rhs === undefined) { rhsPrefix = GitService.isUncommitted(args.rhs.sha) ? ' (deleted)' : 'deleted in '; } - else if (lhs === undefined || args.lhs.sha === GitService.deletedSha) { + else if (lhs === undefined) { rhsPrefix = 'added in '; } @@ -127,18 +136,14 @@ export class DiffWithCommand extends ActiveEditorCommand { } } - if ( - args.lhs.title === undefined && - args.lhs.sha !== GitService.deletedSha && - (lhs !== undefined || lhsPrefix !== '') - ) { - const suffix = GitService.shortenSha(args.lhs.sha) || ''; + if (args.lhs.title === undefined && (lhs !== undefined || lhsPrefix !== '')) { + const suffix = GitService.shortenSha(lhsSha) || ''; args.lhs.title = `${path.basename(args.lhs.uri.fsPath)}${ suffix !== '' ? ` (${lhsPrefix}${suffix})` : '' }`; } - if (args.rhs.title === undefined && args.rhs.sha !== GitService.deletedSha) { - const suffix = GitService.shortenSha(args.rhs.sha, { uncommitted: 'working tree' }) || ''; + if (args.rhs.title === undefined) { + const suffix = GitService.shortenSha(rhsSha, { uncommitted: 'working tree' }) || ''; args.rhs.title = `${path.basename(args.rhs.uri.fsPath)}${ suffix !== '' ? ` (${rhsPrefix}${suffix})` : rhsPrefix }`; @@ -164,10 +169,10 @@ export class DiffWithCommand extends ActiveEditorCommand { return await commands.executeCommand( BuiltInCommands.Diff, lhs === undefined - ? GitUri.toRevisionUri(GitService.deletedSha, args.lhs.uri.fsPath, args.repoPath) + ? GitUri.toRevisionUri(GitService.deletedOrMissingSha, args.lhs.uri.fsPath, args.repoPath) : lhs, rhs === undefined - ? GitUri.toRevisionUri(GitService.deletedSha, args.rhs.uri.fsPath, args.repoPath) + ? GitUri.toRevisionUri(GitService.deletedOrMissingSha, args.rhs.uri.fsPath, args.repoPath) : rhs, title, args.showOptions diff --git a/src/commands/diffWithPrevious.ts b/src/commands/diffWithPrevious.ts index 1f45924..637f511 100644 --- a/src/commands/diffWithPrevious.ts +++ b/src/commands/diffWithPrevious.ts @@ -44,7 +44,7 @@ export class DiffWithPreviousCommand extends ActiveEditorCommand { try { let sha = args.commit === undefined ? gitUri.sha : args.commit.sha; - if (sha === GitService.deletedSha) return Messages.showCommitHasNoPreviousCommitWarningMessage(); + if (sha === GitService.deletedOrMissingSha) return Messages.showCommitHasNoPreviousCommitWarningMessage(); // If we are a fake "staged" sha, remove it let isStagedUncommitted = false; @@ -101,7 +101,7 @@ export class DiffWithPreviousCommand extends ActiveEditorCommand { repoPath: args.commit.repoPath, lhs: { sha: args.inDiffEditor - ? args.commit.previousSha || GitService.deletedSha + ? args.commit.previousSha || GitService.deletedOrMissingSha : args.commit.sha, uri: args.inDiffEditor ? args.commit.previousUri : args.commit.uri }, @@ -152,7 +152,7 @@ export class DiffWithPreviousCommand extends ActiveEditorCommand { const diffArgs: DiffWithCommandArgs = { repoPath: args.commit.repoPath, lhs: { - sha: args.commit.previousSha !== undefined ? args.commit.previousSha : GitService.deletedSha, + sha: args.commit.previousSha !== undefined ? args.commit.previousSha : GitService.deletedOrMissingSha, uri: args.commit.previousUri }, rhs: { diff --git a/src/git/fsProvider.ts b/src/git/fsProvider.ts index bfdb19e..c7a0163 100644 --- a/src/git/fsProvider.ts +++ b/src/git/fsProvider.ts @@ -1,4 +1,5 @@ 'use strict'; +import * as paths from 'path'; import { Disposable, Event, @@ -14,7 +15,7 @@ import { import { DocumentSchemes } from '../constants'; import { Container } from '../container'; import { GitService, GitTree, GitUri } from '../git/gitService'; -import { Iterables, TernarySearchTree } from '../system'; +import { Iterables, Strings, TernarySearchTree } from '../system'; export function fromGitLensFSUri(uri: Uri): { path: string; ref: string; repoPath: string } { const gitUri = uri instanceof GitUri ? uri : GitUri.fromRevisionUri(uri); @@ -60,20 +61,27 @@ export class GitFileSystemProvider implements FileSystemProvider, Disposable { } async readDirectory(uri: Uri): Promise<[string, FileType][]> { - const tree = await this.getTree(uri); + const { path, ref, repoPath } = fromGitLensFSUri(uri); + + const tree = await this.getTree(path, ref, repoPath); if (tree === undefined) { debugger; throw FileSystemError.FileNotFound(uri); } - const items = [...Iterables.map(tree, t => [t.path, typeToFileType(t.type)])]; + const items = [ + ...Iterables.map(tree, t => [ + path !== '' ? Strings.normalizePath(paths.relative(path, t.path)) : t.path, + typeToFileType(t.type) + ]) + ]; return items; } async readFile(uri: Uri): Promise { const { path, ref, repoPath } = fromGitLensFSUri(uri); - if (ref === GitService.deletedSha) return emptyArray; + if (ref === GitService.deletedOrMissingSha) return emptyArray; const buffer = await Container.git.getVersionedFileBuffer(repoPath, path, ref); if (buffer === undefined) return emptyArray; @@ -88,7 +96,7 @@ export class GitFileSystemProvider implements FileSystemProvider, Disposable { async stat(uri: Uri): Promise { const { path, ref, repoPath } = fromGitLensFSUri(uri); - if (ref === GitService.deletedSha) { + if (ref === GitService.deletedOrMissingSha) { return { type: FileType.File, size: 0, @@ -151,9 +159,7 @@ export class GitFileSystemProvider implements FileSystemProvider, Disposable { return searchTree; } - private async getTree(uri: Uri) { - const { path, ref, repoPath } = fromGitLensFSUri(uri); - + private async getTree(path: string, ref: string, repoPath: string) { const searchTree = await this.getOrCreateSearchTree(ref, repoPath); // Add the fake root folder to the path return searchTree!.findSuperstr(`/~/${path}`, true); diff --git a/src/git/git.ts b/src/git/git.ts index 48b2971..7b634d9 100644 --- a/src/git/git.ts +++ b/src/git/git.ts @@ -48,7 +48,8 @@ const stashFormat = [ const defaultStashParams = ['stash', 'list', '--name-status', '-M', `--format=${stashFormat}`]; const GitErrors = { - badRevision: /bad revision \'.*?\'/i + badRevision: /bad revision \'.*?\'/i, + notAValidObjectName: /Not a valid object name/i }; const GitWarnings = { @@ -66,7 +67,7 @@ const GitWarnings = { interface GitCommandOptions extends RunOptions { readonly correlationKey?: string; - exceptionHandler?(ex: Error): string; + exceptionHandler?(ex: Error): string | void; } // A map of running git commands -- avoids running duplicate overlaping commands @@ -164,6 +165,7 @@ function throwExceptionHandler(ex: Error) { let gitInfo: GitLocation; export class Git { + static deletedOrMissingSha = 'ffffffffffffffffffffffffffffffffffffffff'; static shaRegex = /^[0-9a-f]{40}(\^[0-9]*?)??( -)?$/; static shaStrictRegex = /^[0-9a-f]{40}$/; static stagedUncommittedRegex = /^[0]{40}(\^[0-9]*?)??:$/; @@ -545,6 +547,26 @@ export class Git { return data === '' ? undefined : data.trim(); } + static async cat_file_validate(repoPath: string, fileName: string, ref: string) { + try { + await git( + { cwd: repoPath, exceptionHandler: throwExceptionHandler }, + 'cat-file', + '-e', + `${ref}:./${fileName}` + ); + return ref; + } + catch (ex) { + const msg = ex && ex.toString(); + if (GitErrors.notAValidObjectName.test(msg)) { + return Git.deletedOrMissingSha; + } + + return undefined; + } + } + static async log_resolve(repoPath: string, fileName: string, ref: string) { const data = await git( { cwd: repoPath, exceptionHandler: ignoreExceptionsHandler }, diff --git a/src/git/gitService.ts b/src/git/gitService.ts index 0f6dc2c..84b9f69 100644 --- a/src/git/gitService.ts +++ b/src/git/gitService.ts @@ -84,7 +84,7 @@ export enum GitRepoSearchBy { export class GitService implements Disposable { static emptyPromise: Promise = Promise.resolve(undefined); - static deletedSha = 'ffffffffffffffffffffffffffffffffffffffff'; + static deletedOrMissingSha = Git.deletedOrMissingSha; static stagedUncommittedSha = Git.stagedUncommittedSha; static uncommittedSha = Git.uncommittedSha; @@ -1681,9 +1681,9 @@ export class GitService implements Disposable { fileName: string, sha: string | undefined ): Promise { - Logger.log(`getVersionedFile('${repoPath}', '${fileName}', '${sha}')`); + if (sha === GitService.deletedOrMissingSha) return undefined; - if (sha === GitService.deletedSha) return undefined; + Logger.log(`getVersionedFile('${repoPath}', '${fileName}', '${sha}')`); if (!sha || (Git.isUncommitted(sha) && !Git.isStagedUncommitted(sha))) { if (await this.fileExists(repoPath!, fileName)) return Uri.file(fileName); @@ -1735,7 +1735,7 @@ export class GitService implements Disposable { repoPath?: string, options: { ref?: string; skipCacheUpdate?: boolean } = {} ): Promise { - if (options.ref === GitService.deletedSha) return false; + if (options.ref === GitService.deletedOrMissingSha) return false; let ref = options.ref; let cacheKey: string; @@ -1784,7 +1784,7 @@ export class GitService implements Disposable { } private async isTrackedCore(fileName: string, repoPath: string, ref?: string) { - if (ref === GitService.deletedSha) return false; + if (ref === GitService.deletedOrMissingSha) return false; try { // Even if we have a sha, check first to see if the file exists (that way the cache will be better reused) @@ -1833,15 +1833,19 @@ export class GitService implements Disposable { } async resolveReference(repoPath: string, ref: string, uri?: Uri) { - if (!GitService.isResolveRequired(ref) || ref.endsWith('^3')) return ref; + if (ref.endsWith('^3')) return ref; Logger.log(`resolveReference('${repoPath}', '${ref}', '${uri && uri.toString(true)}')`); if (uri == null) return (await Git.revparse(repoPath, ref)) || ref; - return ( - (await Git.log_resolve(repoPath, Strings.normalizePath(path.relative(repoPath, uri.fsPath)), ref)) || ref - ); + const fileName = Strings.normalizePath(path.relative(repoPath, uri.fsPath)); + const resolvedRef = await Git.log_resolve(repoPath, fileName, ref); + + const ensuredRef = await Git.cat_file_validate(repoPath, fileName, resolvedRef || ref); + if (ensuredRef === undefined) return ref; + + return ensuredRef; } stopWatchingFileSystem() { @@ -1919,14 +1923,14 @@ export class GitService implements Disposable { static shortenSha( sha: string | undefined, - strings: { deleted?: string; stagedUncommitted?: string; uncommitted?: string; working?: string } = {} + strings: { deletedOrMissing?: string; stagedUncommitted?: string; uncommitted?: string; working?: string } = {} ) { if (sha === undefined) return undefined; - strings = { deleted: '(deleted)', working: '', ...strings }; + strings = { deletedOrMissing: '(deleted)', working: '', ...strings }; if (sha === '') return strings.working; - if (sha === GitService.deletedSha) return strings.deleted; + if (sha === GitService.deletedOrMissingSha) return strings.deletedOrMissing; return Git.isSha(sha) || Git.isStagedUncommitted(sha) ? Git.shortenSha(sha, strings) : sha; } diff --git a/src/views/explorerCommands.ts b/src/views/explorerCommands.ts index de38b7c..34beb44 100644 --- a/src/views/explorerCommands.ts +++ b/src/views/explorerCommands.ts @@ -268,7 +268,7 @@ export class ExplorerCommands implements Disposable { repoPath, { uri: uri, - sha: node.commit.previousSha !== undefined ? node.commit.previousSha : GitService.deletedSha + sha: node.commit.previousSha !== undefined ? node.commit.previousSha : GitService.deletedOrMissingSha }, { uri: uri, sha: node.commit.sha }, options diff --git a/src/views/nodes/lineHistoryNode.ts b/src/views/nodes/lineHistoryNode.ts index 5191b67..cb9b00b 100644 --- a/src/views/nodes/lineHistoryNode.ts +++ b/src/views/nodes/lineHistoryNode.ts @@ -2,7 +2,13 @@ import { Disposable, Selection, TreeItem, TreeItemCollapsibleState } from 'vscode'; import { Container } from '../../container'; import { GitCommitType, GitLogCommit, IGitStatusFile } from '../../git/git'; -import { GitUri, RepositoryChange, RepositoryChangeEvent, RepositoryFileSystemChangeEvent } from '../../git/gitService'; +import { + GitService, + GitUri, + RepositoryChange, + RepositoryChangeEvent, + RepositoryFileSystemChangeEvent +} from '../../git/gitService'; import { Logger } from '../../logger'; import { Iterables } from '../../system'; import { LineHistoryExplorer } from '../lineHistoryExplorer'; @@ -82,7 +88,17 @@ export class LineHistoryNode extends SubscribeableExplorerNode