From efcc3ca65dd412a278ff817e4cbafd57aa3b6053 Mon Sep 17 00:00:00 2001 From: Eric Amodio Date: Mon, 27 Nov 2023 00:05:45 -0500 Subject: [PATCH] Fixes #2482 lazy loads files for merge commits Consolidated log into log2 calls (and renames it) --- CHANGELOG.md | 4 ++ src/env/node/git/git.ts | 79 --------------------- src/env/node/git/localGitProvider.ts | 82 ++++++++++++++-------- src/git/gitProvider.ts | 2 +- src/git/gitProviderService.ts | 2 +- src/git/parsers/logParser.ts | 9 --- .../providers/github/githubGitProvider.ts | 6 +- 7 files changed, 63 insertions(+), 121 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a5c8cd3..583bcfd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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 diff --git a/src/env/node/git/git.ts b/src/env/node/git/git.ts index f6f3899..b585cdd 100644 --- a/src/env/node/git/git.ts +++ b/src/env/node/git/git.ts @@ -1083,85 +1083,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({ cwd: repoPath, configs: gitLogDefaultConfigsWithFiles }, ...params, '--'); - } - - log2( - repoPath: string, options?: { cancellation?: CancellationToken; configs?: readonly string[]; diff --git a/src/env/node/git/localGitProvider.ts b/src/env/node/git/localGitProvider.ts index 0592100..66a7bce 100644 --- a/src/env/node/git/localGitProvider.ts +++ b/src/env/node/git/localGitProvider.ts @@ -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), @@ -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)) { @@ -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) { @@ -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(); @@ -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) { @@ -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`); } @@ -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, @@ -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); } @@ -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; @@ -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; @@ -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, diff --git a/src/git/gitProvider.ts b/src/git/gitProvider.ts index 83a4e2b..7c98096 100644 --- a/src/git/gitProvider.ts +++ b/src/git/gitProvider.ts @@ -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; diff --git a/src/git/gitProviderService.ts b/src/git/gitProviderService.ts index 2e8d3e0..ba3d82d 100644 --- a/src/git/gitProviderService.ts +++ b/src/git/gitProviderService.ts @@ -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; diff --git a/src/git/parsers/logParser.ts b/src/git/parsers/logParser.ts index 2a57321..f05476f 100644 --- a/src/git/parsers/logParser.ts +++ b/src/git/parsers/logParser.ts @@ -527,15 +527,6 @@ export function parseGitLog( let hasFiles = true; if (next.done || next.value === '') { - // 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; } diff --git a/src/plus/integrations/providers/github/githubGitProvider.ts b/src/plus/integrations/providers/github/githubGitProvider.ts index 7bb9705..ecdb074 100644 --- a/src/plus/integrations/providers/github/githubGitProvider.ts +++ b/src/plus/integrations/providers/github/githubGitProvider.ts @@ -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; @@ -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; @@ -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; },