Skip to content

Commit

Permalink
Fixes row selection jumping on page loads & search
Browse files Browse the repository at this point in the history
Adds search cancellation to avoid needless calls
Adds search limit setting for the Graph
  • Loading branch information
eamodio committed Sep 23, 2022
1 parent 19fe4a8 commit db5a640
Show file tree
Hide file tree
Showing 13 changed files with 134 additions and 64 deletions.
7 changes: 7 additions & 0 deletions package.json
Expand Up @@ -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",
Expand Down
1 change: 1 addition & 0 deletions src/config.ts
Expand Up @@ -386,6 +386,7 @@ export interface GraphConfig {
defaultItemLimit: number;
highlightRowsOnRefHover: boolean;
pageItemLimit: number;
searchItemLimit: number;
statusBar: {
enabled: boolean;
};
Expand Down
13 changes: 11 additions & 2 deletions src/env/node/git/git.ts
Expand Up @@ -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');
Expand All @@ -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,
);
}
Expand Down
21 changes: 16 additions & 5 deletions src/env/node/git/localGitProvider.ts
Expand Up @@ -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';
Expand Down Expand Up @@ -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 };
Expand Down Expand Up @@ -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];
Expand All @@ -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,
Expand Down
4 changes: 2 additions & 2 deletions 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';
Expand Down Expand Up @@ -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,
Expand Down
3 changes: 2 additions & 1 deletion src/git/gitProviderService.ts
@@ -1,5 +1,6 @@
import { encodingExists } from 'iconv-lite';
import type {
CancellationToken,
ConfigurationChangeEvent,
Event,
Range,
Expand Down Expand Up @@ -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);
Expand Down
4 changes: 2 additions & 2 deletions 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';
Expand Down Expand Up @@ -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);
}
Expand Down
2 changes: 1 addition & 1 deletion src/git/search.ts
Expand Up @@ -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) {
Expand Down
3 changes: 2 additions & 1 deletion src/plus/github/githubGitProvider.ts
Expand Up @@ -2,6 +2,7 @@
import type {
AuthenticationSession,
AuthenticationSessionsChangeEvent,
CancellationToken,
Disposable,
Event,
Range,
Expand Down Expand Up @@ -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 {
Expand Down
82 changes: 55 additions & 27 deletions 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';
Expand All @@ -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';
Expand All @@ -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,
Expand Down Expand Up @@ -195,14 +201,17 @@ export class GraphWebview extends WebviewBase<State> {
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;
case GetMoreCommitsCommandType.method:
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));
Expand All @@ -213,9 +222,6 @@ export class GraphWebview extends WebviewBase<State> {
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;
}
}

Expand Down Expand Up @@ -349,22 +355,31 @@ export class GraphWebview extends WebviewBase<State> {
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 }) {
Expand Down Expand Up @@ -392,6 +407,7 @@ export class GraphWebview extends WebviewBase<State> {
}

@gate()
@debug()
private async onGetMoreCommits(sha?: string, completionId?: string) {
if (this._graph?.more == null || this._repository?.etag !== this._etagRepository) {
this.updateState(true);
Expand All @@ -410,29 +426,45 @@ export class GraphWebview extends WebviewBase<State> {
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]);
}

void this.notify(
DidSearchCommitsNotificationType,
{
searchResults: {
results: {
ids: search.results,
paging: {
startingCursor: search.paging?.startingCursor,
Expand All @@ -451,7 +483,7 @@ export class GraphWebview extends WebviewBase<State> {

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) {
Expand Down Expand Up @@ -515,11 +547,7 @@ export class GraphWebview extends WebviewBase<State> {
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()
Expand Down

0 comments on commit db5a640

Please sign in to comment.