Skip to content

SCM - cleanup SourceControlHistoryProvider api #227376

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
lszomoru merged 5 commits into from
Sep 2, 2024
Merged

SCM - cleanup SourceControlHistoryProvider api #227376

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
fb92b22
Remove provideHistoryItemSummary
lszomoru Sep 2, 2024
d7020ec
Remove resolveHistoryItemGroupCommonAncestor
lszomoru Sep 2, 2024
f7e17fa
Remove provideHistoryItems
lszomoru Sep 2, 2024
f4b3dfd
Rename methods and remove unused code
lszomoru Sep 2, 2024
6c26170
Remove duplicated code
lszomoru Sep 2, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions extensions/git/src/decorationProvider.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -224,12 +224,12 @@ class GitIncomingChangesFileDecorationProvider implements FileDecorationProvider
return [];
}

const ancestor = await historyProvider.resolveHistoryItemGroupCommonAncestor(currentHistoryItemGroup.id, currentHistoryItemGroup.remote.id);
const ancestor = await historyProvider.resolveHistoryItemGroupCommonAncestor([currentHistoryItemGroup.id, currentHistoryItemGroup.remote.id]);
if (!ancestor) {
return [];
}

const changes = await this.repository.diffBetween(ancestor.id, currentHistoryItemGroup.remote.id);
const changes = await this.repository.diffBetween(ancestor, currentHistoryItemGroup.remote.id);
return changes;
} catch (err) {
return [];
Expand Down
20 changes: 1 addition & 19 deletions extensions/git/src/git.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2647,7 +2647,7 @@ export class Repository {

async getDefaultBranch(): Promise<Branch> {
const result = await this.exec(['symbolic-ref', '--short', 'refs/remotes/origin/HEAD']);
if (!result.stdout) {
if (!result.stdout || result.stderr) {
throw new Error('No default branch');
}

Expand Down Expand Up @@ -2714,24 +2714,6 @@ export class Repository {
return commits[0];
}

async getCommitFiles(ref: string): Promise<string[]> {
const result = await this.exec(['diff-tree', '--no-commit-id', '--name-only', '-r', ref]);
return result.stdout.split('\n').filter(l => !!l);
}

async getCommitCount(range: string): Promise<{ ahead: number; behind: number }> {
const args = ['rev-list', '--count', '--left-right', range];

if (isWindows) {
args.splice(0, 0, '-c', 'core.longpaths=true');
}

const result = await this.exec(args);
const [ahead, behind] = result.stdout.trim().split('\t');

return { ahead: Number(ahead) || 0, behind: Number(behind) || 0 };
}

async revParse(ref: string): Promise<string | undefined> {
try {
const result = await fs.readFile(path.join(this.dotGit.path, ref), 'utf8');
Expand Down
108 changes: 5 additions & 103 deletions extensions/git/src/historyProvider.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import { Disposable, Event, EventEmitter, FileDecoration, FileDecorationProvider
import { Repository, Resource } from './repository';
import { IDisposable, dispose } from './util';
import { toGitUri } from './uri';
import { Branch, LogOptions, RefType, UpstreamRef } from './api/git';
import { Branch, LogOptions, RefType } from './api/git';
import { emojify, ensureEmojis } from './emoji';
import { Commit } from './git';

Expand Down Expand Up @@ -97,42 +97,7 @@ export class GitHistoryProvider implements SourceControlHistoryProvider, FileDec
this.logger.trace(`[GitHistoryProvider][onDidRunGitStatus] currentHistoryItemGroup: ${JSON.stringify(this.currentHistoryItemGroup)}`);
}

async provideHistoryItems(historyItemGroupId: string, options: SourceControlHistoryOptions): Promise<SourceControlHistoryItem[]> {
//TODO@lszomoru - support limit and cursor
if (typeof options.limit === 'number') {
throw new Error('Unsupported options.');
}
if (typeof options.limit?.id !== 'string') {
throw new Error('Unsupported options.');
}

const refParentId = options.limit.id;
const refId = await this.repository.revParse(historyItemGroupId) ?? '';

const historyItems: SourceControlHistoryItem[] = [];
const commits = await this.repository.log({ range: `${refParentId}..${refId}`, shortStats: true, sortByAuthorDate: true });

await ensureEmojis();

historyItems.push(...commits.map(commit => {
const newLineIndex = commit.message.indexOf('\n');
const subject = newLineIndex !== -1 ? commit.message.substring(0, newLineIndex) : commit.message;

return {
id: commit.hash,
parentIds: commit.parents,
message: emojify(subject),
author: commit.authorName,
icon: new ThemeIcon('git-commit'),
timestamp: commit.authorDate?.getTime(),
statistics: commit.shortStat ?? { files: 0, insertions: 0, deletions: 0 },
};
}));

return historyItems;
}

async provideHistoryItems2(options: SourceControlHistoryOptions): Promise<SourceControlHistoryItem[]> {
async provideHistoryItems(options: SourceControlHistoryOptions): Promise<SourceControlHistoryItem[]> {
if (!this.currentHistoryItemGroup || !options.historyItemGroupIds) {
return [];
}
Expand Down Expand Up @@ -180,18 +145,11 @@ export class GitHistoryProvider implements SourceControlHistoryProvider, FileDec
};
});
} catch (err) {
this.logger.error(`[GitHistoryProvider][provideHistoryItems2] Failed to get history items with options '${JSON.stringify(options)}': ${err}`);
this.logger.error(`[GitHistoryProvider][provideHistoryItems] Failed to get history items with options '${JSON.stringify(options)}': ${err}`);
return [];
}
}

async provideHistoryItemSummary(historyItemId: string, historyItemParentId: string | undefined): Promise<SourceControlHistoryItem> {
historyItemParentId = historyItemParentId ?? await this.repository.getEmptyTree();
const allChanges = await this.repository.diffBetweenShortStat(historyItemParentId, historyItemId);

return { id: historyItemId, parentIds: [historyItemParentId], message: '', statistics: allChanges };
}

async provideHistoryItemChanges(historyItemId: string, historyItemParentId: string | undefined): Promise<SourceControlHistoryItemChange[]> {
historyItemParentId = historyItemParentId ?? await this.repository.getEmptyTree();

Expand Down Expand Up @@ -226,35 +184,7 @@ export class GitHistoryProvider implements SourceControlHistoryProvider, FileDec
return historyItemChanges;
}

async resolveHistoryItemGroupCommonAncestor(historyItemId1: string, historyItemId2: string | undefined): Promise<{ id: string; ahead: number; behind: number } | undefined> {
if (!historyItemId2) {
const upstreamRef = await this.resolveHistoryItemGroupMergeBase(historyItemId1);
if (!upstreamRef) {
this.logger.info(`[GitHistoryProvider][resolveHistoryItemGroupCommonAncestor] Failed to resolve history item group base for '${historyItemId1}'`);
return undefined;
}

historyItemId2 = `refs/remotes/${upstreamRef.remote}/${upstreamRef.name}`;
}

const ancestor = await this.repository.getMergeBase(historyItemId1, historyItemId2);
if (!ancestor) {
this.logger.info(`[GitHistoryProvider][resolveHistoryItemGroupCommonAncestor] Failed to resolve common ancestor for '${historyItemId1}' and '${historyItemId2}'`);
return undefined;
}

try {
const commitCount = await this.repository.getCommitCount(`${historyItemId1}...${historyItemId2}`);
this.logger.trace(`[GitHistoryProvider][resolveHistoryItemGroupCommonAncestor] Resolved common ancestor for '${historyItemId1}' and '${historyItemId2}': ${JSON.stringify({ id: ancestor, ahead: commitCount.ahead, behind: commitCount.behind })}`);
return { id: ancestor, ahead: commitCount.ahead, behind: commitCount.behind };
} catch (err) {
this.logger.error(`[GitHistoryProvider][resolveHistoryItemGroupCommonAncestor] Failed to get ahead/behind for '${historyItemId1}...${historyItemId2}': ${err.message}`);
}

return undefined;
}

async resolveHistoryItemGroupCommonAncestor2(historyItemGroupIds: string[]): Promise<string | undefined> {
async resolveHistoryItemGroupCommonAncestor(historyItemGroupIds: string[]): Promise<string | undefined> {
try {
if (historyItemGroupIds.length === 0) {
// TODO@lszomoru - log
Expand Down Expand Up @@ -283,7 +213,7 @@ export class GitHistoryProvider implements SourceControlHistoryProvider, FileDec
}
}
catch (err) {
this.logger.error(`[GitHistoryProvider][resolveHistoryItemGroupCommonAncestor2] Failed to resolve common ancestor for ${historyItemGroupIds.join(',')}: ${err}`);
this.logger.error(`[GitHistoryProvider][resolveHistoryItemGroupCommonAncestor] Failed to resolve common ancestor for ${historyItemGroupIds.join(',')}: ${err}`);
}

return undefined;
Expand Down Expand Up @@ -311,34 +241,6 @@ export class GitHistoryProvider implements SourceControlHistoryProvider, FileDec
return labels;
}

private async resolveHistoryItemGroupMergeBase(historyItemId: string): Promise<UpstreamRef | undefined> {
try {
// Upstream
const branch = await this.repository.getBranch(historyItemId);
if (branch.upstream) {
return branch.upstream;
}

// Base (config -> reflog -> default)
const remoteBranch = await this.repository.getBranchBase(historyItemId);
if (!remoteBranch?.remote || !remoteBranch?.name || !remoteBranch?.commit || remoteBranch?.type !== RefType.RemoteHead) {
this.logger.info(`[GitHistoryProvider][resolveHistoryItemGroupUpstreamOrBase] Failed to resolve history item group base for '${historyItemId}'`);
return undefined;
}

return {
name: remoteBranch.name,
remote: remoteBranch.remote,
commit: remoteBranch.commit
};
}
catch (err) {
this.logger.error(`[GitHistoryProvider][resolveHistoryItemGroupUpstreamOrBase] Failed to get branch base for '${historyItemId}': ${err.message}`);
}

return undefined;
}

private async resolveHEADMergeBase(): Promise<Branch | undefined> {
if (this.repository.HEAD?.type !== RefType.Head || !this.repository.HEAD?.name) {
return undefined;
Expand Down
23 changes: 6 additions & 17 deletions extensions/git/src/repository.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1508,16 +1508,13 @@ export class Repository implements Disposable {

private async getDefaultBranch(): Promise<Branch | undefined> {
try {
const defaultBranchResult = await this.repository.exec(['symbolic-ref', '--short', 'refs/remotes/origin/HEAD']);
if (defaultBranchResult.stdout.trim() === '' || defaultBranchResult.stderr) {
return undefined;
}

return this.getBranch(defaultBranchResult.stdout.trim());
const defaultBranch = await this.repository.getDefaultBranch();
return defaultBranch;
}
catch (err) {
this.logger.warn(`[Repository][getDefaultBranch] Failed to get default branch details: ${err.message}.`);
return undefined;
}
catch (err) { }

return undefined;
}

private async getUpstreamBranch(branch: Branch): Promise<Branch | undefined> {
Expand Down Expand Up @@ -1618,14 +1615,6 @@ export class Repository implements Disposable {
return this._EMPTY_TREE;
}

async getCommitCount(range: string): Promise<{ ahead: number; behind: number }> {
return await this.run(Operation.RevList, () => this.repository.getCommitCount(range));
}

async revParse(ref: string): Promise<string | undefined> {
return await this.run(Operation.RevParse, () => this.repository.revParse(ref));
}

async reset(treeish: string, hard?: boolean): Promise<void> {
await this.run(Operation.Reset, () => this.repository.reset(treeish, hard));
}
Expand Down
22 changes: 4 additions & 18 deletions src/vs/workbench/api/browser/mainThreadSCM.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -170,29 +170,15 @@ class MainThreadSCMHistoryProvider implements ISCMHistoryProvider {

constructor(private readonly proxy: ExtHostSCMShape, private readonly handle: number) { }

async resolveHistoryItemGroupCommonAncestor(historyItemGroupId1: string, historyItemGroupId2: string | undefined): Promise<{ id: string; ahead: number; behind: number } | undefined> {
return this.proxy.$resolveHistoryItemGroupCommonAncestor(this.handle, historyItemGroupId1, historyItemGroupId2, CancellationToken.None);
async resolveHistoryItemGroupCommonAncestor(historyItemGroupIds: string[]): Promise<string | undefined> {
return this.proxy.$resolveHistoryItemGroupCommonAncestor(this.handle, historyItemGroupIds, CancellationToken.None);
}

async resolveHistoryItemGroupCommonAncestor2(historyItemGroupIds: string[]): Promise<string | undefined> {
return this.proxy.$resolveHistoryItemGroupCommonAncestor2(this.handle, historyItemGroupIds, CancellationToken.None);
}

async provideHistoryItems(historyItemGroupId: string, options: ISCMHistoryOptions): Promise<ISCMHistoryItem[] | undefined> {
const historyItems = await this.proxy.$provideHistoryItems(this.handle, historyItemGroupId, options, CancellationToken.None);
async provideHistoryItems(options: ISCMHistoryOptions): Promise<ISCMHistoryItem[] | undefined> {
const historyItems = await this.proxy.$provideHistoryItems(this.handle, options, CancellationToken.None);
return historyItems?.map(historyItem => toISCMHistoryItem(historyItem));
}

async provideHistoryItems2(options: ISCMHistoryOptions): Promise<ISCMHistoryItem[] | undefined> {
const historyItems = await this.proxy.$provideHistoryItems2(this.handle, options, CancellationToken.None);
return historyItems?.map(historyItem => toISCMHistoryItem(historyItem));
}

async provideHistoryItemSummary(historyItemId: string, historyItemParentId: string | undefined): Promise<ISCMHistoryItem | undefined> {
const historyItem = await this.proxy.$provideHistoryItemSummary(this.handle, historyItemId, historyItemParentId, CancellationToken.None);
return historyItem ? toISCMHistoryItem(historyItem) : undefined;
}

async provideHistoryItemChanges(historyItemId: string, historyItemParentId: string | undefined): Promise<ISCMHistoryItemChange[] | undefined> {
const changes = await this.proxy.$provideHistoryItemChanges(this.handle, historyItemId, historyItemParentId, CancellationToken.None);
return changes?.map(change => ({
Expand Down
7 changes: 2 additions & 5 deletions src/vs/workbench/api/common/extHost.protocol.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2351,12 +2351,9 @@ export interface ExtHostSCMShape {
$executeResourceCommand(sourceControlHandle: number, groupHandle: number, handle: number, preserveFocus: boolean): Promise<void>;
$validateInput(sourceControlHandle: number, value: string, cursorPosition: number): Promise<[string | IMarkdownString, number] | undefined>;
$setSelectedSourceControl(selectedSourceControlHandle: number | undefined): Promise<void>;
$provideHistoryItems(sourceControlHandle: number, historyItemGroupId: string, options: any, token: CancellationToken): Promise<SCMHistoryItemDto[] | undefined>;
$provideHistoryItems2(sourceControlHandle: number, options: any, token: CancellationToken): Promise<SCMHistoryItemDto[] | undefined>;
$provideHistoryItemSummary(sourceControlHandle: number, historyItemId: string, historyItemParentId: string | undefined, token: CancellationToken): Promise<SCMHistoryItemDto | undefined>;
$provideHistoryItems(sourceControlHandle: number, options: any, token: CancellationToken): Promise<SCMHistoryItemDto[] | undefined>;
$provideHistoryItemChanges(sourceControlHandle: number, historyItemId: string, historyItemParentId: string | undefined, token: CancellationToken): Promise<SCMHistoryItemChangeDto[] | undefined>;
$resolveHistoryItemGroupCommonAncestor(sourceControlHandle: number, historyItemGroupId1: string, historyItemGroupId2: string | undefined, token: CancellationToken): Promise<{ id: string; ahead: number; behind: number } | undefined>;
$resolveHistoryItemGroupCommonAncestor2(sourceControlHandle: number, historyItemGroupIds: string[], token: CancellationToken): Promise<string | undefined>;
$resolveHistoryItemGroupCommonAncestor(sourceControlHandle: number, historyItemGroupIds: string[], token: CancellationToken): Promise<string | undefined>;
}

export interface ExtHostQuickDiffShape {
Expand Down
30 changes: 4 additions & 26 deletions src/vs/workbench/api/common/extHostSCM.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -975,40 +975,18 @@ export class ExtHostSCM implements ExtHostSCMShape {
return Promise.resolve(undefined);
}

async $resolveHistoryItemGroupCommonAncestor(sourceControlHandle: number, historyItemGroupId1: string, historyItemGroupId2: string | undefined, token: CancellationToken): Promise<{ id: string; ahead: number; behind: number } | undefined> {
async $resolveHistoryItemGroupCommonAncestor(sourceControlHandle: number, historyItemGroupIds: string[], token: CancellationToken): Promise<string | undefined> {
const historyProvider = this._sourceControls.get(sourceControlHandle)?.historyProvider;
return await historyProvider?.resolveHistoryItemGroupCommonAncestor(historyItemGroupId1, historyItemGroupId2, token) ?? undefined;
return await historyProvider?.resolveHistoryItemGroupCommonAncestor(historyItemGroupIds, token) ?? undefined;
}

async $resolveHistoryItemGroupCommonAncestor2(sourceControlHandle: number, historyItemGroupIds: string[], token: CancellationToken): Promise<string | undefined> {
async $provideHistoryItems(sourceControlHandle: number, options: any, token: CancellationToken): Promise<SCMHistoryItemDto[] | undefined> {
const historyProvider = this._sourceControls.get(sourceControlHandle)?.historyProvider;
return await historyProvider?.resolveHistoryItemGroupCommonAncestor2(historyItemGroupIds, token) ?? undefined;
}

async $provideHistoryItems(sourceControlHandle: number, historyItemGroupId: string, options: any, token: CancellationToken): Promise<SCMHistoryItemDto[] | undefined> {
const historyProvider = this._sourceControls.get(sourceControlHandle)?.historyProvider;
const historyItems = await historyProvider?.provideHistoryItems(historyItemGroupId, options, token);
const historyItems = await historyProvider?.provideHistoryItems(options, token);

return historyItems?.map(item => toSCMHistoryItemDto(item)) ?? undefined;
}

async $provideHistoryItems2(sourceControlHandle: number, options: any, token: CancellationToken): Promise<SCMHistoryItemDto[] | undefined> {
const historyProvider = this._sourceControls.get(sourceControlHandle)?.historyProvider;
const historyItems = await historyProvider?.provideHistoryItems2(options, token);

return historyItems?.map(item => toSCMHistoryItemDto(item)) ?? undefined;
}

async $provideHistoryItemSummary(sourceControlHandle: number, historyItemId: string, historyItemParentId: string | undefined, token: CancellationToken): Promise<SCMHistoryItemDto | undefined> {
const historyProvider = this._sourceControls.get(sourceControlHandle)?.historyProvider;
if (typeof historyProvider?.provideHistoryItemSummary !== 'function') {
return undefined;
}

const historyItem = await historyProvider.provideHistoryItemSummary(historyItemId, historyItemParentId, token);
return historyItem ? toSCMHistoryItemDto(historyItem) : undefined;
}

async $provideHistoryItemChanges(sourceControlHandle: number, historyItemId: string, historyItemParentId: string | undefined, token: CancellationToken): Promise<SCMHistoryItemChangeDto[] | undefined> {
const historyProvider = this._sourceControls.get(sourceControlHandle)?.historyProvider;
return await historyProvider?.provideHistoryItemChanges(historyItemId, historyItemParentId, token) ?? undefined;
Expand Down
4 changes: 2 additions & 2 deletions src/vs/workbench/contrib/scm/browser/scmHistoryViewPane.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,7 @@ registerAction2(class extends Action2 {
const historyProvider = provider.historyProvider.get();

if (historyItems.length > 1) {
const ancestor = await historyProvider?.resolveHistoryItemGroupCommonAncestor2([historyItem.id, historyItemLast.id]);
const ancestor = await historyProvider?.resolveHistoryItemGroupCommonAncestor([historyItem.id, historyItemLast.id]);
if (!ancestor || (ancestor !== historyItem.id && ancestor !== historyItemLast.id)) {
return;
}
Expand Down Expand Up @@ -674,7 +674,7 @@ class SCMHistoryViewModel extends Disposable {
const existingHistoryItems = state?.items ?? [];
const limit = clamp(this._configurationService.getValue<number>('scm.graph.pageSize'), 1, 1000);

const historyItems = await historyProvider.provideHistoryItems2({
const historyItems = await historyProvider.provideHistoryItems({
historyItemGroupIds, limit, skip: existingHistoryItems.length
}) ?? [];

Expand Down
7 changes: 2 additions & 5 deletions src/vs/workbench/contrib/scm/common/history.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,12 +22,9 @@ export interface ISCMHistoryProvider {
readonly currentHistoryItemGroupRemoteId: IObservable<string | undefined>;
readonly currentHistoryItemGroupRemoteRevision: IObservable<string | undefined>;

provideHistoryItems(historyItemGroupId: string, options: ISCMHistoryOptions): Promise<ISCMHistoryItem[] | undefined>;
provideHistoryItems2(options: ISCMHistoryOptions): Promise<ISCMHistoryItem[] | undefined>;
provideHistoryItemSummary(historyItemId: string, historyItemParentId: string | undefined): Promise<ISCMHistoryItem | undefined>;
provideHistoryItems(options: ISCMHistoryOptions): Promise<ISCMHistoryItem[] | undefined>;
provideHistoryItemChanges(historyItemId: string, historyItemParentId: string | undefined): Promise<ISCMHistoryItemChange[] | undefined>;
resolveHistoryItemGroupCommonAncestor(historyItemGroupId1: string, historyItemGroupId2: string | undefined): Promise<{ id: string; ahead: number; behind: number } | undefined>;
resolveHistoryItemGroupCommonAncestor2(historyItemGroupIds: string[]): Promise<string | undefined>;
resolveHistoryItemGroupCommonAncestor(historyItemGroupIds: string[]): Promise<string | undefined>;
}

export interface ISCMHistoryOptions {
Expand Down
Loading