Considers selected row in graph search

Adds search shortcuts F3/Cmd+G to the graph

src/env/node/git/git.ts

@@ -146,7 +146,7 @@ export class Git {
 				'core.quotepath=false',
 				'-c',
 				'color.ui=false',
-				...(configs !== undefined ? configs : emptyArray),
+				...(configs != null ? configs : emptyArray),
 			);
 
 			if (process.platform === 'win32') {
@@ -861,20 +861,6 @@ export class Git {
 		options?: { cancellation?: CancellationToken; 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);
-		}
-
-		if (!params.includes('--')) {
-			params.push('--');
-		}
-
 		return this.git<string>(
 			{
 				cwd: repoPath,
@@ -882,7 +868,11 @@ export class Git {
 				configs: options?.configs ?? gitLogDefaultConfigs,
 				stdin: options?.stdin,
 			},
-			...params,
+			'log',
+			...(options?.stdin ? ['--stdin'] : emptyArray),
+			...args,
+			...(options?.ref && !GitRevision.isUncommittedStaged(options.ref) ? [options.ref] : emptyArray),
+			...(!args.includes('--') ? ['--'] : emptyArray),
 		);
 	}
 
@@ -1185,35 +1175,34 @@ export class Git {
 	log__search(
 		repoPath: string,
 		search: string[] = emptyArray,
-		{
-			limit,
-			ordering,
-			skip,
-			useShow,
-		}: { limit?: number; ordering?: 'date' | 'author-date' | 'topo' | null; skip?: number; useShow?: boolean } = {},
+		options?: {
+			limit?: number;
+			ordering?: 'date' | 'author-date' | 'topo' | null;
+			skip?: number;
+			useShow?: boolean;
+		},
 	) {
-		const params = [
-			useShow ? 'show' : 'log',
-			'--name-status',
-			`--format=${GitLogParser.defaultFormat}`,
-			'--use-mailmap',
-		];
-
-		if (limit && !useShow) {
-			params.push(`-n${limit + 1}`);
-		}
-
-		if (skip && !useShow) {
-			params.push(`--skip=${skip}`);
-		}
-
-		if (ordering && !useShow) {
-			params.push(`--${ordering}-order`);
+		if (options?.useShow) {
+			return this.git<string>(
+				{ cwd: repoPath },
+				'show',
+				'-s',
+				'--name-status',
+				`--format=${GitLogParser.defaultFormat}`,
+				'--use-mailmap',
+				...search,
+			);
 		}
 
 		return this.git<string>(
-			{ cwd: repoPath, configs: useShow ? undefined : gitLogDefaultConfigs },
-			...params,
+			{ cwd: repoPath, configs: gitLogDefaultConfigs },
+			'log',
+			'--name-status',
+			`--format=${GitLogParser.defaultFormat}`,
+			'--use-mailmap',
+			...(options?.limit ? [`-n${options.limit + 1}`] : emptyArray),
+			...(options?.skip ? [`--skip=${options.skip}`] : emptyArray),
+			...(options?.ordering ? [`--${options.ordering}-order`] : emptyArray),
 			...search,
 		);
 	}
@@ -1567,6 +1556,23 @@ export class Git {
 		}
 	}
 
+	show2(
+		repoPath: string,
+		options?: { cancellation?: CancellationToken; configs?: readonly string[] },
+		...args: string[]
+	) {
+		return this.git<string>(
+			{
+				cwd: repoPath,
+				cancellation: options?.cancellation,
+				configs: options?.configs ?? gitLogDefaultConfigs,
+			},
+			'show',
+			...args,
+			...(!args.includes('--') ? ['--'] : emptyArray),
+		);
+	}
+
 	show__diff(
 		repoPath: string,
 		fileName: string,

src/env/node/git/localGitProvider.ts

@@ -92,7 +92,8 @@ import {
 	createLogParserSingle,
 	createLogParserWithFiles,
 	getGraphParser,
-	getGraphRefParser,
+	getRefAndDateParser,
+	getRefParser,
 	GitLogParser,
 	LogType,
 } from '../../../git/parsers/logParser';
@@ -1620,7 +1621,7 @@ export class LocalGitProvider implements GitProvider, Disposable {
 		},
 	): Promise<GitGraph> {
 		const parser = getGraphParser();
-		const refParser = getGraphRefParser();
+		const refParser = getRefParser();
 
 		const defaultLimit = options?.limit ?? configuration.get('graph.defaultItemLimit') ?? 5000;
 		const defaultPageLimit = configuration.get('graph.pageItemLimit') ?? 1000;
@@ -2646,22 +2647,40 @@ export class LocalGitProvider implements GitProvider, Disposable {
 
 		const comparisonKey = getSearchQueryComparisonKey(search);
 		try {
+			const refAndDateParser = getRefAndDateParser();
+
 			const { args: searchArgs, files, shas } = getGitArgsFromSearchQuery(search);
 			if (shas?.size) {
+				const data = await this.git.show2(
+					repoPath,
+					{ cancellation: options?.cancellation },
+					'-s',
+					...refAndDateParser.arguments,
+					...shas.values(),
+					...searchArgs,
+					'--',
+				);
+
+				const results = new Map<string, number>(
+					map(refAndDateParser.parse(data), c => [
+						c.sha,
+						Number(options?.ordering === 'author-date' ? c.authorDate : c.committerDate) * 1000,
+					]),
+				);
+
 				return {
 					repoPath: repoPath,
 					query: search,
 					comparisonKey: comparisonKey,
-					results: shas,
+					results: results,
 				};
 			}
 
-			const refParser = getGraphRefParser();
 			const limit = options?.limit ?? configuration.get('advanced.maxSearchItems') ?? 0;
 			const similarityThreshold = configuration.get('advanced.similarityThreshold');
 
 			const args = [
-				...refParser.arguments,
+				...refAndDateParser.arguments,
 				`-M${similarityThreshold == null ? '' : `${similarityThreshold}%`}`,
 				'--use-mailmap',
 			];
@@ -2669,7 +2688,7 @@ export class LocalGitProvider implements GitProvider, Disposable {
 				args.push(`--${options.ordering}-order`);
 			}
 
-			const results = new Set<string>();
+			const results = new Map<string, number>();
 			let total = 0;
 			let iterations = 0;
 
@@ -2702,19 +2721,21 @@ export class LocalGitProvider implements GitProvider, Disposable {
 				}
 
 				let count = 0;
-				let last: string | undefined;
-				for (const r of refParser.parse(data)) {
-					results.add(r);
+				for (const r of refAndDateParser.parse(data)) {
+					results.set(
+						r.sha,
+						Number(options?.ordering === 'author-date' ? r.authorDate : r.committerDate) * 1000,
+					);
 
 					count++;
-					last = r;
 				}
 
 				total += count;
+				const lastSha = last(results)?.[0];
 				cursor =
-					last != null
+					lastSha != null
 						? {
-								sha: last,
+								sha: lastSha,
 								skip: total - iterations,
 						  }
 						: undefined;
@@ -2743,7 +2764,7 @@ export class LocalGitProvider implements GitProvider, Disposable {
 				repoPath: repoPath,
 				query: search,
 				comparisonKey: comparisonKey,
-				results: new Set<string>(),
+				results: new Map<string, number>(),
 			};
 		}
 	}

src/git/parsers/logParser.ts

@@ -106,14 +106,28 @@ export function getGraphParser(): GraphParser {
 	return _graphParser;
 }
 
-type GraphRefParser = Parser<string>;
+type RefParser = Parser<string>;
 
-let _graphRefParser: GraphRefParser | undefined;
-export function getGraphRefParser(): GraphRefParser {
-	if (_graphRefParser == null) {
-		_graphRefParser = createLogParserSingle('%H');
+let _refParser: RefParser | undefined;
+export function getRefParser(): RefParser {
+	if (_refParser == null) {
+		_refParser = createLogParserSingle('%H');
 	}
-	return _graphRefParser;
+	return _refParser;
+}
+
+type RefAndDateParser = Parser<{ sha: string; authorDate: string; committerDate: string }>;
+
+let _refAndDateParser: RefAndDateParser | undefined;
+export function getRefAndDateParser(): RefAndDateParser {
+	if (_refAndDateParser == null) {
+		_refAndDateParser = createLogParser({
+			sha: '%H',
+			authorDate: '%at',
+			committerDate: '%ct',
+		});
+	}
+	return _refAndDateParser;
 }
 
 export function createLogParser<T extends Record<string, unknown>>(

src/git/search.ts

@@ -47,7 +47,7 @@ export interface GitSearch {
 	repoPath: string;
 	query: SearchQuery;
 	comparisonKey: string;
-	results: Set<string>;
+	results: Map<string, number>;
 
 	readonly paging?: {
 		readonly limit: number | undefined;

src/plus/github/github.ts

@@ -1910,7 +1910,7 @@ export class GitHubApi implements Disposable {
 			order?: 'asc' | 'desc' | undefined;
 			sort?: 'author-date' | 'committer-date' | undefined;
 		},
-	): Promise<GitHubPagedResult<string> | undefined> {
+	): Promise<GitHubPagedResult<{ sha: string; authorDate: number; committerDate: number }> | undefined> {
 		const scope = getLogScope();
 
 		const limit = Math.min(100, options?.limit ?? 100);
@@ -1959,7 +1959,11 @@ export class GitHubApi implements Disposable {
 					hasNextPage: hasMore,
 				},
 				totalCount: data.total_count,
-				values: data.items.map(r => r.sha),
+				values: data.items.map(r => ({
+					sha: r.sha,
+					authorDate: new Date(r.commit.author.date).getTime(),
+					committerDate: new Date(r.commit.committer?.date ?? r.commit.author.date).getTime(),
+				})),
 			};
 		} catch (ex) {
 			if (ex instanceof ProviderRequestNotFoundError) return undefined;

src/plus/github/githubGitProvider.ts

@@ -1597,14 +1597,23 @@ export class GitHubGitProvider implements GitProvider, Disposable {
 		const comparisonKey = getSearchQueryComparisonKey(search);
 
 		try {
-			const results = new Set<string>();
+			const results = new Map<string, number>();
 			const operations = parseSearchQuery(search.query);
 
 			let op;
 			let values = operations.get('commit:');
 			if (values != null) {
-				for (const value of values) {
-					results.add(value.replace(doubleQuoteRegex, ''));
+				const commitsResults = await Promise.allSettled<Promise<GitCommit | undefined>[]>(
+					values.map(v => this.getCommit(repoPath, v.replace(doubleQuoteRegex, ''))),
+				);
+				for (const commitResult of commitsResults) {
+					const commit = getSettledValue(commitResult);
+					if (commit == null) continue;
+
+					results.set(
+						commit.sha,
+						Number(options?.ordering === 'author-date' ? commit.author.date : commit.committer.date),
+					);
 				}
 
 				return {
@@ -1681,8 +1690,11 @@ export class GitHubGitProvider implements GitProvider, Disposable {
 					return { repoPath: repoPath, query: search, comparisonKey: comparisonKey, results: results };
 				}
 
-				for (const sha of result.values) {
-					results.add(sha);
+				for (const commit of result.values) {
+					results.set(
+						commit.sha,
+						Number(options?.ordering === 'author-date' ? commit.authorDate : commit.committerDate),
+					);
 				}
 
 				cursor = result.pageInfo?.endCursor ?? undefined;
@@ -1710,7 +1722,7 @@ export class GitHubGitProvider implements GitProvider, Disposable {
 				repoPath: repoPath,
 				query: search,
 				comparisonKey: comparisonKey,
-				results: new Set<string>(),
+				results: new Map<string, number>(),
 			};
 		}
 	}

src/plus/webviews/graph/graphWebview.ts

@@ -451,7 +451,7 @@ export class GraphWebview extends WebviewBase {
 					DidSearchCommitsNotificationType,
 					{
 						results: {
-							ids: [...search.results.values()],
+							ids: Object.fromEntries(search.results),
 							paging: { hasMore: search.paging?.hasMore ?? false },
 						},
 						selectedRows: this._selectedRows,
@@ -497,14 +497,14 @@ export class GraphWebview extends WebviewBase {
 		}
 
 		if (search.results.size > 0) {
-			this.setSelectedRows(first(search.results));
+			this.setSelectedRows(first(search.results)![0]);
 		}
 
 		void this.notify(
 			DidSearchCommitsNotificationType,
 			{
 				results: {
-					ids: [...search.results.values()],
+					ids: Object.fromEntries(search.results),
 					paging: { hasMore: search.paging?.hasMore ?? false },
 				},
 				selectedRows: this._selectedRows,
@@ -778,8 +778,8 @@ export class GraphWebview extends WebviewBase {
 
 			if (this._search != null) {
 				const search = this._search;
-				const lastResult = last(search.results);
-				if (lastResult != null && updatedGraph.ids.has(lastResult)) {
+				const lastId = last(search.results)?.[0];
+				if (lastId != null && updatedGraph.ids.has(lastId)) {
 					queueMicrotask(() => void this.onSearchCommits({ search: search.query, more: true }));
 				}
 			}

src/plus/webviews/graph/protocol.ts

@@ -174,7 +174,7 @@ export const DidEnsureCommitNotificationType = new IpcNotificationType
 );
 
 export interface DidSearchCommitsParams {
-	results: { ids: string[]; paging?: { hasMore: boolean } } | undefined;
+	results: { ids: { [sha: string]: number }; paging?: { hasMore: boolean } } | undefined;
 	selectedRows?: { [id: string]: true };
 }
 export const DidSearchCommitsNotificationType = new IpcNotificationType<DidSearchCommitsParams>(

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

@@ -99,11 +99,11 @@ const getGraphDateFormatter = (config?: GraphComponentConfig): OnFormatCommitDat
 	return (commitDateTime: number) => formatCommitDateTime(commitDateTime, config?.dateStyle, config?.dateFormat);
 };
 
-const getSearchHighlights = (searchIds?: string[]): { [id: string]: boolean } | undefined => {
+const getSearchHighlights = (searchIds?:
 [string, number][]): { [id: string]: boolean } | undefined => {
 	if (!searchIds?.length) return undefined;
 
 	const highlights: { [id: string]: boolean } = {};
-	for (const sha of searchIds) {
+	for (const [sha] of searchIds) {
 		highlights[sha] = true;
 	}
 	return highlights;
@@ -251,8 +251,11 @@ export function GraphWrapper({
 	// search state
 	const [searchQuery, setSearchQuery] = useState<SearchQuery | undefined>(undefined);
 	const [searchResultKey, setSearchResultKey] = useState<string | undefined>(undefined);
-	const [searchResultIds, setSearchResultIds] = useState(searchResults?.ids);
+	const [searchResultIds, setSearchResultIds] = useState(
+		searchResults != null ? Object.entries(searchResults.ids) : undefined,
+	);
 	const [hasMoreSearchResults, setHasMoreSearchResults] = useState(searchResults?.paging?.hasMore ?? false);
+	const [selectedRow, setSelectedRow] = useState<GraphRow | undefined>(undefined);
 
 	useEffect(() => {
 		if (graphRows.length === 0) {
@@ -266,8 +269,11 @@ export function GraphWrapper({
 			return;
 		}
 
-		if (searchResultKey == null || (searchResultKey != null && !searchResultIds.includes(searchResultKey))) {
-			setSearchResultKey(searchResultIds[0]);
+		if (
+			searchResultKey == null ||
+			(searchResultKey != null && !searchResultIds.some(id => id[0] === searchResultKey))
+		) {
+			setSearchResultKey(searchResultIds[0][0]);
 		}
 	}, [searchResultIds]);
 
@@ -276,16 +282,56 @@ export function GraphWrapper({
 	const searchPosition: number = useMemo(() => {
 		if (searchResultKey == null || searchResultIds == null) return 0;
 
-		const idx = searchResultIds.indexOf(searchResultKey);
+		const idx = searchResultIds.findIndex(id => id[0] === searchResultKey);
 		return idx < 1 ? 1 : idx + 1;
 	}, [searchResultKey, searchResultIds]);
 
 	const handleSearchNavigation = async (next = true) => {
 		if (searchResultKey == null || searchResultIds == null) return;
 
+		let selected = searchResultKey;
+		if (selectedRow != null && selectedRow.sha !== searchResultKey) {
+			selected = selectedRow.sha;
+		}
+
 		let resultIds = searchResultIds;
-		let rowIndex = resultIds.indexOf(searchResultKey);
-		if (rowIndex === -1) return;
+		const selectedDate = selectedRow != null ? selectedRow.date + (next ? 1 : -1) : undefined;
+
+		// Loop through the search results and:
+		//  try to find the selected sha
+		//  if next=true find the nearest date before the selected date
+		//  if next=false find the nearest date after the selected date
+		let rowIndex: number | undefined;
+		let nearestDate: number | undefined;
+		let nearestIndex: number | undefined;
+
+		let i = -1;
+		let date: number;
+		let sha: string;
+		for ([sha, date] of resultIds) {
+			i++;
+
+			if (sha === selected) {
+				rowIndex = i;
+				break;
+			}
+
+			if (selectedDate != null) {
+				if (next) {
+					if (date < selectedDate && (nearestDate == null || date > nearestDate)) {
+						nearestDate = date;
+						nearestIndex = i;
+					}
+				} else if (date > selectedDate && (nearestDate == null || date <= nearestDate)) {
+					nearestDate = date;
+					nearestIndex = i;
+				}
+			}
+		}
+
+		if (rowIndex == null) {
+			rowIndex = nearestIndex == null ? resultIds.length - 1 : nearestIndex + (next ? -1 : 1);
+		}
 
 		if (next) {
 			if (rowIndex < resultIds.length - 1) {
@@ -294,7 +340,7 @@ export function GraphWrapper({
 				const results = await onSearchCommitsPromise?.(searchQuery, { more: true });
 				if (results?.results != null) {
 					if (resultIds.length < results.results.ids.length) {
-						resultIds = results.results.ids;
+						resultIds = Object.entries(results.results.ids);
 						rowIndex++;
 					} else {
 						rowIndex = 0;
@@ -312,7 +358,7 @@ export function GraphWrapper({
 				const results = await onSearchCommitsPromise?.(searchQuery, { limit: 0, more: true });
 				if (results?.results != null) {
 					if (resultIds.length < results.results.ids.length) {
-						resultIds = results.results.ids;
+						resultIds = Object.entries(results.results.ids);
 					}
 				}
 			}
@@ -320,7 +366,7 @@ export function GraphWrapper({
 			rowIndex = resultIds.length - 1;
 		}
 
-		const nextSha = resultIds[rowIndex];
+		const nextSha = resultIds[rowIndex][0];
 		if (nextSha == null) return;
 
 		if (onEnsureCommitPromise != null) {
@@ -394,7 +440,7 @@ export function GraphWrapper({
 		setSubscriptionSnapshot(state.subscription);
 		setIsPrivateRepo(state.selectedRepositoryVisibility === RepositoryVisibility.Private);
 		setIsLoading(state.loading);
-		setSearchResultIds(state.searchResults?.ids);
+		setSearchResultIds(state.searchResults != null ? Object.entries(state.searchResults.ids) : undefined);
 		setHasMoreSearchResults(state.searchResults?.paging?.hasMore ?? false);
 	}
 
@@ -443,6 +489,7 @@ export function GraphWrapper({
 	};
 
 	const handleSelectGraphRows = (graphRows: GraphRow[]) => {
+		setSelectedRow(graphRows[0]);
 		onSelectionChange?.(graphRows.map(r => ({ id: r.sha, type: r.type as GitGraphRowType })));
 	};
 

src/webviews/apps/shared/components/search/search-field.ts

@@ -1,4 +1,4 @@
-import { attr, css, customElement, FASTElement, html, observable, ref } from '@microsoft/fast-element';
+import { attr, css, customElement, FASTElement, html, observable } from '@microsoft/fast-element';
 import type { SearchQuery } from '../../../../../git/search';
 import '../codicon';
 
@@ -207,8 +207,7 @@ export class SearchField extends FASTElement {
 	}
 
 	handleShortcutKeys(e: KeyboardEvent) {
-		if (e.key !== 'Enter' && e.key !== 'F3') return;
-		if (e.ctrlKey || e.metaKey || e.altKey) return;
+		if (e.key !== 'Enter' || e.ctrlKey || e.metaKey || e.altKey) return;
 
 		e.preventDefault();
 		if (e.shiftKey) {

src/webviews/apps/shared/components/search/search-nav.ts

@@ -1,5 +1,8 @@
 import { attr, css, customElement, FASTElement, html, volatile, when } from '@microsoft/fast-element';
+import { isMac } from '@env/platform';
 import { pluralize } from '../../../../../system/string';
+import type { Disposable } from '../../../shared/dom';
+import { DOM } from '../../../shared/dom';
 import { numberConverter } from '../converters/number-converter';
 import '../codicon';
 
@@ -119,11 +122,48 @@ export class SearchNav extends FASTElement {
 		return this.total !== 0;
 	}
 
-	handlePrevious(_e: Event) {
+	private _disposable: Disposable | undefined;
+	override connectedCallback(): void {
+		super.connectedCallback();
+
+		this._disposable = DOM.on(window, 'keyup', e => this.handleShortcutKeys(e));
+	}
+
+	override disconnectedCallback(): void {
+		super.disconnectedCallback();
+
+		this._disposable?.dispose();
+	}
+
+	next() {
+		this.$emit('next');
+	}
+
+	previous() {
 		this.$emit('previous');
 	}
 
+	handleShortcutKeys(e: KeyboardEvent) {
+		if (
+			(e.key !== 'F3' && e.key !== 'g') ||
+			(e.key !== 'g' && (e.ctrlKey || e.metaKey || e.altKey)) ||
+			(e.key === 'g' && (!e.metaKey || !isMac))
+		) {
+			return;
+		}
+
+		if (e.shiftKey) {
+			this.previous();
+		} else {
+			this.next();
+		}
+	}
+
+	handlePrevious(_e: Event) {
+		this.previous();
+	}
+
 	handleNext(_e: Event) {
-		this.$emit('next');
+		this.next();
 	}
 }