Reworks data paging using `--skip`

Streams git log data to find specific commit

src/env/node/git/git.ts

@@ -1,9 +1,11 @@
+import type { ChildProcess, SpawnOptions } from 'child_process';
+import { spawn } from 'child_process';
 import * as process from 'process';
 import type { CancellationToken } from 'vscode';
 import { Uri, window, workspace } from 'vscode';
 import { hrtime } from '@env/hrtime';
 import { GlyphChars } from '../../../constants';
-import type { GitCommandOptions } from '../../../git/commandOptions';
+import type { GitCommandOptions, GitSpawnOptions } from '../../../git/commandOptions';
 import { GitErrorHandling } from '../../../git/commandOptions';
 import type { GitDiffFilter } from '../../../git/models/diff';
 import { GitRevision } from '../../../git/models/reference';
@@ -23,6 +25,17 @@ import { fsExists, run, RunError } from './shell';
 const emptyArray = Object.freeze([]) as unknown as any[];
 const emptyObj = Object.freeze({});
 
+const gitBranchDefaultConfigs = Object.freeze(['-c', 'color.branch=false']);
+const gitDiffDefaultConfigs = Object.freeze(['-c', 'color.diff=false']);
+const gitLogDefaultConfigs = Object.freeze(['-c', 'log.showSignature=false']);
+export const gitLogDefaultConfigsWithFiles = Object.freeze([
+	'-c',
+	'log.showSignature=false',
+	'-c',
+	'diff.renameLimit=0',
+]);
+const gitStatusDefaultConfigs = Object.freeze(['-c', 'color.status=false']);
+
 export const maxGitCliLength = 30000;
 
 const textDecoder = new TextDecoder('utf8');
