From 04c3dde6e49613d716dd16ef253c6351d8b5077d Mon Sep 17 00:00:00 2001 From: Ilya Biryukov Date: Mon, 7 Jan 2019 11:31:28 -0800 Subject: [PATCH] Address PR feedback on Git provide APIs --- extensions/git/src/api/api1.ts | 16 ++++++------ extensions/git/src/api/git.d.ts | 8 ++++-- extensions/git/src/git.ts | 44 ++++++++++++++++++++++---------- extensions/git/src/repository.ts | 11 +++++--- 4 files changed, 53 insertions(+), 26 deletions(-) diff --git a/extensions/git/src/api/api1.ts b/extensions/git/src/api/api1.ts index 9d8a590178664..769a646baebf7 100644 --- a/extensions/git/src/api/api1.ts +++ b/extensions/git/src/api/api1.ts @@ -5,7 +5,7 @@ import { Model } from '../model'; import { Repository as BaseRepository, Resource } from '../repository'; -import { InputBox, Git, API, Repository, Remote, RepositoryState, Branch, Ref, Submodule, Commit, Change, RepositoryUIState, Status, GitLogOptions } from './git'; +import { InputBox, Git, API, Repository, Remote, RepositoryState, Branch, Ref, Submodule, Commit, Change, RepositoryUIState, Status, LogOptions } from './git'; import { Event, SourceControlInputBox, Uri, SourceControl } from 'vscode'; import { mapEvent } from '../util'; @@ -111,25 +111,25 @@ export class ApiRepository implements Repository { diffWithHEAD(): Promise; diffWithHEAD(path: string): Promise; diffWithHEAD(path?: string): Promise { - return path ? this._repository.diffWithHEAD(path) : this._repository.diffWithHEAD(); + return this._repository.diffWithHEAD(path); } diffWith(ref: string): Promise; diffWith(ref: string, path: string): Promise; diffWith(ref: string, path?: string): Promise { - return path ? this._repository.diffWith(ref, path) : this._repository.diffWith(ref); + return this._repository.diffWith(ref, path); } diffIndexWithHEAD(): Promise; diffIndexWithHEAD(path: string): Promise; diffIndexWithHEAD(path?: string): Promise { - return path ? this._repository.diffIndexWithHEAD(path) : this._repository.diffIndexWithHEAD(); + return this._repository.diffIndexWithHEAD(path); } diffIndexWith(ref: string): Promise; diffIndexWith(ref: string, path: string): Promise; diffIndexWith(ref: string, path?: string): Promise { - return path ? this._repository.diffIndexWith(ref, path) : this._repository.diffIndexWith(ref); + return this._repository.diffIndexWith(ref, path); } diffBlobs(object1: string, object2: string): Promise { @@ -139,7 +139,7 @@ export class ApiRepository implements Repository { diffBetween(ref1: string, ref2: string): Promise; diffBetween(ref1: string, ref2: string, path: string): Promise; diffBetween(ref1: string, ref2: string, path?: string): Promise { - return path ? this._repository.diffBetween(ref1, ref2, path) : this._repository.diffBetween(ref1, ref2); + return this._repository.diffBetween(ref1, ref2, path); } hashObject(data: string): Promise { @@ -194,8 +194,8 @@ export class ApiRepository implements Repository { return this._repository.pushTo(remoteName, branchName, setUpstream); } - getLog(options?: GitLogOptions): Promise { - return this._repository.getLog(options); + log(options?: LogOptions): Promise { + return this._repository.log(options); } } diff --git a/extensions/git/src/api/git.d.ts b/extensions/git/src/api/git.d.ts index ff6ce04ab999a..c5e2b3bc9d2cb 100644 --- a/extensions/git/src/api/git.d.ts +++ b/extensions/git/src/api/git.d.ts @@ -110,7 +110,11 @@ export interface RepositoryUIState { readonly onDidChange: Event; } -export interface GitLogOptions { +/** + * Log options. + */ +export interface LogOptions { + /** Max number of log entries to retrieve. If not specified, the default is 32. */ readonly maxEntries?: number; } @@ -167,7 +171,7 @@ export interface Repository { pull(): Promise; push(remoteName?: string, branchName?: string, setUpstream?: boolean): Promise; - getLog(options?: GitLogOptions): Promise; + log(options?: LogOptions): Promise; } export interface API { diff --git a/extensions/git/src/git.ts b/extensions/git/src/git.ts index d104d3cbdf48c..a06598f597484 100644 --- a/extensions/git/src/git.ts +++ b/extensions/git/src/git.ts @@ -14,7 +14,7 @@ import * as filetype from 'file-type'; import { assign, groupBy, denodeify, IDisposable, toDisposable, dispose, mkdirp, readBytes, detectUnicodeEncoding, Encoding, onceEvent } from './util'; import { CancellationToken, Uri } from 'vscode'; import { detectEncoding } from './encoding'; -import { Ref, RefType, Branch, Remote, GitErrorCodes, GitLogOptions, Change, Status } from './api/git'; +import { Ref, RefType, Branch, Remote, GitErrorCodes, LogOptions, Change, Status } from './api/git'; const readfile = denodeify(fs.readFile); @@ -700,33 +700,36 @@ export class Repository { }); } - async getLog(options?: GitLogOptions): Promise { - const args = ['log']; - if (options) { - if (typeof options.maxEntries === 'number' && options.maxEntries > 0) { - args.push('-' + options.maxEntries); - } - } - - args.push(`--pretty=format:${COMMIT_FORMAT}%x00%x00`); + async log(options?: LogOptions): Promise { + const maxEntries = options && typeof options.maxEntries === 'number' && options.maxEntries > 0 ? options.maxEntries : 32; + const args = ['log', '-' + maxEntries, `--pretty=format:${COMMIT_FORMAT}%x00%x00`]; const gitResult = await this.run(args); if (gitResult.exitCode) { // An empty repo. return []; } - const entries = gitResult.stdout.split('\x00\x00'); + const s = gitResult.stdout; const result: Commit[] = []; - for (let entry of entries) { + let index = 0; + while (index < s.length) { + let nextIndex = s.indexOf('\x00\x00', index); + if (nextIndex === -1) { + nextIndex = s.length; + } + + let entry = s.substr(index, nextIndex - index); if (entry.startsWith('\n')) { entry = entry.substring(1); } + const commit = parseGitCommit(entry); if (!commit) { break; } result.push(commit); + index = nextIndex + 2; } return result; @@ -886,7 +889,10 @@ export class Repository { return result.stdout; } - async diffWithHEAD(path?: string): Promise { + diffWithHEAD(): Promise; + diffWithHEAD(path: string): Promise; + diffWithHEAD(path?: string | undefined): Promise; + async diffWithHEAD(path?: string | undefined): Promise { if (!path) { return await this.diffFiles(false); } @@ -896,6 +902,9 @@ export class Repository { return result.stdout; } + diffWith(ref: string): Promise; + diffWith(ref: string, path: string): Promise; + diffWith(ref: string, path?: string | undefined): Promise; async diffWith(ref: string, path?: string): Promise { if (!path) { return await this.diffFiles(false, ref); @@ -906,6 +915,9 @@ export class Repository { return result.stdout; } + diffIndexWithHEAD(): Promise; + diffIndexWithHEAD(path: string): Promise; + diffIndexWithHEAD(path?: string | undefined): Promise; async diffIndexWithHEAD(path?: string): Promise { if (!path) { return await this.diffFiles(true); @@ -916,6 +928,9 @@ export class Repository { return result.stdout; } + diffIndexWith(ref: string): Promise; + diffIndexWith(ref: string, path: string): Promise; + diffIndexWith(ref: string, path?: string | undefined): Promise; async diffIndexWith(ref: string, path?: string): Promise { if (!path) { return await this.diffFiles(true, ref); @@ -932,6 +947,9 @@ export class Repository { return result.stdout; } + diffBetween(ref1: string, ref2: string): Promise; + diffBetween(ref1: string, ref2: string, path: string): Promise; + diffBetween(ref1: string, ref2: string, path?: string | undefined): Promise; async diffBetween(ref1: string, ref2: string, path?: string): Promise { const range = `${ref1}...${ref2}`; if (!path) { diff --git a/extensions/git/src/repository.ts b/extensions/git/src/repository.ts index e9650b905a792..dc3399e3ec942 100644 --- a/extensions/git/src/repository.ts +++ b/extensions/git/src/repository.ts @@ -13,7 +13,7 @@ import * as path from 'path'; import * as nls from 'vscode-nls'; import * as fs from 'fs'; import { StatusBarCommands } from './statusbar'; -import { Branch, Ref, Remote, RefType, GitErrorCodes, Status, GitLogOptions, Change } from './api/git'; +import { Branch, Ref, Remote, RefType, GitErrorCodes, Status, LogOptions, Change } from './api/git'; const timeout = (millis: number) => new Promise(c => setTimeout(c, millis)); @@ -706,8 +706,8 @@ export class Repository implements Disposable { return this.run(Operation.Config, () => this.repository.config('local', key, value)); } - getLog(options?: GitLogOptions): Promise { - return this.run(Operation.Log, () => this.repository.getLog(options)); + log(options?: LogOptions): Promise { + return this.run(Operation.Log, () => this.repository.log(options)); } @throttle @@ -721,24 +721,28 @@ export class Repository implements Disposable { diffWithHEAD(): Promise; diffWithHEAD(path: string): Promise; + diffWithHEAD(path?: string | undefined): Promise; diffWithHEAD(path?: string | undefined): Promise { return this.run(Operation.Diff, () => this.repository.diffWithHEAD(path)); } diffWith(ref: string): Promise; diffWith(ref: string, path: string): Promise; + diffWith(ref: string, path?: string | undefined): Promise; diffWith(ref: string, path?: string): Promise { return this.run(Operation.Diff, () => this.repository.diffWith(ref, path)); } diffIndexWithHEAD(): Promise; diffIndexWithHEAD(path: string): Promise; + diffIndexWithHEAD(path?: string | undefined): Promise; diffIndexWithHEAD(path?: string): Promise { return this.run(Operation.Diff, () => this.repository.diffIndexWithHEAD(path)); } diffIndexWith(ref: string): Promise; diffIndexWith(ref: string, path: string): Promise; + diffIndexWith(ref: string, path?: string | undefined): Promise; diffIndexWith(ref: string, path?: string): Promise { return this.run(Operation.Diff, () => this.repository.diffIndexWith(ref, path)); } @@ -749,6 +753,7 @@ export class Repository implements Disposable { diffBetween(ref1: string, ref2: string): Promise; diffBetween(ref1: string, ref2: string, path: string): Promise; + diffBetween(ref1: string, ref2: string, path?: string | undefined): Promise; diffBetween(ref1: string, ref2: string, path?: string): Promise { return this.run(Operation.Diff, () => this.repository.diffBetween(ref1, ref2, path)); }