Fixes row selection jumping on page loads & search

Adds search cancellation to avoid needless calls
Adds search limit setting for the Graph

package.json

@@ -2120,6 +2120,13 @@
 						"scope": "window",
 						"order": 21
 					},
+					"gitlens.graph.searchItemLimit": {
+						"type": "number",
+						"default": 100,
+						"markdownDescription": "Specifies the number of results to gather when searching in the _Commit Graph_. Use 0 to specify no limit",
+						"scope": "window",
+						"order": 22
+					},
 					"gitlens.graph.commitOrdering": {
 						"type": "string",
 						"default": "date",

src/config.ts

@@ -386,6 +386,7 @@ export interface GraphConfig {
 	defaultItemLimit: number;
 	highlightRowsOnRefHover: boolean;
 	pageItemLimit: number;
+	searchItemLimit: number;
 	statusBar: {
 		enabled: boolean;
 	};

src/env/node/git/git.ts

@@ -856,7 +856,11 @@ export class Git {
 		return this.git<string>({ cwd: repoPath, configs: gitLogDefaultConfigsWithFiles }, ...params, '--');
 	}
 
-	log2(repoPath: string, options?: { configs?: readonly string[]; ref?: string; stdin?: string }, ...args: string[]) {
+	log2(
+		repoPath: string,
+		options?: { cancellation?: CancellationToken; configs?: readonly string[]; ref?: string; stdin?: string },
+		...args: string[]
+	) {
 		const params = ['log'];
 		if (options?.stdin) {
 			params.push('--stdin');
@@ -872,7 +876,12 @@ export class Git {
 		}
 
 		return this.git<string>(
-			{ cwd: repoPath, configs: options?.configs ?? gitLogDefaultConfigs, stdin: options?.stdin },
+			{
+				cwd: repoPath,
+				cancellation: options?.cancellation,
+				configs: options?.configs ?? gitLogDefaultConfigs,
+				stdin: options?.stdin,
+			},
 			...params,
 		);
 	}

src/env/node/git/localGitProvider.ts

@@ -2,7 +2,7 @@ import { readdir, realpath } from 'fs';
 import { homedir, hostname, userInfo } from 'os';
 import { resolve as resolvePath } from 'path';
 import { env as process_env } from 'process';
-import type { Event, TextDocument, WorkspaceFolder } from 'vscode';
+import type { CancellationToken, Event, TextDocument, WorkspaceFolder } from 'vscode';
 import { Disposable, env, EventEmitter, extensions, FileType, Range, Uri, window, workspace } from 'vscode';
 import { fetch, getProxyAgent } from '@env/fetch';
 import { hrtime } from '@env/hrtime';
@@ -2641,7 +2641,7 @@ export class LocalGitProvider implements GitProvider, Disposable {
 	async searchForCommitsSimple(
 		repoPath: string,
 		search: SearchPattern,
-		options?: { limit?: number; ordering?: 'date' | 'author-date' | 'topo' },
+		options?: { cancellation?: CancellationToken; limit?: number; ordering?: 'date' | 'author-date' | 'topo' },
 	): Promise<GitSearch> {
 		search = { matchAll: false, matchCase: false, matchRegex: true, ...search };
 
@@ -2676,15 +2676,26 @@ export class LocalGitProvider implements GitProvider, Disposable {
 				limit: number,
 				cursor?: { sha: string; skip: number },
 			): Promise<GitSearch> {
+				if (options?.cancellation?.isCancellationRequested) {
+					// TODO@eamodio: Should we throw an error here?
+					return { repoPath: repoPath, pattern: search, results: [] };
+				}
+
 				const data = await this.git.log2(
 					repoPath,
-					undefined,
+					{ cancellation: options?.cancellation },
 					...args,
 					...(cursor?.skip ? [`--skip=${cursor.skip}`] : []),
 					...searchArgs,
 					'--',
 					...files,
 				);
+
+				if (options?.cancellation?.isCancellationRequested) {
+					// TODO@eamodio: Should we throw an error here?
+					return { repoPath: repoPath, pattern: search, results: [] };
+				}
+
 				const results = [...refParser.parse(data)];
 
 				const last = results[results.length - 1];
@@ -2708,13 +2719,13 @@ export class LocalGitProvider implements GitProvider, Disposable {
 									more: true,
 							  }
 							: undefined,
-					more: async (limit: number): Promise<GitSearch | undefined> =>
-						searchForCommitsCore.call(this, limit, cursor),
+					more: async (limit: number): Promise<GitSearch> => searchForCommitsCore.call(this, limit, cursor),
 				};
 			}
 
 			return searchForCommitsCore.call(this, limit);
 		} catch (ex) {
+			// TODO@eamodio: Should we throw an error here?
 			// TODO@eamodio handle error reporting -- just invalid patterns? or more detailed?
 			return {
 				repoPath: repoPath,

src/git/gitProvider.ts

@@ -1,4 +1,4 @@
-import type { Disposable, Event, Range, TextDocument, Uri, WorkspaceFolder } from 'vscode';
+import type { CancellationToken, Disposable, Event, Range, TextDocument, Uri, WorkspaceFolder } from 'vscode';
 import type { Commit, InputBox } from '../@types/vscode.git';
 import type { ForcePushMode } from '../@types/vscode.git.enums';
 import type { Features } from '../features';
@@ -299,7 +299,7 @@ export interface GitProvider extends Disposable {
 	searchForCommitsSimple(
 		repoPath: string | Uri,
 		search: SearchPattern,
-		options?: { limit?: number; ordering?: 'date' | 'author-date' | 'topo' },
+		options?: { cancellation?: CancellationToken; limit?: number; ordering?: 'date' | 'author-date' | 'topo' },
 	): Promise<GitSearch>;
 	getLogForSearch(
 		repoPath: string,

src/git/gitProviderService.ts

@@ -1,5 +1,6 @@
 import { encodingExists } from 'iconv-lite';
 import type {
+	CancellationToken,
 	ConfigurationChangeEvent,
 	Event,
 	Range,
@@ -1468,7 +1469,7 @@ export class GitProviderService implements Disposable {
 	searchForCommitsSimple(
 		repoPath: string | Uri,
 		search: SearchPattern,
-		options?: { limit?: number; ordering?: 'date' | 'author-date' | 'topo' },
+		options?: { cancellation?: CancellationToken; limit?: number; ordering?: 'date' | 'author-date' | 'topo' },
 	): Promise<GitSearch> {
 		const { provider, path } = this.getProvider(repoPath);
 		return provider.searchForCommitsSimple(path, search, options);

src/git/models/repository.ts

@@ -1,4 +1,4 @@
-import type { ConfigurationChangeEvent, Event, WorkspaceFolder } from 'vscode';
+import type { CancellationToken, ConfigurationChangeEvent, Event, WorkspaceFolder } from 'vscode';
 import { Disposable, EventEmitter, ProgressLocation, RelativePattern, Uri, window, workspace } from 'vscode';
 import { ForcePushMode } from '../../@types/vscode.git.enums';
 import type { CreatePullRequestActionContext } from '../../api/gitlens';
@@ -861,7 +861,7 @@ export class Repository implements Disposable {
 
 	searchForCommitsSimple(
 		search: SearchPattern,
-		options?: { limit?: number; ordering?: 'date' | 'author-date' | 'topo' },
+		options?: { cancellation?: CancellationToken; limit?: number; ordering?: 'date' | 'author-date' | 'topo' },
 	): Promise<GitSearch> {
 		return this.container.git.searchForCommitsSimple(this.path, search, options);
 	}

src/git/search.ts

@@ -46,7 +46,7 @@ export interface GitSearch {
 		readonly more: boolean;
 	};
 
-	more?(limit: number): Promise<GitSearch | undefined>;
+	more?(limit: number): Promise<GitSearch>;
 }
 
 export function getKeyForSearchPattern(search: SearchPattern) {

src/plus/github/githubGitProvider.ts

@@ -2,6 +2,7 @@
 import type {
 	AuthenticationSession,
 	AuthenticationSessionsChangeEvent,
+	CancellationToken,
 	Disposable,
 	Event,
 	Range,
@@ -1584,7 +1585,7 @@ export class GitHubGitProvider implements GitProvider, Disposable {
 	async searchForCommitsSimple(
 		repoPath: string,
 		search: SearchPattern,
-		_options?: { limit?: number; ordering?: 'date' | 'author-date' | 'topo' },
+		_options?: { cancellation?: CancellationToken; limit?: number; ordering?: 'date' | 'author-date' | 'topo' },
 	): Promise<GitSearch> {
 		search = { matchAll: false, matchCase: false, matchRegex: true, ...search };
 		return {

src/plus/webviews/graph/graphWebview.ts

@@ -1,5 +1,5 @@
 import type { ColorTheme, ConfigurationChangeEvent, Disposable, Event, StatusBarItem } from 'vscode';
-import { EventEmitter, MarkdownString, ProgressLocation, StatusBarAlignment, ViewColumn, window } from 'vscode';
+import { CancellationTokenSource, EventEmitter, MarkdownString, StatusBarAlignment, ViewColumn, window } from 'vscode';
 import { getAvatarUri } from '../../../avatars';
 import { parseCommandContext } from '../../../commands/base';
 import { GitActions } from '../../../commands/gitCommands.actions';
@@ -14,7 +14,6 @@ import { GitGraphRowType } from '../../../git/models/graph';
 import type { GitGraph } from '../../../git/models/graph';
 import type { Repository, RepositoryChangeEvent } from '../../../git/models/repository';
 import { RepositoryChange, RepositoryChangeComparisonMode } from '../../../git/models/repository';
-import type { SearchPattern } from '../../../git/search';
 import { registerCommand } from '../../../system/command';
 import { gate } from '../../../system/decorators/gate';
 import { debug } from '../../../system/decorators/log';
@@ -28,7 +27,14 @@ import { onIpc } from '../../../webviews/protocol';
 import { WebviewBase } from '../../../webviews/webviewBase';
 import type { SubscriptionChangeEvent } from '../../subscription/subscriptionService';
 import { ensurePlusFeaturesEnabled } from '../../subscription/utils';
-import type { DismissBannerParams, GraphComponentConfig, GraphRepository, State } from './protocol';
+import type {
+	DismissBannerParams,
+	EnsureCommitParams,
+	GraphComponentConfig,
+	GraphRepository,
+	SearchCommitsParams,
+	State,
+} from './protocol';
 import {
 	DidChangeAvatarsNotificationType,
 	DidChangeCommitsNotificationType,
@@ -195,6 +201,9 @@ export class GraphWebview extends WebviewBase {
 			case DismissBannerCommandType.method:
 				onIpc(DismissBannerCommandType, e, params => this.dismissBanner(params.key));
 				break;
+			case EnsureCommitCommandType.method:
+				onIpc(EnsureCommitCommandType, e, params => this.onEnsureCommit(params, e.completionId));
+				break;
 			case GetMissingAvatarsCommandType.method:
 				onIpc(GetMissingAvatarsCommandType, e, params => this.onGetMissingAvatars(params.emails));
 				break;
@@ -202,7 +211,7 @@ export class GraphWebview extends WebviewBase {
 				onIpc(GetMoreCommitsCommandType, e, params => this.onGetMoreCommits(params.sha, e.id));
 				break;
 			case SearchCommitsCommandType.method:
-				onIpc(SearchCommitsCommandType, e, params => this.onSearchCommits(params.search, e.id));
+				onIpc(SearchCommitsCommandType, e, params => this.onSearchCommits(params, e.id));
 				break;
 			case UpdateColumnCommandType.method:
 				onIpc(UpdateColumnCommandType, e, params => this.onColumnUpdated(params.name, params.config));
@@ -213,9 +222,6 @@ export class GraphWebview extends WebviewBase {
 			case UpdateSelectionCommandType.method:
 				onIpc(UpdateSelectionCommandType, e, params => this.onSelectionChanged(params.selection));
 				break;
-			case EnsureCommitCommandType.method:
-				onIpc(EnsureCommitCommandType, e, params => this.onEnsureCommit(params.id, e.completionId));
-				break;
 		}
 	}
 
@@ -349,22 +355,31 @@ export class GraphWebview extends WebviewBase {
 		void this.notifyDidChangeGraphConfiguration();
 	}
 
-	private async onEnsureCommit(id: string, completionId?: string) {
+	@debug()
+	private async onEnsureCommit(e: EnsureCommitParams, completionId?: string) {
 		if (this._graph?.more == null) return;
 
-		if (!this._graph.ids.has(id)) {
+		let selected: boolean | undefined;
+		if (!this._graph.ids.has(e.id)) {
 			const { defaultItemLimit, pageItemLimit } = configuration.get('graph');
-			const newGraph = await this._graph.more(pageItemLimit ?? defaultItemLimit, id);
+			const newGraph = await this._graph.more(pageItemLimit ?? defaultItemLimit, e.id);
 			if (newGraph != null) {
 				this.setGraph(newGraph);
 			} else {
 				debugger;
 			}
 
+			if (e.select && this._graph.ids.has(e.id)) {
+				selected = true;
+				this.setSelectedRows(e.id);
+			}
 			void this.notifyDidChangeCommits();
+		} else if (e.select) {
+			selected = true;
+			this.setSelectedRows(e.id);
 		}
 
-		void this.notify(DidEnsureCommitNotificationType, { id: id }, completionId);
+		void this.notify(DidEnsureCommitNotificationType, { id: e.id, selected: selected }, completionId);
 	}
 
 	private async onGetMissingAvatars(emails: { [email: string]: string }) {
@@ -392,6 +407,7 @@ export class GraphWebview extends WebviewBase {
 	}
 
 	@gate()
+	@debug()
 	private async onGetMoreCommits(sha?: string, completionId?: string) {
 		if (this._graph?.more == null || this._repository?.etag !== this._etagRepository) {
 			this.updateState(true);
@@ -410,21 +426,37 @@ export class GraphWebview extends WebviewBase {
 		void this.notifyDidChangeCommits(completionId);
 	}
 
-	@gate()
-	private async onSearchCommits(searchPattern: SearchPattern, completionId?: string) {
-		// if (this._repository?.etag !== this._etagRepository) {
-		// 	this.updateState(true);
-
-		// 	return;
-		// }
+	private _searchCancellation: CancellationTokenSource | undefined;
 
+	@debug()
+	private async onSearchCommits(e: SearchCommitsParams, completionId?: string) {
 		if (this._repository == null) return;
 
-		const search = await this._repository.searchForCommitsSimple(searchPattern, {
-			limit: 100,
+		if (this._repository.etag !== this._etagRepository) {
+			this.updateState(true);
+		}
+
+		if (this._searchCancellation != null) {
+			this._searchCancellation.cancel();
+			this._searchCancellation.dispose();
+		}
+
+		const cancellation = new CancellationTokenSource();
+		this._searchCancellation = cancellation;
+
+		const search = await this._repository.searchForCommitsSimple(e.search, {
+			limit: configuration.get('graph.searchItemLimit') ?? 100,
 			ordering: configuration.get('graph.commitOrdering'),
+			cancellation: cancellation.token,
 		});
 
+		if (cancellation.token.isCancellationRequested) {
+			if (completionId != null) {
+				void this.notify(DidSearchCommitsNotificationType, { results: undefined }, completionId);
+			}
+			return;
+		}
+
 		if (search.results.length > 0) {
 			this.setSelectedRows(search.results[0]);
 		}
@@ -432,7 +464,7 @@ export class GraphWebview extends WebviewBase {
 		void this.notify(
 			DidSearchCommitsNotificationType,
 			{
-				searchResults: {
+				results: {
 					ids: search.results,
 					paging: {
 						startingCursor: search.paging?.startingCursor,
@@ -451,7 +483,7 @@ export class GraphWebview extends WebviewBase {
 
 	private async onSelectionChanged(selection: { id: string; type: GitGraphRowType }[]) {
 		const item = selection[0];
-		this._selectedSha = item?.id;
+		this.setSelectedRows(item?.id);
 
 		let commits: GitCommit[] | undefined;
 		if (item?.id != null) {
@@ -515,11 +547,7 @@ export class GraphWebview extends WebviewBase {
 	private async notifyDidChangeState() {
 		if (!this.isReady || !this.visible) return false;
 
-		return window.withProgress({ location: ProgressLocation.Window, title: 'Loading Commit Graph...' }, async () =>
-			this.notify(DidChangeNotificationType, {
-				state: await this.getState(),
-			}),
-		);
+		return this.notify(DidChangeNotificationType, { state: await this.getState() });
 	}
 
 	@debug()

src/plus/webviews/graph/protocol.ts

@@ -25,7 +25,7 @@ export interface State {
 
 	// Props below are computed in the webview (not passed)
 	mixedColumnColors?: Record<string, string>;
-	searchResults?: DidSearchCommitsParams['searchResults'];
+	searchResults?: DidSearchCommitsParams['results'];
 }
 
 export interface GraphPaging {
@@ -79,6 +79,12 @@ export interface DismissBannerParams {
 }
 export const DismissBannerCommandType = new IpcCommandType<DismissBannerParams>('graph/dismissBanner');
 
+export interface EnsureCommitParams {
+	id: string;
+	select?: boolean;
+}
+export const EnsureCommitCommandType = new IpcCommandType<EnsureCommitParams>('graph/ensureCommit');
+
 export interface GetMissingAvatarsParams {
 	emails: { [email: string]: string };
 }
@@ -112,11 +118,6 @@ export interface UpdateSelectionParams {
 }
 export const UpdateSelectionCommandType = new IpcCommandType<UpdateSelectionParams>('graph/update/selection');
 
-export interface EnsureCommitParams {
-	id: string;
-}
-export const EnsureCommitCommandType = new IpcCommandType<EnsureCommitParams>('graph/ensureCommit');
-
 // Notifications
 export interface DidChangeParams {
 	state: State;
@@ -162,20 +163,23 @@ export const DidChangeSelectionNotificationType = new IpcNotificationType
 	'graph/selection/didChange',
 );
 
-export interface DidSearchCommitsParams {
-	searchResults: {
-		ids: string[];
-		paging?: GraphPaging;
-	};
-	selectedRows?: { [id: string]: true };
-}
-export const DidSearchCommitsNotificationType = new IpcNotificationType<DidSearchCommitsParams>(
-	'graph/commits/didSearch',
-);
-
 export interface DidEnsureCommitParams {
 	id?: string;
+	selected?: boolean;
 }
 export const DidEnsureCommitNotificationType = new IpcNotificationType<DidEnsureCommitParams>(
 	'graph/commits/didEnsureCommit',
 );
+
+export interface DidSearchCommitsParams {
+	results:
+		| {
+				ids: string[];
+				paging?: GraphPaging;
+		  }
+		| undefined;
+	selectedRows?: { [id: string]: true };
+}
+export const DidSearchCommitsNotificationType = new IpcNotificationType<DidSearchCommitsParams>(
+	'graph/commits/didSearch',
+);

src/webviews/apps/plus/graph/GraphWrapper.tsx

@@ -40,7 +40,7 @@ export interface GraphWrapperProps extends State {
 	onSearchCommits?: (search: SearchPattern) => void; //Promise<DidSearchCommitsParams>;
 	onDismissBanner?: (key: DismissBannerParams['key']) => void;
 	onSelectionChange?: (selection: { id: string; type: GitGraphRowType }[]) => void;
-	onEnsureCommit?: (id: string) => Promise<DidEnsureCommitParams>;
+	onEnsureCommit?: (id: string, select: boolean) => Promise<DidEnsureCommitParams>;
 }
 
 const getStyleProps = (
@@ -299,7 +299,7 @@ export function GraphWrapper({
 				timeout = undefined;
 				setIsLoading(true);
 			}, 250);
-			onEnsureCommit(nextSha).finally(() => {
+			onEnsureCommit(nextSha, true).finally(() => {
 				if (timeout == null) {
 					setIsLoading(false);
 				} else {
@@ -347,7 +347,9 @@ export function GraphWrapper({
 		setAvatars(state.avatars ?? {});
 		setReposList(state.repositories ?? []);
 		setCurrentRepository(reposList.find(item => item.path === state.selectedRepository));
-		setSelectedRows(state.selectedRows);
+		if (JSON.stringify(graphSelectedRows) !== JSON.stringify(state.selectedRows)) {
+			setSelectedRows(state.selectedRows);
+		}
 		setGraphConfig(state.config);
 		// setGraphDateFormatter(getGraphDateFormatter(config));
 		setGraphColSettings(getGraphColSettingsModel(state.config));

src/webviews/apps/plus/graph/graph.tsx

@@ -192,9 +192,11 @@ export class GraphApp extends App {
 
 			case DidSearchCommitsNotificationType.method:
 				onIpc(DidSearchCommitsNotificationType, msg, params => {
+					if (params.results == null && params.selectedRows == null) return;
+
 					this.setState({
 						...this.state,
-						searchResults: params.searchResults,
+						searchResults: params.results,
 						selectedRows: params.selectedRows,
 					});
 					this.refresh(this.state);
@@ -316,8 +318,12 @@ export class GraphApp extends App {
 		return this.sendCommand(SearchCommitsCommandType, { search: search });
 	}
 
-	private onEnsureCommit(id: string) {
-		return this.sendCommandWithCompletion(EnsureCommitCommandType, { id: id }, DidEnsureCommitNotificationType);
+	private onEnsureCommit(id: string, select: boolean) {
+		return this.sendCommandWithCompletion(
+			EnsureCommitCommandType,
+			{ id: id, select: select },
+			DidEnsureCommitNotificationType,
+		);
 	}
 
 	private onSelectionChanged(selection: { id: string; type: GitGraphRowType }[]) {