@@ -91,7 +104,7 @@ function defaultExceptionHandler(ex: Error, cwd: string | undefined, start?: [nu
 type ExitCodeOnlyGitCommandOptions = GitCommandOptions & { exitCodeOnly: true };
 
 export class Git {
-	// A map of running git commands -- avoids running duplicate overlaping commands
+	/** Map of running git commands -- avoids running duplicate overlaping commands */
 	private readonly pendingCommands = new Map<string, Promise<string | Buffer>>();
 
 	async git(options: ExitCodeOnlyGitCommandOptions, ...args: any[]): Promise<number>;
@@ -199,6 +212,84 @@ export class Git {
 		}
 	}
 
+	async gitSpawn(options: GitSpawnOptions, ...args: any[]): Promise<ChildProcess> {
+		const start = hrtime();
+
+		const { cancellation, configs, stdin, stdinEncoding, ...opts } = options;
+
+		const spawnOpts: SpawnOptions = {
+			// Unless provided, ignore stdin and leave default streams for stdout and stderr
+			stdio: [stdin ? 'pipe' : 'ignore', null, null],
+			...opts,
+			// Adds GCM environment variables to avoid any possible credential issues -- from https://github.com/Microsoft/vscode/issues/26573#issuecomment-338686581
+			// Shouldn't *really* be needed but better safe than sorry
+			env: {
+				...process.env,
+				...(options.env ?? emptyObj),
+				GCM_INTERACTIVE: 'NEVER',
+				GCM_PRESERVE_CREDS: 'TRUE',
+				LC_ALL: 'C',
+			},
+		};
+
+		const gitCommand = `[${spawnOpts.cwd as string}] git ${args.join(' ')}`;
+
+		// Fixes https://github.com/gitkraken/vscode-gitlens/issues/73 & https://github.com/gitkraken/vscode-gitlens/issues/161
+		// See https://stackoverflow.com/questions/4144417/how-to-handle-asian-characters-in-file-names-in-git-on-os-x
+		args.splice(
+			0,
+			0,
+			'-c',
+			'core.quotepath=false',
+			'-c',
+			'color.ui=false',
+			...(configs !== undefined ? configs : emptyArray),
+		);
+
+		if (process.platform === 'win32') {
+			args.splice(0, 0, '-c', 'core.longpaths=true');
+		}
+
+		if (cancellation) {
+			const controller = new AbortController();
+			spawnOpts.signal = controller.signal;
+			cancellation.onCancellationRequested(() => controller.abort());
+		}
+
+		const proc = spawn(await this.path(), args, spawnOpts);
+		if (stdin) {
+			proc.stdin?.end(stdin, (stdinEncoding ?? 'utf8') as BufferEncoding);
+		}
+
+		let exception: Error | undefined;
+		proc.once('error', e => (exception = e));
+		proc.once('exit', () => {
+			const duration = getDurationMilliseconds(start);
+			const slow = duration > Logger.slowCallWarningThreshold;
+			const status = slow ? ' (slow)' : '';
+
+			if (exception != null) {
+				Logger.error(
+					'',
+					`[SGIT ] ${gitCommand} ${GlyphChars.Dot} ${(exception.message || String(exception) || '')
+						.trim()
+						.replace(/fatal: /g, '')
+						.replace(/\r?\n|\r/g, ` ${GlyphChars.Dot} `)} ${GlyphChars.Dot} ${duration} ms${status}`,
+				);
+			} else if (slow) {
+				Logger.warn(`[SGIT ] ${gitCommand} ${GlyphChars.Dot} ${duration} ms${status}`);
+			} else {
+				Logger.log(`[SGIT ] ${gitCommand} ${GlyphChars.Dot} ${duration} ms${status}`);
+			}
+			Logger.logGitCommand(
+				`${gitCommand}${exception != null ? ` ${GlyphChars.Dot} FAILED` : ''}`,
+				duration,
+				exception,
+			);
+		});
+		return proc;
+	}
+
 	private gitLocator!: () => Promise<GitLocation>;
 	setLocator(locator: () => Promise<GitLocation>): void {
 		this.gitLocator = locator;
@@ -355,7 +446,7 @@ export class Git {
 		}
 
 		return this.git<string>(
-			{ cwd: repoPath, configs: ['-c', 'color.branch=false'], errors: GitErrorHandling.Ignore },
+			{ cwd: repoPath, configs: gitBranchDefaultConfigs, errors: GitErrorHandling.Ignore },
 			...params,
 		);
 	}
@@ -472,7 +563,7 @@ export class Git {
 			return await this.git<string>(
 				{
 					cwd: repoPath,
-					configs: ['-c', 'color.diff=false'],
+					configs: gitDiffDefaultConfigs,
 					encoding: options.encoding,
 				},
 				...params,
@@ -525,7 +616,7 @@ export class Git {
 			return await this.git<string>(
 				{
 					cwd: repoPath,
-					configs: ['-c', 'color.diff=false'],
+					configs: gitDiffDefaultConfigs,
 					encoding: options.encoding,
 					stdin: contents,
 				},
@@ -577,7 +668,7 @@ export class Git {
 			params.push(ref2);
 		}
 
-		return this.git<string>({ cwd: repoPath, configs: ['-c', 'color.diff=false'] }, ...params, '--');
+		return this.git<string>({ cwd: repoPath, configs: gitDiffDefaultConfigs }, ...params, '--');
 	}
 
 	async diff__shortstat(repoPath: string, ref?: string) {
@@ -587,7 +678,7 @@ export class Git {
 		}
 
 		try {
-			return await this.git<string>({ cwd: repoPath, configs: ['-c', 'color.diff=false'] }, ...params, '--');
+			return await this.git<string>({ cwd: repoPath, configs: gitDiffDefaultConfigs }, ...params, '--');
 		} catch (ex) {
 			const msg: string = ex?.toString() ?? '';
 			if (GitErrors.noMergeBase.test(msg)) {
@@ -762,29 +853,104 @@ export class Git {
 			params.push(ref);
 		}
 
+		return this.git<string>({ cwd: repoPath, configs: gitLogDefaultConfigsWithFiles }, ...params, '--');
+	}
+
+	log2(repoPath: string, options?: { configs?: readonly string[]; ref?: string; stdin?: string }, ...args: string[]) {
+		const params = ['log'];
+		if (options?.stdin) {
+			params.push('--stdin');
+		}
+		params.push(...args);
+
+		if (options?.ref && !GitRevision.isUncommittedStaged(options.ref)) {
+			params.push(options?.ref);
+		}
+
 		return this.git<string>(
-			{ cwd: repoPath, configs: ['-c', 'diff.renameLimit=0', '-c', 'log.showSignature=false'] },
+			{ cwd: repoPath, configs: options?.configs ?? gitLogDefaultConfigs, stdin: options?.stdin },
 			...params,
 			'--',
 		);
 	}
 
-	log2(repoPath: string, ref: string | undefined, stdin: string | undefined, ...args: unknown[]) {
-		const params = ['log', ...args];
-
-		if (ref && !GitRevision.isUncommittedStaged(ref)) {
-			params.push(ref);
-		}
-
-		if (stdin) {
+	async logStream(
+		repoPath: string,
+		sha: string,
+		limit: number,
+		options?: { configs?: readonly string[]; stdin?: string },
+		...args: string[]
+	): Promise<[data: string, count: number]> {
+		const params = ['log'];
+		if (options?.stdin) {
 			params.push('--stdin');
 		}
+		params.push(...args);
 
-		return this.git<string>(
-			{ cwd: repoPath, configs: ['-c', 'diff.renameLimit=0', '-c', 'log.showSignature=false'], stdin: stdin },
+		const proc = await this.gitSpawn(
+			{ cwd: repoPath, configs: options?.configs ?? gitLogDefaultConfigs, stdin: options?.stdin },
 			...params,
 			'--',
 		);
+
+		const shaRegex = new RegExp(`(^${sha}\x00)|(\x00\x00${sha}\x00)`);
+
+		let found = false;
+		let count = 0;
+
+		return new Promise<[data: string, count: number]>((resolve, reject) => {
+			const errData: string[] = [];
+			const data: string[] = [];
+
+			function onErrData(s: string) {
+				errData.push(s);
+			}
+
+			function onError(e: Error) {
+				reject(e);
+			}
+
+			function onExit(exitCode: number) {
+				if (exitCode !== 0) {
+					reject(new Error(errData.join('')));
+				}
+
+				resolve([data.join(''), count]);
+			}
+
+			function onData(s: string) {
+				data.push(s);
+				// eslint-disable-next-line no-control-regex
+				count += (s.match(/\x00\x00[0-9a-f]{40}\x00/g)?.length ?? 0) + 1;
+
+				if (!found && shaRegex.test(s)) {
+					found = true;
+					// Buffer a bit past the sha we are looking for
+					if (count > limit) {
+						limit = count + 50;
+					}
+				}
+
+				if (!found || count <= limit) return;
+
+				proc.removeListener('exit', onExit);
+				proc.removeListener('error', onError);
+				proc.stdout!.removeListener('data', onData);
+				proc.stderr!.removeListener('data', onErrData);
+				proc.kill();
+
+				resolve([data.join(''), count]);
+			}
+
+			proc.on('error', onError);
+			proc.on('exit', onExit);
+
+			proc.stdout!.setEncoding('utf8');
+			proc.stdout!.on('data', onData);
+
+			proc.stderr!.setEncoding('utf8');
+			proc.stderr!.on('data', onErrData);
+		});
 	}
 
 	log__file(
@@ -901,7 +1067,7 @@ export class Git {
 			params.push('--', file);
 		}
 
-		return this.git<string>({ cwd: root, configs: ['-c', 'log.showSignature=false'] }, ...params);
+		return this.git<string>({ cwd: root, configs: gitLogDefaultConfigs }, ...params);
 	}
 
 	async log__file_recent(
@@ -933,7 +1099,7 @@ export class Git {
 			{
 				cancellation: options?.cancellation,
 				cwd: repoPath,
-				configs: ['-c', 'log.showSignature=false'],
+				configs: gitLogDefaultConfigs,
 				errors: GitErrorHandling.Ignore,
 			},
 			...params,
@@ -965,7 +1131,7 @@ export class Git {
 			{
 				cancellation: cancellation,
 				cwd: repoPath,
-				configs: ['-c', 'log.showSignature=false'],
+				configs: gitLogDefaultConfigs,
 				errors: GitErrorHandling.Ignore,
 			},
 			...params,
@@ -981,7 +1147,7 @@ export class Git {
 		}
 
 		const data = await this.git<string>(
-			{ cwd: repoPath, configs: ['-c', 'log.showSignature=false'], errors: GitErrorHandling.Ignore },
+			{ cwd: repoPath, configs: gitLogDefaultConfigs, errors: GitErrorHandling.Ignore },
 			...params,
 			'--',
 		);
@@ -997,7 +1163,7 @@ export class Git {
 		}
 
 		const data = await this.git<string>(
-			{ cwd: repoPath, configs: ['-c', 'log.showSignature=false'], errors: GitErrorHandling.Ignore },
+			{ cwd: repoPath, configs: gitLogDefaultConfigs, errors: GitErrorHandling.Ignore },
 			...params,
 			'--',
 		);
@@ -1035,7 +1201,7 @@ export class Git {
 		}
 
 		return this.git<string>(
-			{ cwd: repoPath, configs: useShow ? undefined : ['-c', 'log.showSignature=false'] },
+			{ cwd: repoPath, configs: useShow ? undefined : gitLogDefaultConfigs },
 			...params,
 			...search,
 		);
@@ -1046,7 +1212,7 @@ export class Git {
 	//     if (options.ref && !GitRevision.isUncommittedStaged(options.ref)) {
 	//         params.push(options.ref);
 	//     }
-	//     return this.git<string>({ cwd: repoPath, configs: ['-c', 'log.showSignature=false'] }, ...params, '--');
+	//     return this.git<string>({ cwd: repoPath, configs: gitLogDefaultConfigs }, ...params, '--');
 	// }
 
 	async ls_files(
@@ -1144,7 +1310,7 @@ export class Git {
 			params.push(branch);
 		}
 
-		return this.git<string>({ cwd: repoPath, configs: ['-c', 'log.showSignature=false'] }, ...params, '--');
+		return this.git<string>({ cwd: repoPath, configs: gitLogDefaultConfigs }, ...params, '--');
 	}
 
 	remote(repoPath: string): Promise<string> {
@@ -1167,14 +1333,13 @@ export class Git {
 		return this.git<string>({ cwd: repoPath }, 'reset', '-q', '--', fileName);
 	}
 
-	async rev_list__count(repoPath: string, ref: string): Promise<number | undefined> {
-		let data = await this.git<string>(
-			{ cwd: repoPath, errors: GitErrorHandling.Ignore },
-			'rev-list',
-			'--count',
-			ref,
-			'--',
-		);
+	async rev_list__count(repoPath: string, ref: string, all?: boolean): Promise<number | undefined> {
+		const params = ['rev-list', '--count'];
+		if (all) {
+			params.push('--all');
+		}
+
+		let data = await this.git<string>({ cwd: repoPath, errors: GitErrorHandling.Ignore }, ...params, ref, '--');
 		data = data.trim();
 		if (data.length === 0) return undefined;
 
@@ -1363,7 +1528,7 @@ export class Git {
 		if (GitRevision.isUncommitted(ref)) throw new Error(`ref=${ref} is uncommitted`);
 
 		const opts: GitCommandOptions = {
-			configs: ['-c', 'log.showSignature=false'],
+			configs: gitLogDefaultConfigs,
 			cwd: root,
 			encoding: options.encoding ?? 'utf8',
 			errors: GitErrorHandling.Throw,
@@ -1523,7 +1688,7 @@ export class Git {
 		}
 
 		return this.git<string>(
-			{ cwd: repoPath, configs: ['-c', 'color.status=false'], env: { GIT_OPTIONAL_LOCKS: '0' } },
+			{ cwd: repoPath, configs: gitStatusDefaultConfigs, env: { GIT_OPTIONAL_LOCKS: '0' } },
 			...params,
 			'--',
 		);
@@ -1543,7 +1708,7 @@ export class Git {
 		}
 
 		return this.git<string>(
-			{ cwd: root, configs: ['-c', 'color.status=false'], env: { GIT_OPTIONAL_LOCKS: '0' } },
+			{ cwd: root, configs: gitStatusDefaultConfigs, env: { GIT_OPTIONAL_LOCKS: '0' } },
 			...params,
 			'--',
 			file,

src/env/node/git/localGitProvider.ts

@@ -141,7 +141,7 @@ import { compare, fromString } from '../../../system/version';
 import type { CachedBlame, CachedDiff, CachedLog, TrackedDocument } from '../../../trackers/gitDocumentTracker';
 import { GitDocumentState } from '../../../trackers/gitDocumentTracker';
 import type { Git } from './git';
-import { GitErrors, maxGitCliLength } from './git';
+import { GitErrors, gitLogDefaultConfigsWithFiles, maxGitCliLength } from './git';
 import type { GitLocation } from './locator';
 import { findGitPath, InvalidGitConfigError, UnableToFindGitError } from './locator';
 import { fsExists, RunError } from './shell';
@@ -1626,31 +1626,16 @@ export class LocalGitProvider implements GitProvider, Disposable {
 		const defaultPageLimit = configuration.get('graph.pageItemLimit') ?? 1000;
 		const ordering = configuration.get('graph.commitOrdering', undefined, 'date');
 
-		const [headResult, refResult, stashResult, remotesResult] = await Promise.allSettled([
-			this.git.rev_parse(repoPath, 'HEAD'),
-			options?.ref != null && options?.ref !== 'HEAD'
-				? this.git.log2(repoPath, options.ref, undefined, ...refParser.arguments, '-n1')
-				: undefined,
+		const [refResult, stashResult, remotesResult] = await Promise.allSettled([
+			this.git.log2(repoPath, undefined, ...refParser.arguments, '-n1', options?.ref ?? 'HEAD'),
 			this.getStash(repoPath),
 			this.getRemotes(repoPath),
 		]);
 
-		let limit = defaultLimit;
-		let selectSha: string | undefined;
-		let since: string | undefined;
-
-		const commit = first(refParser.parse(getSettledValue(refResult) ?? ''));
-		const head = getSettledValue(headResult);
-		if (commit != null && commit.sha !== head) {
-			since = ordering === 'author-date' ? commit.authorDate : commit.committerDate;
-			selectSha = commit.sha;
-			limit = 0;
-		} else if (options?.ref != null && (options.ref === 'HEAD' || options.ref === head)) {
-			selectSha = head;
-		}
-
+		const limit = defaultLimit;
 		const remotes = getSettledValue(remotesResult);
 		const remoteMap = remotes != null ? new Map(remotes.map(r => [r.name, r])) : new Map();
+		const selectSha = first(refParser.parse(getSettledValue(refResult) ?? ''));
 		const skipStashParents = new Set();
 
 		let stdin: string | undefined;
@@ -1663,12 +1648,18 @@ export class LocalGitProvider implements GitProvider, Disposable {
 			);
 		}
 
+		const ids = new Set<string>();
+		let total = 0;
+		let iterations = 0;
+
 		async function getCommitsForGraphCore(
 			this: LocalGitProvider,
 			limit: number,
-			shaOrCursor?: string | { sha: string; timestamp: string },
+			shaOrCursor?: string | { sha: string; skip?: number },
 		): Promise<GitGraph> {
-			let cursor: { sha: string; timestamp: string } | undefined;
+			iterations++;
+
+			let cursor: { sha: string; skip?: number } | undefined;
 			let sha: string | undefined;
 			if (shaOrCursor != null) {
 				if (typeof shaOrCursor === 'string') {
@@ -1683,49 +1674,59 @@ export class LocalGitProvider implements GitProvider, Disposable {
 			let size;
 
 			do {
-				const args = [...parser.arguments, '-m', `--${ordering}-order`, '--all'];
+				const args = [...parser.arguments, `--${ordering}-order`, '--all'];
 
-				if (since) {
-					args.push(`--since=${since}`, '--boundary');
-					// Only allow `since` once
-					since = undefined;
+				let data;
+				if (sha) {
+					[data, limit] = await this.git.logStream(
+						repoPath,
+						sha,
+						limit,
+						stdin ? { stdin: stdin } : undefined,
+						...args,
+					);
 				} else {
 					args.push(`-n${nextPageLimit + 1}`);
-					if (cursor) {
-						args.push(`--until=${cursor.timestamp}`, '--boundary');
+					if (cursor?.skip) {
+						args.push(`--skip=${cursor.skip}`);
 					}
+
+					data = await this.git.log2(repoPath, stdin ? { stdin: stdin } : undefined, ...args);
 				}
 
-				let data = await this.git.log2(repoPath, undefined, stdin, ...args);
 				if (cursor || sha) {
-					const cursorIndex = data.startsWith(`${cursor?.sha ?? sha}\0`)
+					const cursorIndex = data.startsWith(`${cursor?.sha ?? sha}\x00`)
 						? 0
-						: data.indexOf(`\0\0${cursor?.sha ?? sha}\0`);
+						: data.indexOf(`\x00\x00${cursor?.sha ?? sha}\x00`);
 					if (cursorIndex === -1) {
 						// If we didn't find any new commits, we must have them all so return that we have everything
-						if (size === data.length) return { repoPath: repoPath, rows: [] };
+						if (size === data.length) return { repoPath: repoPath, ids: ids, rows: [] };
 
 						size = data.length;
 						nextPageLimit = (nextPageLimit === 0 ? defaultPageLimit : nextPageLimit) * 2;
+						if (cursor?.skip) {
+							cursor.skip -= Math.floor(cursor.skip * 0.1);
+						}
+
 						continue;
 					}
 
-					if (cursorIndex > 0 && cursor != null) {
-						const duplicates = data.substring(0, cursorIndex);
-						if (data.length - duplicates.length < (size ?? data.length) / 4) {
-							size = data.length;
-							nextPageLimit = (nextPageLimit === 0 ? defaultPageLimit : nextPageLimit) * 2;
-							continue;
-						}
+					// if (cursorIndex > 0 && cursor != null) {
+					// 	const duplicates = data.substring(0, cursorIndex);
+					// 	if (data.length - duplicates.length < (size ?? data.length) / 4) {
+					// 		size = data.length;
+					// 		nextPageLimit = (nextPageLimit === 0 ? defaultPageLimit : nextPageLimit) * 2;
+					// 		continue;
+					// 	}
 
-						// Substract out any duplicate commits (regex is faster than parsing and counting)
-						nextPageLimit -= (duplicates.match(/\0\0[0-9a-f]{40}\0/g)?.length ?? 0) + 1;
+					// 	// Substract out any duplicate commits (regex is faster than parsing and counting)
+					// 	nextPageLimit -= (duplicates.match(/\0\0[0-9a-f]{40}\0/g)?.length ?? 0) + 1;
 
-						data = data.substring(cursorIndex + 2);
-					}
+					// 	data = data.substring(cursorIndex + 2);
+					// }
 				}
 
-				if (!data) return { repoPath: repoPath, rows: [] };
+				if (!data) return { repoPath: repoPath, ids: ids, rows: [] };
 
 				log = data;
 				if (limit !== 0) {
@@ -1745,14 +1746,17 @@ export class LocalGitProvider implements GitProvider, Disposable {
 			let remoteName: string;
 			let isStashCommit: boolean;
 
-			let commitCount = 0;
-			const startingCursor = cursor?.sha;
+			let count = 0;
 
 			const commits = parser.parse(log);
 			for (const commit of commits) {
-				commitCount++;
-				// If we are paging, skip the first commit since its a duplicate of the last commit from the previous page
-				if (startingCursor === commit.sha || skipStashParents.has(commit.sha)) continue;
+				count++;
+				if (ids.has(commit.sha)) continue;
+
+				total++;
+				if (skipStashParents.has(commit.sha)) continue;
+
+				ids.add(commit.sha);
 
 				refHeads = [];
 				refRemoteHeads = [];
@@ -1838,25 +1842,26 @@ export class LocalGitProvider implements GitProvider, Disposable {
 			}
 
 			const last = rows[rows.length - 1];
+			const startingCursor = cursor?.sha;
 			cursor =
 				last != null
 					? {
 							sha: last.sha,
-							timestamp: String(Math.floor(last.date / 1000)),
+							skip: total - iterations,
 					  }
 					: undefined;
 
 			return {
 				repoPath: repoPath,
+				ids: ids,
+				rows: rows,
+				sha: sha,
+
 				paging: {
-					limit: limit,
-					endingCursor: cursor?.timestamp,
+					limit: limit === 0 ? count : limit,
 					startingCursor: startingCursor,
-					more: commitCount > limit,
+					more: count > limit,
 				},
-				rows: rows,
-				sha: sha ?? head,
-
 				more: async (limit: number): Promise<GitGraph | undefined> =>
 					getCommitsForGraphCore.call(this, limit, cursor),
 			};
@@ -2411,7 +2416,11 @@ export class LocalGitProvider implements GitProvider, Disposable {
 				args.push(`-n${limit + 1}`);
 			}
 
-			const data = await this.git.log2(repoPath, options?.ref, options?.stdin, ...args);
+			const data = await this.git.log2(
+				repoPath,
+				{ configs: gitLogDefaultConfigsWithFiles, ref: options?.ref, stdin: options?.stdin },
+				...args,
+			);
 
 			// const parser = GitLogParser.defaultParser;
 

src/git/commandOptions.ts

@@ -7,14 +7,14 @@ export const enum GitErrorHandling {
 
 export interface GitCommandOptions {
 	// extends RunOptions<BufferEncoding | 'buffer' | string> {
-	configs?: string[];
+	cancellation?: CancellationToken;
+	configs?: readonly string[];
 	readonly correlationKey?: string;
 	errors?: GitErrorHandling;
 	// Specifies that this command should always be executed locally if possible
 	local?: boolean;
 
 	// Below options comes from RunOptions<BufferEncoding | 'buffer' | string>
-	cancellation?: CancellationToken;
 	cwd?: string;
 	readonly env?: Record<string, any>;
 	readonly encoding?: BufferEncoding | 'buffer' | string;
@@ -22,3 +22,14 @@ export interface GitCommandOptions {
 	readonly stdin?: string | Buffer;
 	readonly stdinEncoding?: string;
 }
+
+export interface GitSpawnOptions {
+	cancellation?: CancellationToken;
+	configs?: readonly string[];
+
+	// Below options comes from SpawnOptions
+	cwd?: string;
+	readonly env?: Record<string, any>;
+	readonly stdin?: string | Buffer;
+	readonly stdinEncoding?: string;
+}

src/git/models/graph.ts

@@ -21,13 +21,16 @@ export interface GitGraphRow extends GraphRow {
 
 export interface GitGraph {
 	readonly repoPath: string;
+	/** A set of all "seen" commit ids */
+	readonly ids: Set<string>;
+	/** The rows for the set of commits requested */
 	readonly rows: GitGraphRow[];
 	readonly sha?: string;
 
 	readonly paging?: {
 		readonly limit: number | undefined;
 		readonly startingCursor: string | undefined;
-		readonly endingCursor: string | undefined;
+		// readonly endingCursor: string | undefined;
 		readonly more: boolean;
 	};
 

src/git/parsers/logParser.ts

@@ -106,20 +106,12 @@ export function getGraphParser(): GraphParser {
 	return _graphParser;
 }
 
-type GraphRefParser = Parser<{
-	sha: string;
-	authorDate: string;
-	committerDate: string;
-}>;
+type GraphRefParser = Parser<string>;
 
 let _graphRefParser: GraphRefParser | undefined;
 export function getGraphRefParser(): GraphRefParser {
 	if (_graphRefParser == null) {
-		_graphRefParser = createLogParser({
-			sha: '%H',
-			authorDate: '%at',
-			committerDate: '%ct',
-		});
+		_graphRefParser = createLogParserSingle('%H');
 	}
 	return _graphRefParser;
 }

src/plus/github/githubGitProvider.ts

@@ -1075,6 +1075,8 @@ export class GitHubGitProvider implements GitProvider, Disposable {
 			this.getTags(repoPath),
 		]);
 
+		const ids = new Set<string>();
+
 		return this.getCommitsForGraphCore(
 			repoPath,
 			asWebviewUri,
@@ -1082,6 +1084,7 @@ export class GitHubGitProvider implements GitProvider, Disposable {
 			getSettledValue(branchResult),
 			getSettledValue(remotesResult)?.[0],
 			getSettledValue(tagsResult)?.values,
+			ids,
 			options,
 		);
 	}
@@ -1093,6 +1096,7 @@ export class GitHubGitProvider implements GitProvider, Disposable {
 		branch: GitBranch | undefined,
 		remote: GitRemote | undefined,
 		tags: GitTag[] | undefined,
+		ids: Set<string>,
 		options?: {
 			branch?: string;
 			limit?: number;
@@ -1103,6 +1107,7 @@ export class GitHubGitProvider implements GitProvider, Disposable {
 		if (log == null) {
 			return {
 				repoPath: repoPath,
+				ids: ids,
 				rows: [],
 			};
 		}
@@ -1111,6 +1116,7 @@ export class GitHubGitProvider implements GitProvider, Disposable {
 		if (commits == null) {
 			return {
 				repoPath: repoPath,
+				ids: ids,
 				rows: [],
 			};
 		}
@@ -1124,6 +1130,8 @@ export class GitHubGitProvider implements GitProvider, Disposable {
 		const hasHeadShaAndRemote = branch?.sha != null && remote != null;
 
 		for (const commit of commits) {
+			ids.add(commit.sha);
+
 			if (hasHeadShaAndRemote && commit.sha === branch.sha) {
 				refHeads = [
 					{
@@ -1185,18 +1193,19 @@ export class GitHubGitProvider implements GitProvider, Disposable {
 
 		return {
 			repoPath: repoPath,
+			ids: ids,
+			rows: rows,
+			sha: options?.ref,
+
 			paging: {
 				limit: log.limit,
-				endingCursor: log.endingCursor,
+				// endingCursor: log.endingCursor,
 				startingCursor: log.startingCursor,
 				more: log.hasMore,
 			},
-			rows: rows,
-			sha: options?.ref,
-
 			more: async (limit: number | { until: string } | undefined): Promise<GitGraph | undefined> => {
 				const moreLog = await log.more?.(limit);
-				return this.getCommitsForGraphCore(repoPath, asWebviewUri, moreLog, branch, remote, tags, options);
+				return this.getCommitsForGraphCore(repoPath, asWebviewUri, moreLog, branch, remote, tags, ids, options);
 			},
 		};
 	}

src/plus/webviews/graph/graphWebview.ts

@@ -83,7 +83,6 @@ export class GraphWebview extends WebviewBase {
 	private _etagSubscription?: number;
 	private _etagRepository?: number;
 	private _graph?: GitGraph;
-	private _ids: Set<string> = new Set();
 	private _selectedSha?: string;
 	private _repositoryEventsDisposable: Disposable | undefined;
 
@@ -119,7 +118,7 @@ export class GraphWebview extends WebviewBase {
 				if (this._panel == null) {
 					void this.show({ preserveFocus: args.preserveFocus });
 				} else {
-					if (this._ids.has(args.sha)) {
+					if (this._graph?.ids.has(args.sha)) {
 						void this.notifyDidChangeSelection();
 						return;
 					}
@@ -319,7 +318,7 @@ export class GraphWebview extends WebviewBase {
 		const { defaultItemLimit, pageItemLimit } = this.getConfig();
 		const newGraph = await this._graph.more(limit ?? pageItemLimit ?? defaultItemLimit);
 		if (newGraph != null) {
-			this.setGraph(newGraph, true);
+			this.setGraph(newGraph);
 		} else {
 			debugger;
 		}
@@ -424,7 +423,6 @@ export class GraphWebview extends WebviewBase {
 			rows: data.rows,
 			paging: {
 				startingCursor: data.paging?.startingCursor,
-				endingCursor: data.paging?.endingCursor,
 				more: data.paging?.more ?? false,
 			},
 		});
@@ -463,7 +461,7 @@ export class GraphWebview extends WebviewBase {
 		const config = this.getConfig();
 
 		// If we have a set of data refresh to the same set
-		const limit = this._graph?.paging?.limit ?? config.defaultItemLimit;
+		const limit = Math.max(config.defaultItemLimit, this._graph?.ids.size ?? config.defaultItemLimit);
 
 		// Check for GitLens+ access
 		const access = await this.getGraphAccess();
@@ -490,7 +488,6 @@ export class GraphWebview extends WebviewBase {
 			rows: data.rows,
 			paging: {
 				startingCursor: data.paging?.startingCursor,
-				endingCursor: data.paging?.endingCursor,
 				more: data.paging?.more ?? false,
 			},
 			config: config,
@@ -515,19 +512,8 @@ export class GraphWebview extends WebviewBase {
 		this._selectedSha = undefined;
 	}
 
-	private setGraph(graph: GitGraph | undefined, incremental?: boolean) {
+	private setGraph(graph: GitGraph | undefined) {
 		this._graph = graph;
-
-		if (graph == null || !incremental) {
-			this._ids.clear();
-
-			if (graph == null) return;
-		}
-
-		// TODO@eamodio see if we can ask the graph if it can select the sha, so we don't have to maintain a set of ids
-		for (const row of graph.rows) {
-			this._ids.add(row.sha);
-		}
 	}
 }
 

src/plus/webviews/graph/protocol.ts

@@ -25,7 +25,6 @@ export interface State {
 
 export interface GraphPaging {
 	startingCursor?: string;
-	endingCursor?: string;
 	more: boolean;
 }