Skip to content

Commit 02c6535

Browse files
eamodioeamodio
committed
Refactors stash SHA remapping for graph
 - Centralizes logic for remapping parent SHAs (index/untracked files) - Avoids complexity outside of raw data handling Fixes change statistics for stashes
1 parent 8242cd1 commit 02c6535

File tree

5 files changed

+129
-112
lines changed

5 files changed

+129
-112
lines changed

‎src/env/node/git/sub-providers/graph.ts‎

Lines changed: 77 additions & 55 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@ import {
3737
getBranchNameWithoutRemote,
3838
getRemoteNameFromBranchName,
3939
} from '../../../../git/utils/branch.utils';
40+
import { getChangedFilesCount } from '../../../../git/utils/commit.utils';
4041
import { createReference } from '../../../../git/utils/reference.utils';
4142
import { isUncommittedStaged } from '../../../../git/utils/revision.utils';
4243
import { getTagId } from '../../../../git/utils/tag.utils';
@@ -135,14 +136,13 @@ export class GraphGitSubProvider implements GitGraphSubProvider {
135136

136137
// TODO@eamodio this is insanity -- there *HAS* to be a better way to get git log to return stashes
137138
const gitStash = getSettledValue(stashResult);
138-
const { stdin, stashes } = convertStashesToStdin(gitStash?.stashes);
139+
const { stdin, stashes, remappedIds } = convertStashesToStdin(gitStash?.stashes);
139140

140141
const useAvatars = configuration.get('graph.avatars', undefined, true);
141142

142143
const avatars = new Map<string, string>();
143144
const ids = new Set<string>();
144145
const reachableFromHEAD = new Set<string>();
145-
const remappedIds = new Map<string, string>();
146146
const rowStats: GitGraphRowsStats = new Map<string, GitGraphRowStats>();
147147
let pendingRowsStatsCount = 0;
148148
let iterations = 0;
@@ -208,6 +208,7 @@ export class GraphGitSubProvider implements GitGraphSubProvider {
208208
let remote: GitRemote | undefined;
209209
let remoteBranchId: string;
210210
let remoteName: string;
211+
let shaOrRemapped: string | undefined;
211212
let stash: GitStashCommit | undefined;
212213
let tagId: string;
213214
let tagName: string;
@@ -233,9 +234,11 @@ export class GraphGitSubProvider implements GitGraphSubProvider {
233234
if (ids.has(commit.sha)) continue;
234235

235236
total++;
236-
if (remappedIds.has(commit.sha)) continue;
237+
shaOrRemapped = remappedIds.get(commit.sha);
238+
if (shaOrRemapped && ids.has(shaOrRemapped)) continue;
239+
shaOrRemapped ??= commit.sha;
237240

238-
ids.add(commit.sha);
241+
ids.add(shaOrRemapped);
239242

240243
refHeads = [];
241244
refRemoteHeads = [];
@@ -279,7 +282,7 @@ export class GraphGitSubProvider implements GitGraphSubProvider {
279282

280283
if (tip.startsWith('HEAD')) {
281284
head = true;
282-
reachableFromHEAD.add(commit.sha);
285+
reachableFromHEAD.add(shaOrRemapped);
283286

284287
if (tip !== 'HEAD') {
285288
tip = tip.substring(8);
@@ -424,56 +427,67 @@ export class GraphGitSubProvider implements GitGraphSubProvider {
424427
}
425428
}
426429

427-
stash = gitStash?.stashes.get(commit.sha);
428-
429430
parents = commit.parents ? commit.parents.split(' ') : [];
430-
if (reachableFromHEAD.has(commit.sha)) {
431+
if (reachableFromHEAD.has(shaOrRemapped)) {
431432
for (parent of parents) {
432433
reachableFromHEAD.add(parent);
433434
}
434435
}
435436

436-
// Remove the second & third parent, if exists, from each stash commit as it is a Git implementation for the index and untracked files
437-
if (stash != null && parents.length > 1) {
438-
// Remap the "index commit" (e.g. contains staged files) of the stash
439-
remappedIds.set(parents[1], commit.sha);
440-
// Remap the "untracked commit" (e.g. contains untracked files) of the stash
441-
if (parents.length > 2) {
442-
remappedIds.set(parents[2], commit.sha);
443-
}
444-
parents.splice(1, 2);
445-
}
446-
447-
if (stash == null && !avatars.has(commit.authorEmail)) {
448-
avatarUri = getCachedAvatarUri(commit.authorEmail);
449-
if (avatarUri != null) {
450-
avatars.set(commit.authorEmail, avatarUri.toString(true));
451-
}
452-
}
453-
454-
isCurrentUser = isUserMatch(currentUser, commit.author, commit.authorEmail);
455-
437+
stash = gitStash?.stashes.get(shaOrRemapped);
456438
if (stash != null) {
457439
contexts.row = serializeWebviewItemContext<GraphItemRefContext>({
458440
webviewItem: 'gitlens:stash',
459441
webviewItemValue: {
460442
type: 'stash',
461-
ref: createReference(commit.sha, repoPath, {
443+
ref: createReference(shaOrRemapped, repoPath, {
462444
refType: 'stash',
463445
name: stash.name,
464446
message: stash.message,
465447
number: stash.stashNumber,
466448
}),
467449
},
468450
});
451+
452+
rows.push({
453+
sha: shaOrRemapped,
454+
// Always only return the first parent for stashes, as it is a Git implementation for the index and untracked files
455+
parents: parents.slice(0, 1),
456+
author: 'You',
457+
email: commit.authorEmail,
458+
date: Number(ordering === 'author-date' ? commit.authorDate : commit.committerDate) * 1000,
459+
message: emojify(stash.message ?? commit.message.trim()),
460+
type: 'stash-node',
461+
heads: refHeads,
462+
remotes: refRemoteHeads,
463+
tags: refTags,
464+
contexts: contexts,
465+
});
466+
467+
if (stash.stats != null) {
468+
rowStats.set(shaOrRemapped, {
469+
files: getChangedFilesCount(stash.stats.files),
470+
additions: stash.stats.additions,
471+
deletions: stash.stats.deletions,
472+
});
473+
}
469474
} else {
475+
isCurrentUser = isUserMatch(currentUser, commit.author, commit.authorEmail);
476+
477+
if (!avatars.has(commit.authorEmail)) {
478+
avatarUri = getCachedAvatarUri(commit.authorEmail);
479+
if (avatarUri != null) {
480+
avatars.set(commit.authorEmail, avatarUri.toString(true));
481+
}
482+
}
483+
470484
contexts.row = serializeWebviewItemContext<GraphItemRefContext>({
471485
webviewItem: `gitlens:commit${head ? '+HEAD' : ''}${
472-
reachableFromHEAD.has(commit.sha) ? '+current' : ''
486+
reachableFromHEAD.has(shaOrRemapped) ? '+current' : ''
473487
}`,
474488
webviewItemValue: {
475489
type: 'commit',
476-
ref: createReference(commit.sha, repoPath, {
490+
ref: createReference(shaOrRemapped, repoPath, {
477491
refType: 'revision',
478492
message: commit.message,
479493
}),
@@ -490,25 +504,24 @@ export class GraphGitSubProvider implements GitGraphSubProvider {
490504
current: isCurrentUser,
491505
},
492506
});
493-
}
494507

495-
rows.push({
496-
sha: commit.sha,
497-
parents: onlyFollowFirstParent ? [parents[0]] : parents,
498-
author: isCurrentUser ? 'You' : commit.author,
499-
email: commit.authorEmail,
500-
date: Number(ordering === 'author-date' ? commit.authorDate : commit.committerDate) * 1000,
501-
message: emojify(commit.message.trim()),
502-
// TODO: review logic for stash, wip, etc
503-
type: stash != null ? 'stash-node' : parents.length > 1 ? 'merge-node' : 'commit-node',
504-
heads: refHeads,
505-
remotes: refRemoteHeads,
506-
tags: refTags,
507-
contexts: contexts,
508-
});
508+
rows.push({
509+
sha: shaOrRemapped,
510+
parents: onlyFollowFirstParent ? parents.slice(0, 1) : parents,
511+
author: isCurrentUser ? 'You' : commit.author,
512+
email: commit.authorEmail,
513+
date: Number(ordering === 'author-date' ? commit.authorDate : commit.committerDate) * 1000,
514+
message: emojify(commit.message.trim()),
515+
type: parents.length > 1 ? 'merge-node' : 'commit-node',
516+
heads: refHeads,
517+
remotes: refRemoteHeads,
518+
tags: refTags,
519+
contexts: contexts,
520+
});
509521

510-
if (commit.stats != null) {
511-
rowStats.set(commit.sha, commit.stats);
522+
if (commit.stats != null) {
523+
rowStats.set(shaOrRemapped, commit.stats);
524+
}
512525
}
513526
}
514527

@@ -541,9 +554,14 @@ export class GraphGitSubProvider implements GitGraphSubProvider {
541554
);
542555

543556
if (statsResult.stdout) {
544-
const commitStats = statsParser.parse(statsResult.stdout);
545-
for (const stat of commitStats) {
546-
rowStats.set(stat.sha, stat.stats);
557+
let statShaOrRemapped;
558+
for (const stat of statsParser.parse(statsResult.stdout)) {
559+
statShaOrRemapped = remappedIds.get(stat.sha) ?? stat.sha;
560+
561+
// If we already have the stats for this sha, skip it (e.g. stashes)
562+
if (rowStats.has(statShaOrRemapped)) continue;
563+
564+
rowStats.set(statShaOrRemapped, stat.stats);
547565
}
548566
}
549567
} finally {
@@ -563,7 +581,6 @@ export class GraphGitSubProvider implements GitGraphSubProvider {
563581
avatars: avatars,
564582
ids: ids,
565583
includes: options?.include,
566-
remappedIds: remappedIds,
567584
branches: branchMap,
568585
remotes: remoteMap,
569586
downstreams: downstreamMap,
@@ -635,13 +652,16 @@ export class GraphGitSubProvider implements GitGraphSubProvider {
635652

636653
let stashes: Map<string, GitStashCommit> | undefined;
637654
let stdin: string | undefined;
655+
let remappedIds: Map<string, string>;
638656

639657
if (shas?.size) {
640658
stdin = join(shas, '\n');
641659
args.push('--no-walk');
660+
661+
remappedIds = new Map();
642662
} else {
643663
// TODO@eamodio this is insanity -- there *HAS* to be a better way to get git log to return stashes
644-
({ stdin, stashes } = convertStashesToStdin(
664+
({ stdin, stashes, remappedIds } = convertStashesToStdin(
645665
await this.provider.stash?.getStash(repoPath, undefined, cancellation),
646666
));
647667
}
@@ -696,6 +716,7 @@ export class GraphGitSubProvider implements GitGraphSubProvider {
696716

697717
let count = 0;
698718
let hasMore = false;
719+
let sha;
699720
const stashesOnly = filters.type === 'stash';
700721

701722
for await (const r of parser.parseAsync(stream)) {
@@ -707,11 +728,12 @@ export class GraphGitSubProvider implements GitGraphSubProvider {
707728
}
708729

709730
count++;
710-
if (results.has(r.sha) || (stashesOnly && !stashes?.has(r.sha))) {
731+
sha = remappedIds.get(r.sha) ?? r.sha;
732+
if (results.has(sha) || (stashesOnly && !stashes?.has(sha))) {
711733
continue;
712734
}
713735

714-
results.set(r.sha, {
736+
results.set(sha, {
715737
i: results.size,
716738
date: Number(options?.ordering === 'author-date' ? r.authorDate : r.committerDate) * 1000,
717739
});

‎src/env/node/git/sub-providers/stash.ts‎

Lines changed: 28 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -19,16 +19,12 @@ import { splitPath } from '../../../../system/-webview/path';
1919
import { countStringLength } from '../../../../system/array';
2020
import { gate } from '../../../../system/decorators/-webview/gate';
2121
import { log } from '../../../../system/decorators/log';
22-
import { join, map, min, skip } from '../../../../system/iterable';
22+
import { min, skip } from '../../../../system/iterable';
2323
import { getSettledValue } from '../../../../system/promise';
2424
import type { Git } from '../git';
2525
import { GitError, maxGitCliLength } from '../git';
2626
import { createCommitFileset } from './commits';
2727

28-
const stashSummaryRegex =
29-
// eslint-disable-next-line no-control-regex
30-
/(?:(?:(?<wip>WIP) on|On) (?<onref>[^/](?!.*\/\.)(?!.*\.\.)(?!.*\/\/)(?!.*@\{)[^\x00-\x1F\x7F ~^:?*[\\]+[^./]):\s*)?(?<summary>.*)$/s;
31-
3228
export class StashGitSubProvider implements GitStashSubProvider {
3329
constructor(
3430
private readonly container: Container,
@@ -349,10 +345,12 @@ export class StashGitSubProvider implements GitStashSubProvider {
349345
}
350346

351347
export function convertStashesToStdin(stashOrStashes: GitStash | Map<string, GitStashCommit> | undefined): {
352-
stdin: string | undefined;
353-
stashes: Map<string, GitStashCommit>;
348+
readonly stdin: string | undefined;
349+
readonly stashes: Map<string, GitStashCommit>;
350+
readonly remappedIds: Map<string, string>;
354351
} {
355-
if (stashOrStashes == null) return { stdin: undefined, stashes: new Map() };
352+
const remappedIds = new Map<string, string>();
353+
if (stashOrStashes == null) return { stdin: undefined, stashes: new Map(), remappedIds: remappedIds };
356354

357355
let stdin: string | undefined;
358356
let stashes: Map<string, GitStashCommit>;
@@ -365,22 +363,39 @@ export function convertStashesToStdin(stashOrStashes: GitStash | Map<string, Git
365363
stdin += `${stash.sha.substring(0, 9)}\n`;
366364
// Include the stash's 2nd (index files) and 3rd (untracked files) parents
367365
for (const p of skip(stash.parents, 1)) {
366+
remappedIds.set(p, stash.sha);
367+
368368
stashes.set(p, stash);
369369
stdin += `${p.substring(0, 9)}\n`;
370370
}
371371
}
372372
}
373373
} else {
374-
stdin = join(
375-
map(stashOrStashes.values(), c => c.sha.substring(0, 9)),
376-
'\n',
377-
);
374+
if (stashOrStashes.size) {
375+
stdin = '';
376+
for (const stash of stashOrStashes.values()) {
377+
stdin += `${stash.sha.substring(0, 9)}\n`;
378+
// Include the stash's 2nd (index files) and 3rd (untracked files) parents (if they aren't already in the map)
379+
for (const p of skip(stash.parents, 1)) {
380+
remappedIds.set(p, stash.sha);
381+
382+
if (!stashOrStashes.has(p)) {
383+
stashOrStashes.set(p, stash);
384+
stdin += `${p.substring(0, 9)}\n`;
385+
}
386+
}
387+
}
388+
}
378389
stashes = stashOrStashes;
379390
}
380391

381-
return { stdin: stdin || undefined, stashes: stashes };
392+
return { stdin: stdin || undefined, stashes: stashes, remappedIds: remappedIds };
382393
}
383394

395+
const stashSummaryRegex =
396+
// eslint-disable-next-line no-control-regex
397+
/(?:(?:(?<wip>WIP) on|On) (?<onref>[^/](?!.*\/\.)(?!.*\.\.)(?!.*\/\/)(?!.*@\{)[^\x00-\x1F\x7F ~^:?*[\\]+[^./]):\s*)?(?<summary>.*)$/s;
398+
384399
function createStash(container: Container, s: ParsedStash, repoPath: string): GitStashCommit {
385400
let message = s.summary.trim();
386401

‎src/git/models/graph.ts‎

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -43,10 +43,6 @@ export interface GitGraph {
4343
/** A set of all "seen" commit ids */
4444
readonly ids: Set<string>;
4545
readonly includes: { stats?: boolean } | undefined;
46-
/** A set of all remapped commit ids -- typically for stash index/untracked commits
47-
* (key = remapped from id, value = remapped to id)
48-
*/
49-
readonly remappedIds?: Map<string, string>;
5046
readonly branches: Map<string, GitBranch>;
5147
readonly remotes: Map<string, GitRemote>;
5248
readonly downstreams: Map<string, string[]>;

0 commit comments

Comments
 (0)