Skip to content

Commit

Permalink
Fixes #2482 lazy loads files for merge commits
Browse files Browse the repository at this point in the history 
Consolidated log into log2 calls (and renames it)
  • Loading branch information
@eamodio
eamodio committed Nov 27, 2023
1 parent 32a3f64 commit efcc3ca
Show file tree
Hide file tree
Showing 7 changed files with 63 additions and 121 deletions.
4 changes: 4 additions & 0 deletions CHANGELOG.md
View file
Expand Up @@ -6,6 +6,10 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/) and this p

## [Unreleased]

### Fixed

- Fixes [#2482](https://github.com/gitkraken/vscode-gitlens/issues/2482) - Unresponsive "commits" view and "branches" view update due to git log

## [14.5.1] - 2023-11-21

### Added
Expand Down
79 changes: 0 additions & 79 deletions src/env/node/git/git.ts
View file
Expand Up @@ -1082,85 +1082,6 @@ export class Git {
}

log(
repoPath: string,
ref: string | undefined,
{
all,
argsOrFormat,
authors,
limit,
merges,
ordering,
similarityThreshold,
since,
until,
}: {
all?: boolean;
argsOrFormat?: string | string[];
authors?: GitUser[];
limit?: number;
merges?: boolean;
ordering?: 'date' | 'author-date' | 'topo' | null;
similarityThreshold?: number | null;
since?: number | string;
until?: number | string;
},
) {
if (argsOrFormat == null) {
argsOrFormat = ['--name-status', `--format=${all ? parseGitLogAllFormat : parseGitLogDefaultFormat}`];
}

if (typeof argsOrFormat === 'string') {
argsOrFormat = [`--format=${argsOrFormat}`];
}

const params = [
'log',
...argsOrFormat,
'--full-history',
`-M${similarityThreshold == null ? '' : `${similarityThreshold}%`}`,
'-m',
];

if (ordering) {
params.push(`--${ordering}-order`);
}

if (limit) {
params.push(`-n${limit + 1}`);
}

if (since) {
params.push(`--since="${since}"`);
}

if (until) {
params.push(`--until="${until}"`);
}

if (!merges) {
params.push('--first-parent');
}

if (authors != null && authors.length !== 0) {
if (!params.includes('--use-mailmap')) {
params.push('--use-mailmap');
}
params.push(...authors.map(a => `--author=^${a.name} <${a.email}>$`));
}

if (all) {
params.push('--all', '--single-worktree');
}

if (ref && !isUncommittedStaged(ref)) {
params.push(ref);
}

return this.git<string>({ cwd: repoPath, configs: gitLogDefaultConfigsWithFiles }, ...params, '--');
}

log2(
repoPath: string,
options?: {
cancellation?: CancellationToken;
Expand Down
82 changes: 54 additions & 28 deletions src/env/node/git/localGitProvider.ts
View file
Expand Up @@ -2267,7 +2267,7 @@ export class LocalGitProvider implements GitProvider, Disposable {
const statsParser = getGraphStatsParser();

const [refResult, stashResult, branchesResult, remotesResult, currentUserResult] = await Promise.allSettled([
this.git.log2(repoPath, undefined, ...refParser.arguments, '-n1', options?.ref ?? 'HEAD'),
this.git.log(repoPath, undefined, ...refParser.arguments, '-n1', options?.ref ?? 'HEAD'),
this.getStash(repoPath),
this.getBranches(repoPath),
this.getRemotes(repoPath),
Expand Down Expand Up @@ -2339,7 +2339,7 @@ export class LocalGitProvider implements GitProvider, Disposable {
} else {
args.push(`-n${nextPageLimit + 1}`);

data = await this.git.log2(repoPath, stdin ? { stdin: stdin } : undefined, ...args);
data = await this.git.log(repoPath, stdin ? { stdin: stdin } : undefined, ...args);

if (cursor) {
if (!getShaInLogRegex(cursor.sha).test(data)) {
Expand Down Expand Up @@ -2727,7 +2727,7 @@ export class LocalGitProvider implements GitProvider, Disposable {
}
args.push(`--${ordering}-order`, '--all');

const statsData = await this.git.log2(repoPath, stdin ? { stdin: stdin } : undefined, ...args);
const statsData = await this.git.log(repoPath, stdin ? { stdin: stdin } : undefined, ...args);
if (statsData) {
const commitStats = statsParser.parse(statsData);
for (const stat of commitStats) {
Expand Down Expand Up @@ -2810,10 +2810,12 @@ export class LocalGitProvider implements GitProvider, Disposable {
const currentUser = await this.getCurrentUser(repoPath);
const parser = getContributorsParser(options?.stats);

const data = await this.git.log(repoPath, options?.ref, {
all: options?.all,
argsOrFormat: parser.arguments,
});
const args = [...parser.arguments, '--full-history', '--first-parent'];
if (options?.all) {
args.push('--all', '--single-worktree');
}

const data = await this.git.log(repoPath, { ref: options?.ref }, ...args);

const contributors = new Map<string, GitContributor>();

Expand Down Expand Up @@ -3320,14 +3322,10 @@ export class LocalGitProvider implements GitProvider, Disposable {
try {
const limit = options?.limit ?? configuration.get('advanced.maxListItems') ?? 0;
const merges = options?.merges == null ? true : options.merges;
const ordering = options?.ordering ?? configuration.get('advanced.commitOrdering');
const similarityThreshold = configuration.get('advanced.similarityThreshold');
const args = [
`--format=${options?.all ? parseGitLogAllFormat : parseGitLogDefaultFormat}`,
`-M${similarityThreshold == null ? '' : `${similarityThreshold}%`}`,
'-m',
];
if (options?.status !== null) {
Expand All @@ -3336,11 +3334,19 @@ export class LocalGitProvider implements GitProvider, Disposable {
if (options?.all) {
args.push('--all');
}
if (!merges) {
const merges = options?.merges ?? true;
if (merges) {
if (limit <= 2) {
// Ensure we return the merge commit files when we are asking for a specific ref
args.push('-m');
}
args.push(merges === 'first-parent' ? '--first-parent' : '--no-min-parents');
} else {
args.push('--no-merges');
} else if (merges === 'first-parent') {
args.push('--first-parent');
}
const ordering = options?.ordering ?? configuration.get('advanced.commitOrdering');
if (ordering) {
args.push(`--${ordering}-order`);
}
Expand Down Expand Up @@ -3374,7 +3380,7 @@ export class LocalGitProvider implements GitProvider, Disposable {
args.push(`-n${limit + 1}`);
}
const data = await this.git.log2(
const data = await this.git.log(
repoPath,
{ configs: gitLogDefaultConfigsWithFiles, ref: options?.ref, stdin: options?.stdin },
...args,
Expand Down Expand Up @@ -3437,8 +3443,8 @@ export class LocalGitProvider implements GitProvider, Disposable {
if (log.hasMore) {
let opts;
if (options != null) {
let extraArgs;
({ extraArgs, ...opts } = options);
let _;
({ extraArgs: _, ...opts } = options);
}
log.more = this.getLogMoreFn(log, opts);
}
Expand All @@ -3459,7 +3465,7 @@ export class LocalGitProvider implements GitProvider, Disposable {
authors?: GitUser[];
cursor?: string;
limit?: number;
merges?: boolean;
merges?: boolean | 'first-parent';
ordering?: 'date' | 'author-date' | 'topo' | null;
ref?: string;
since?: string;
Expand All @@ -3471,16 +3477,36 @@ export class LocalGitProvider implements GitProvider, Disposable {
try {
const parser = createLogParserSingle('%H');
const args = [...parser.arguments, '--full-history'];
const data = await this.git.log(repoPath, options?.ref, {
authors: options?.authors,
argsOrFormat: parser.arguments,
limit: limit,
merges: options?.merges == null ? true : options.merges,
similarityThreshold: configuration.get('advanced.similarityThreshold'),
since: options?.since,
ordering: options?.ordering ?? configuration.get('advanced.commitOrdering'),
});
const ordering = options?.ordering ?? configuration.get('advanced.commitOrdering');
if (ordering) {
args.push(`--${ordering}-order`);
}
if (limit) {
args.push(`-n${limit + 1}`);
}
if (options?.since) {
args.push(`--since="${options.since}"`);
}
const merges = options?.merges ?? true;
if (merges) {
args.push(merges === 'first-parent' ? '--first-parent' : '--no-min-parents');
} else {
args.push('--no-merges');
}
if (options?.authors?.length) {
if (!args.includes('--use-mailmap')) {
args.push('--use-mailmap');
}
args.push(...options.authors.map(a => `--author=^${a.name} <${a.email}>$`));
}
const data = await this.git.log(repoPath, { ref: options?.ref }, ...args);
const commits = new Set(parser.parse(data));
return commits;
Expand Down Expand Up @@ -5304,7 +5330,7 @@ export class LocalGitProvider implements GitProvider, Disposable {
let data;
try {
data = await this.git.log2(
data = await this.git.log(
repoPath,
{
cancellation: options?.cancellation,
Expand Down
2 changes: 1 addition & 1 deletion src/git/gitProvider.ts
View file
Expand Up @@ -346,7 +346,7 @@ export interface GitProvider extends Disposable {
authors?: GitUser[] | undefined;
cursor?: string | undefined;
limit?: number | undefined;
merges?: boolean | undefined;
merges?: boolean | 'first-parent';
ordering?: 'date' | 'author-date' | 'topo' | null | undefined;
ref?: string | undefined;
since?: string | undefined;
Expand Down
2 changes: 1 addition & 1 deletion src/git/gitProviderService.ts
View file
Expand Up @@ -1884,7 +1884,7 @@ export class GitProviderService implements Disposable {
options?: {
authors?: GitUser[];
limit?: number;
merges?: boolean;
merges?: boolean | 'first-parent';
ordering?: 'date' | 'author-date' | 'topo' | null;
ref?: string;
since?: string;
Expand Down
9 changes: 0 additions & 9 deletions src/git/parsers/logParser.ts
View file
Expand Up @@ -527,15 +527,6 @@ export function parseGitLog(

let hasFiles = true;
if (next.done || next.value === '</f>') {
// If this is a merge commit and there are no files returned, skip the commit and reduce our truncationCount to ensure accurate truncation detection
if ((entry.parentShas?.length ?? 0) > 1) {
if (truncationCount) {
truncationCount--;
}

break;
}

hasFiles = false;
}

Expand Down
6 changes: 3 additions & 3 deletions src/plus/integrations/providers/github/githubGitProvider.ts
View file
Expand Up @@ -1782,7 +1782,7 @@ export class GitHubGitProvider implements GitProvider, Disposable {
authors?: GitUser[];
cursor?: string;
limit?: number;
merges?: boolean;
merges?: boolean | 'first-parent';
ordering?: 'date' | 'author-date' | 'topo' | null;
ref?: string;
since?: string;
Expand Down Expand Up @@ -1887,7 +1887,7 @@ export class GitHubGitProvider implements GitProvider, Disposable {
authors?: GitUser[];
cursor?: string;
limit?: number;
merges?: boolean;
merges?: boolean | 'first-parent';
ordering?: 'date' | 'author-date' | 'topo' | null;
ref?: string;
since?: string;
Expand All @@ -1905,7 +1905,7 @@ export class GitHubGitProvider implements GitProvider, Disposable {
options?: {
authors?: GitUser[];
limit?: number;
merges?: boolean;
merges?: boolean | 'first-parent';
ordering?: 'date' | 'author-date' | 'topo' | null;
ref?: string;
},
Expand Down

0 comments on commit efcc3ca

Please sign in to comment.