Skip to content

Commit

Permalink
Address PR feedback on Git provide APIs
Browse files Browse the repository at this point in the history
  • Loading branch information
Ilya Biryukov committed Jan 7, 2019
1 parent 01292b4 commit 04c3dde
Show file tree
Hide file tree
Showing 4 changed files with 53 additions and 26 deletions.
16 changes: 8 additions & 8 deletions extensions/git/src/api/api1.ts
View file
Expand Up @@ -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';

Expand Down Expand Up @@ -111,25 +111,25 @@ export class ApiRepository implements Repository {
diffWithHEAD(): Promise<Change[]>;
diffWithHEAD(path: string): Promise<string>;
diffWithHEAD(path?: string): Promise<string | Change[]> {
return path ? this._repository.diffWithHEAD(path) : this._repository.diffWithHEAD();
return this._repository.diffWithHEAD(path);
}

diffWith(ref: string): Promise<Change[]>;
diffWith(ref: string, path: string): Promise<string>;
diffWith(ref: string, path?: string): Promise<string | Change[]> {
return path ? this._repository.diffWith(ref, path) : this._repository.diffWith(ref);
return this._repository.diffWith(ref, path);
}

diffIndexWithHEAD(): Promise<Change[]>;
diffIndexWithHEAD(path: string): Promise<string>;
diffIndexWithHEAD(path?: string): Promise<string | Change[]> {
return path ? this._repository.diffIndexWithHEAD(path) : this._repository.diffIndexWithHEAD();
return this._repository.diffIndexWithHEAD(path);
}

diffIndexWith(ref: string): Promise<Change[]>;
diffIndexWith(ref: string, path: string): Promise<string>;
diffIndexWith(ref: string, path?: string): Promise<string | Change[]> {
return path ? this._repository.diffIndexWith(ref, path) : this._repository.diffIndexWith(ref);
return this._repository.diffIndexWith(ref, path);
}

diffBlobs(object1: string, object2: string): Promise<string> {
Expand All @@ -139,7 +139,7 @@ export class ApiRepository implements Repository {
diffBetween(ref1: string, ref2: string): Promise<Change[]>;
diffBetween(ref1: string, ref2: string, path: string): Promise<string>;
diffBetween(ref1: string, ref2: string, path?: string): Promise<string | Change[]> {
return path ? this._repository.diffBetween(ref1, ref2, path) : this._repository.diffBetween(ref1, ref2);
return this._repository.diffBetween(ref1, ref2, path);
}

hashObject(data: string): Promise<string> {
Expand Down Expand Up @@ -194,8 +194,8 @@ export class ApiRepository implements Repository {
return this._repository.pushTo(remoteName, branchName, setUpstream);
}

getLog(options?: GitLogOptions): Promise<Commit[]> {
return this._repository.getLog(options);
log(options?: LogOptions): Promise<Commit[]> {
return this._repository.log(options);
}
}

Expand Down
8 changes: 6 additions & 2 deletions extensions/git/src/api/git.d.ts
View file
Expand Up @@ -110,7 +110,11 @@ export interface RepositoryUIState {
readonly onDidChange: Event<void>;
}

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;
}

Expand Down Expand Up @@ -167,7 +171,7 @@ export interface Repository {
pull(): Promise<void>;
push(remoteName?: string, branchName?: string, setUpstream?: boolean): Promise<void>;

getLog(options?: GitLogOptions): Promise<Commit[]>;
log(options?: LogOptions): Promise<Commit[]>;
}

export interface API {
Expand Down
44 changes: 31 additions & 13 deletions extensions/git/src/git.ts
View file
Expand Up @@ -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<string, string | null, string>(fs.readFile);

Expand Down Expand Up @@ -700,33 +700,36 @@ export class Repository {
});
}

async getLog(options?: GitLogOptions): Promise<Commit[]> {
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<Commit[]> {
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;
Expand Down Expand Up @@ -886,7 +889,10 @@ export class Repository {
return result.stdout;
}

async diffWithHEAD(path?: string): Promise<string | Change[]> {
diffWithHEAD(): Promise<Change[]>;
diffWithHEAD(path: string): Promise<string>;
diffWithHEAD(path?: string | undefined): Promise<string | Change[]>;
async diffWithHEAD(path?: string | undefined): Promise<string | Change[]> {
if (!path) {
return await this.diffFiles(false);
}
Expand All @@ -896,6 +902,9 @@ export class Repository {
return result.stdout;
}

diffWith(ref: string): Promise<Change[]>;
diffWith(ref: string, path: string): Promise<string>;
diffWith(ref: string, path?: string | undefined): Promise<string | Change[]>;
async diffWith(ref: string, path?: string): Promise<string | Change[]> {
if (!path) {
return await this.diffFiles(false, ref);
Expand All @@ -906,6 +915,9 @@ export class Repository {
return result.stdout;
}

diffIndexWithHEAD(): Promise<Change[]>;
diffIndexWithHEAD(path: string): Promise<string>;
diffIndexWithHEAD(path?: string | undefined): Promise<string | Change[]>;
async diffIndexWithHEAD(path?: string): Promise<string | Change[]> {
if (!path) {
return await this.diffFiles(true);
Expand All @@ -916,6 +928,9 @@ export class Repository {
return result.stdout;
}

diffIndexWith(ref: string): Promise<Change[]>;
diffIndexWith(ref: string, path: string): Promise<string>;
diffIndexWith(ref: string, path?: string | undefined): Promise<string | Change[]>;
async diffIndexWith(ref: string, path?: string): Promise<string | Change[]> {
if (!path) {
return await this.diffFiles(true, ref);
Expand All @@ -932,6 +947,9 @@ export class Repository {
return result.stdout;
}

diffBetween(ref1: string, ref2: string): Promise<Change[]>;
diffBetween(ref1: string, ref2: string, path: string): Promise<string>;
diffBetween(ref1: string, ref2: string, path?: string | undefined): Promise<string | Change[]>;
async diffBetween(ref1: string, ref2: string, path?: string): Promise<string | Change[]> {
const range = `${ref1}...${ref2}`;
if (!path) {
Expand Down
11 changes: 8 additions & 3 deletions extensions/git/src/repository.ts
View file
Expand Up @@ -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));

Expand Down Expand Up @@ -706,8 +706,8 @@ export class Repository implements Disposable {
return this.run(Operation.Config, () => this.repository.config('local', key, value));
}

getLog(options?: GitLogOptions): Promise<Commit[]> {
return this.run(Operation.Log, () => this.repository.getLog(options));
log(options?: LogOptions): Promise<Commit[]> {
return this.run(Operation.Log, () => this.repository.log(options));
}

@throttle
Expand All @@ -721,24 +721,28 @@ export class Repository implements Disposable {

diffWithHEAD(): Promise<Change[]>;
diffWithHEAD(path: string): Promise<string>;
diffWithHEAD(path?: string | undefined): Promise<string | Change[]>;
diffWithHEAD(path?: string | undefined): Promise<string | Change[]> {
return this.run(Operation.Diff, () => this.repository.diffWithHEAD(path));
}

diffWith(ref: string): Promise<Change[]>;
diffWith(ref: string, path: string): Promise<string>;
diffWith(ref: string, path?: string | undefined): Promise<string | Change[]>;
diffWith(ref: string, path?: string): Promise<string | Change[]> {
return this.run(Operation.Diff, () => this.repository.diffWith(ref, path));
}

diffIndexWithHEAD(): Promise<Change[]>;
diffIndexWithHEAD(path: string): Promise<string>;
diffIndexWithHEAD(path?: string | undefined): Promise<string | Change[]>;
diffIndexWithHEAD(path?: string): Promise<string | Change[]> {
return this.run(Operation.Diff, () => this.repository.diffIndexWithHEAD(path));
}

diffIndexWith(ref: string): Promise<Change[]>;
diffIndexWith(ref: string, path: string): Promise<string>;
diffIndexWith(ref: string, path?: string | undefined): Promise<string | Change[]>;
diffIndexWith(ref: string, path?: string): Promise<string | Change[]> {
return this.run(Operation.Diff, () => this.repository.diffIndexWith(ref, path));
}
Expand All @@ -749,6 +753,7 @@ export class Repository implements Disposable {

diffBetween(ref1: string, ref2: string): Promise<Change[]>;
diffBetween(ref1: string, ref2: string, path: string): Promise<string>;
diffBetween(ref1: string, ref2: string, path?: string | undefined): Promise<string | Change[]>;
diffBetween(ref1: string, ref2: string, path?: string): Promise<string | Change[]> {
return this.run(Operation.Diff, () => this.repository.diffBetween(ref1, ref2, path));
}
Expand Down

0 comments on commit 04c3dde

Please sign in to comment.