From fe3de81c5806baef647c88ba19db63cb7bb8df65 Mon Sep 17 00:00:00 2001 From: Dan Arad Date: Sat, 20 Mar 2021 16:55:53 +0200 Subject: [PATCH 01/27] Select the parent with which to view the diff Users can now press ctrl key + parent hash (from the details view) to switch the commit we see the diff with. The parents are presented on separate lines, and the chosen parent is in bold. This is done by saving the current parent we view the diff with in the frontend and each time we request commit details we send the parent we want to diff with. --- src/dataSource.ts | 4 ++-- src/gitGraphView.ts | 3 ++- src/types.ts | 2 ++ web/findWidget.ts | 2 +- web/global.d.ts | 1 + web/main.ts | 50 ++++++++++++++++++++++++++++----------------- web/utils.ts | 2 ++ 7 files changed, 41 insertions(+), 23 deletions(-) diff --git a/src/dataSource.ts b/src/dataSource.ts index 92bd6b6e..6640e25c 100644 --- a/src/dataSource.ts +++ b/src/dataSource.ts @@ -341,8 +341,8 @@ export class DataSource extends Disposable { * @param hasParents Does the commit have parents * @returns The commit details. */ - public getCommitDetails(repo: string, commitHash: string, hasParents: boolean): Promise { - const fromCommit = commitHash + (hasParents ? '^' : ''); + public getCommitDetails(repo: string, commitHash: string, hasParents: boolean, parentIndex: number): Promise { + const fromCommit = commitHash + (hasParents ? `^${parentIndex}` : ''); return Promise.all([ this.getCommitDetailsBase(repo, commitHash), this.getDiffNameStatus(repo, fromCommit, commitHash), diff --git a/src/gitGraphView.ts b/src/gitGraphView.ts index 3fa92f21..a9c5043e 100644 --- a/src/gitGraphView.ts +++ b/src/gitGraphView.ts @@ -232,13 +232,14 @@ export class GitGraphView extends Disposable { msg.commitHash === UNCOMMITTED ? this.dataSource.getUncommittedDetails(msg.repo) : msg.stash === null - ? this.dataSource.getCommitDetails(msg.repo, msg.commitHash, msg.hasParents) + ? this.dataSource.getCommitDetails(msg.repo, msg.commitHash, msg.hasParents, msg.parentIndex) : this.dataSource.getStashDetails(msg.repo, msg.commitHash, msg.stash), msg.avatarEmail !== null ? this.avatarManager.getAvatarImage(msg.avatarEmail) : Promise.resolve(null) ]); this.sendMessage({ command: 'commitDetails', ...data[0], + parentIndex: msg.parentIndex, avatar: data[1], codeReview: msg.commitHash !== UNCOMMITTED ? this.extensionState.getCodeReview(msg.repo, msg.commitHash) : null, refresh: msg.refresh diff --git a/src/types.ts b/src/types.ts index 80176974..f325440a 100644 --- a/src/types.ts +++ b/src/types.ts @@ -651,6 +651,7 @@ export interface RequestCommitDetails extends RepoRequest { readonly command: 'commitDetails'; readonly commitHash: string; readonly hasParents: boolean; + readonly parentIndex: number; readonly stash: GitCommitStash | null; // null => request is for a commit, otherwise => request is for a stash readonly avatarEmail: string | null; // string => fetch avatar with the given email, null => don't fetch avatar readonly refresh: boolean; @@ -658,6 +659,7 @@ export interface RequestCommitDetails extends RepoRequest { export interface ResponseCommitDetails extends ResponseWithErrorInfo { readonly command: 'commitDetails'; readonly commitDetails: GitCommitDetails | null; + readonly parentIndex: number; readonly avatar: string | null; readonly codeReview: CodeReview | null; readonly refresh: boolean; diff --git a/web/findWidget.ts b/web/findWidget.ts index ff0e0974..9ec5765d 100644 --- a/web/findWidget.ts +++ b/web/findWidget.ts @@ -397,7 +397,7 @@ class FindWidget { if (commitHash !== null && !this.view.isCdvOpen(commitHash, null)) { const commitElem = findCommitElemWithId(getCommitElems(), this.view.getCommitId(commitHash)); if (commitElem !== null) { - this.view.loadCommitDetails(commitElem); + this.view.loadCommitDetails(commitElem, DEFAULT_PARENT_INDEX); } } } diff --git a/web/global.d.ts b/web/global.d.ts index 5e1072ff..a4bafd06 100644 --- a/web/global.d.ts +++ b/web/global.d.ts @@ -25,6 +25,7 @@ declare global { index: number; commitHash: string; commitElem: HTMLElement | null; + parentIndex: number; compareWithHash: string | null; compareWithElem: HTMLElement | null; commitDetails: GG.GitCommitDetails | null; diff --git a/web/main.ts b/web/main.ts index 95fcaf9c..fb5e0231 100644 --- a/web/main.ts +++ b/web/main.ts @@ -322,7 +322,7 @@ class GitGraphView { if (this.expandedCommit.compareWithHash === null) { // Commit Details View is open if (this.expandedCommit.commitHash === UNCOMMITTED) { - this.requestCommitDetails(this.expandedCommit.commitHash, true); + this.requestCommitDetails(this.expandedCommit.commitHash, true, this.expandedCommit.parentIndex); } } else { // Commit Comparison is open @@ -424,7 +424,7 @@ class GitGraphView { if (compareWithElem !== null) { this.loadCommitComparison(commitElem, compareWithElem); } else { - this.loadCommitDetails(commitElem); + this.loadCommitDetails(commitElem, DEFAULT_PARENT_INDEX); } } else { showErrorMessage('Unable to resume Code Review, it could not be found in the latest ' + this.maxCommits + ' commits that were loaded in this repository.'); @@ -660,13 +660,14 @@ class GitGraphView { this.settingsWidget.refresh(); } - public requestCommitDetails(hash: string, refresh: boolean) { + public requestCommitDetails(hash: string, refresh: boolean, parentIndex: number) { let commit = this.commits[this.commitLookup[hash]]; sendMessage({ command: 'commitDetails', repo: this.currentRepo, commitHash: hash, hasParents: commit.parents.length > 0, + parentIndex: parentIndex, stash: commit.stash, avatarEmail: this.config.fetchAvatars && hash !== UNCOMMITTED ? commit.email : null, refresh: refresh @@ -746,6 +747,7 @@ class GitGraphView { index: index, commitHash: commitHash, commitElem: commitElem, + parentIndex: DEFAULT_PARENT_INDEX, compareWithHash: compareWithHash, compareWithElem: compareWithElem, commitDetails: null, @@ -900,12 +902,12 @@ class GitGraphView { if (expandedCommit.compareWithHash === null) { // Commit Details View is open if (!expandedCommit.loading && expandedCommit.commitDetails !== null && expandedCommit.fileTree !== null) { - this.showCommitDetails(expandedCommit.commitDetails, expandedCommit.fileTree, expandedCommit.avatar, expandedCommit.codeReview, expandedCommit.lastViewedFile, true); + this.showCommitDetails(expandedCommit.commitDetails, expandedCommit.parentIndex, expandedCommit.fileTree, expandedCommit.avatar, expandedCommit.codeReview, expandedCommit.lastViewedFile, true); if (expandedCommit.commitHash === UNCOMMITTED) { - this.requestCommitDetails(expandedCommit.commitHash, true); + this.requestCommitDetails(expandedCommit.commitHash, true, expandedCommit.parentIndex); } } else { - this.loadCommitDetails(commitElem); + this.loadCommitDetails(commitElem, expandedCommit.parentIndex); } } else { // Commit Comparison is open @@ -2006,7 +2008,7 @@ class GitGraphView { if (newHashIndex > -1) { handledEvent(e); const elem = findCommitElemWithId(getCommitElems(), newHashIndex); - if (elem !== null) this.loadCommitDetails(elem); + if (elem !== null) this.loadCommitDetails(elem, this.expandedCommit.parentIndex); } } else if (e.key && (e.ctrlKey || e.metaKey)) { const key = e.key.toLowerCase(), keybindings = this.config.keybindings; @@ -2054,7 +2056,15 @@ class GitGraphView { case 'commit': if (typeof this.commitLookup[value] === 'number' && (this.expandedCommit === null || this.expandedCommit.commitHash !== value || this.expandedCommit.compareWithHash !== null)) { const elem = findCommitElemWithId(getCommitElems(), this.commitLookup[value]); - if (elem !== null) this.loadCommitDetails(elem); + if (elem === null) return; + if (e.ctrlKey || e.metaKey) { + if(this.expandedCommit !== null && this.expandedCommit.commitElem !== null && this.expandedCommit.commitDetails !== null) { + const parentIndex = this.expandedCommit.commitDetails.parents.indexOf(value) + 1; + this.loadCommitDetails(this.expandedCommit.commitElem, parentIndex); + } + } else { + this.loadCommitDetails(elem, DEFAULT_PARENT_INDEX); + } } break; } @@ -2164,10 +2174,10 @@ class GitGraphView { this.loadCommitComparison(this.expandedCommit.commitElem, eventElem); } } else { - this.loadCommitDetails(eventElem); + this.loadCommitDetails(eventElem, this.expandedCommit.parentIndex); } } else { - this.loadCommitDetails(eventElem); + this.loadCommitDetails(eventElem, DEFAULT_PARENT_INDEX); } } }); @@ -2284,7 +2294,7 @@ class GitGraphView { /* Commit Details View */ - public loadCommitDetails(commitElem: HTMLElement) { + public loadCommitDetails(commitElem: HTMLElement, parentIndex: number) { const commit = this.getCommitOfElem(commitElem); if (commit === null) return; @@ -2292,7 +2302,7 @@ class GitGraphView { this.saveExpandedCommitLoading(parseInt(commitElem.dataset.id!), commit.hash, commitElem, null, null); commitElem.classList.add(CLASS_COMMIT_DETAILS_OPEN); this.renderCommitDetailsView(false); - this.requestCommitDetails(commit.hash, false); + this.requestCommitDetails(commit.hash, false, parentIndex); } public closeCommitDetails(saveAndRender: boolean) { @@ -2322,7 +2332,7 @@ class GitGraphView { } } - public showCommitDetails(commitDetails: GG.GitCommitDetails, fileTree: FileTreeFolder, avatar: string | null, codeReview: GG.CodeReview | null, lastViewedFile: string | null, refresh: boolean) { + public showCommitDetails(commitDetails: GG.GitCommitDetails, parentIndex: number, fileTree: FileTreeFolder, avatar: string | null, codeReview: GG.CodeReview | null, lastViewedFile: string | null, refresh: boolean) { const expandedCommit = this.expandedCommit; if (expandedCommit === null || expandedCommit.commitElem === null || expandedCommit.commitHash !== commitDetails.hash || expandedCommit.compareWithHash !== null) return; @@ -2337,6 +2347,7 @@ class GitGraphView { expandedCommit.fileTree = fileTree; GitGraphView.closeCdvContextMenuIfOpen(expandedCommit); } + expandedCommit.parentIndex = parentIndex; expandedCommit.avatar = avatar; expandedCommit.codeReview = codeReview; if (!refresh) { @@ -2415,7 +2426,7 @@ class GitGraphView { if (expandedCommit.commitElem !== null) { this.saveExpandedCommitLoading(expandedCommit.index, expandedCommit.commitHash, expandedCommit.commitElem, null, null); this.renderCommitDetailsView(false); - this.requestCommitDetails(expandedCommit.commitHash, false); + this.requestCommitDetails(expandedCommit.commitHash, false, expandedCommit.parentIndex); } else { this.closeCommitDetails(true); } @@ -2484,16 +2495,17 @@ class GitGraphView { }); const commitDetails = expandedCommit.commitDetails!; const parents = commitDetails.parents.length > 0 - ? commitDetails.parents.map((parent) => { + ? commitDetails.parents.map((parent, parentIndex) => { const escapedParent = escapeHtml(parent); + const isComparedToParent = parentIndex + 1 === expandedCommit.parentIndex; return typeof this.commitLookup[parent] === 'number' - ? '' + escapedParent + '' + ? `  ${isComparedToParent ? '' : ''}#${parentIndex + 1} ${abbrevCommit(escapedParent)}${isComparedToParent ? '' : ''}` : escapedParent; - }).join(', ') + }).join('
') : 'None'; html += '' + 'Commit: ' + escapeHtml(commitDetails.hash) + '
' - + 'Parents: ' + parents + '
' + + 'Parents:
' + parents + '
' + 'Author: ' + escapeHtml(commitDetails.author) + (commitDetails.authorEmail !== '' ? ' <' + escapeHtml(commitDetails.authorEmail) + '>' : '') + '
' + (commitDetails.authorDate !== commitDetails.committerDate ? 'Author Date: ' + formatLongDate(commitDetails.authorDate) + '
' : '') + 'Committer: ' + escapeHtml(commitDetails.committer) + (commitDetails.committerEmail !== '' ? ' <' + escapeHtml(commitDetails.committerEmail) + '>' : '') + (commitDetails.signature !== null ? generateSignatureHtml(commitDetails.signature) : '') + '
' @@ -3130,7 +3142,7 @@ window.addEventListener('load', () => { break; case 'commitDetails': if (msg.commitDetails !== null) { - gitGraph.showCommitDetails(msg.commitDetails, gitGraph.createFileTree(msg.commitDetails.fileChanges, msg.codeReview), msg.avatar, msg.codeReview, msg.codeReview !== null ? msg.codeReview.lastViewedFile : null, msg.refresh); + gitGraph.showCommitDetails(msg.commitDetails, msg.parentIndex, gitGraph.createFileTree(msg.commitDetails.fileChanges, msg.codeReview), msg.avatar, msg.codeReview, msg.codeReview !== null ? msg.codeReview.lastViewedFile : null, msg.refresh); } else { gitGraph.closeCommitDetails(true); dialog.showError('Unable to load Commit Details', msg.error, null, null); diff --git a/web/utils.ts b/web/utils.ts index a63b64c8..b4066463 100644 --- a/web/utils.ts +++ b/web/utils.ts @@ -100,6 +100,8 @@ const CSS_PROP_LIMIT_GRAPH_WIDTH = '--limitGraphWidth'; const ATTR_ERROR = 'data-error'; +const DEFAULT_PARENT_INDEX = 1; + /* General Helpers */ From 90f7b53637a1755227c751643da7f899435910d8 Mon Sep 17 00:00:00 2001 From: Dan Arad Date: Sat, 20 Mar 2021 17:33:17 +0200 Subject: [PATCH 02/27] Fix existing tests --- tests/dataSource.test.ts | 50 ++++++++++++++++++++-------------------- 1 file changed, 25 insertions(+), 25 deletions(-) diff --git a/tests/dataSource.test.ts b/tests/dataSource.test.ts index d722503a..12283570 100644 --- a/tests/dataSource.test.ts +++ b/tests/dataSource.test.ts @@ -2630,7 +2630,7 @@ describe('DataSource', () => { mockGitSuccessOnce(['0 0 dir/deleted.txt', '1 1 dir/modified.txt', '2 3 ', 'dir/renamed-old.txt', 'dir/renamed-new.txt', ''].join('\0')); // Run - const result = await dataSource.getCommitDetails('/path/to/repo', '1a2b3c4d5e6f1a2b3c4d5e6f1a2b3c4d5e6f1a2b', true); + const result = await dataSource.getCommitDetails('/path/to/repo', '1a2b3c4d5e6f1a2b3c4d5e6f1a2b3c4d5e6f1a2b', true, 1); // Assert expect(result).toStrictEqual({ @@ -2672,8 +2672,8 @@ describe('DataSource', () => { error: null }); expect(spyOnSpawn).toBeCalledWith('/path/to/git', ['-c', 'log.showSignature=false', 'show', '--quiet', '1a2b3c4d5e6f1a2b3c4d5e6f1a2b3c4d5e6f1a2b', '--format=%HXX7Nal-YARtTpjCikii9nJxER19D6diSyk-AWkPb%PXX7Nal-YARtTpjCikii9nJxER19D6diSyk-AWkPb%anXX7Nal-YARtTpjCikii9nJxER19D6diSyk-AWkPb%aeXX7Nal-YARtTpjCikii9nJxER19D6diSyk-AWkPb%atXX7Nal-YARtTpjCikii9nJxER19D6diSyk-AWkPb%cnXX7Nal-YARtTpjCikii9nJxER19D6diSyk-AWkPb%ceXX7Nal-YARtTpjCikii9nJxER19D6diSyk-AWkPb%ctXX7Nal-YARtTpjCikii9nJxER19D6diSyk-AWkPbXX7Nal-YARtTpjCikii9nJxER19D6diSyk-AWkPbXX7Nal-YARtTpjCikii9nJxER19D6diSyk-AWkPbXX7Nal-YARtTpjCikii9nJxER19D6diSyk-AWkPb%B'], expect.objectContaining({ cwd: '/path/to/repo' })); - expect(spyOnSpawn).toBeCalledWith('/path/to/git', ['diff', '--name-status', '--find-renames', '--diff-filter=AMDR', '-z', '1a2b3c4d5e6f1a2b3c4d5e6f1a2b3c4d5e6f1a2b^', '1a2b3c4d5e6f1a2b3c4d5e6f1a2b3c4d5e6f1a2b'], expect.objectContaining({ cwd: '/path/to/repo' })); - expect(spyOnSpawn).toBeCalledWith('/path/to/git', ['diff', '--numstat', '--find-renames', '--diff-filter=AMDR', '-z', '1a2b3c4d5e6f1a2b3c4d5e6f1a2b3c4d5e6f1a2b^', '1a2b3c4d5e6f1a2b3c4d5e6f1a2b3c4d5e6f1a2b'], expect.objectContaining({ cwd: '/path/to/repo' })); + expect(spyOnSpawn).toBeCalledWith('/path/to/git', ['diff', '--name-status', '--find-renames', '--diff-filter=AMDR', '-z', '1a2b3c4d5e6f1a2b3c4d5e6f1a2b3c4d5e6f1a2b^1', '1a2b3c4d5e6f1a2b3c4d5e6f1a2b3c4d5e6f1a2b'], expect.objectContaining({ cwd: '/path/to/repo' })); + expect(spyOnSpawn).toBeCalledWith('/path/to/git', ['diff', '--numstat', '--find-renames', '--diff-filter=AMDR', '-z', '1a2b3c4d5e6f1a2b3c4d5e6f1a2b3c4d5e6f1a2b^1', '1a2b3c4d5e6f1a2b3c4d5e6f1a2b3c4d5e6f1a2b'], expect.objectContaining({ cwd: '/path/to/repo' })); }); it('Should return the commit details (commit doesn\'t have parents)', async () => { @@ -2683,7 +2683,7 @@ describe('DataSource', () => { mockGitSuccessOnce(['1a2b3c4d5e6f1a2b3c4d5e6f1a2b3c4d5e6f1a2b', '0 0 dir/deleted.txt', '1 1 dir/modified.txt', '2 3 ', 'dir/renamed-old.txt', 'dir/renamed-new.txt', ''].join('\0')); // Run - const result = await dataSource.getCommitDetails('/path/to/repo', '1a2b3c4d5e6f1a2b3c4d5e6f1a2b3c4d5e6f1a2b', false); + const result = await dataSource.getCommitDetails('/path/to/repo', '1a2b3c4d5e6f1a2b3c4d5e6f1a2b3c4d5e6f1a2b', false, 1); // Assert expect(result).toStrictEqual({ @@ -2742,7 +2742,7 @@ describe('DataSource', () => { onDidChangeConfiguration.emit({ affectsConfiguration: (section) => section === 'git-graph.repository.commits.showSignatureStatus' }); - const result = await dataSource.getCommitDetails('/path/to/repo', '1a2b3c4d5e6f1a2b3c4d5e6f1a2b3c4d5e6f1a2b', true); + const result = await dataSource.getCommitDetails('/path/to/repo', '1a2b3c4d5e6f1a2b3c4d5e6f1a2b3c4d5e6f1a2b', true, 1); // Assert expect(result).toStrictEqual({ @@ -2788,8 +2788,8 @@ describe('DataSource', () => { error: null }); expect(spyOnSpawn).toBeCalledWith('/path/to/git', ['-c', 'log.showSignature=false', 'show', '--quiet', '1a2b3c4d5e6f1a2b3c4d5e6f1a2b3c4d5e6f1a2b', '--format=%HXX7Nal-YARtTpjCikii9nJxER19D6diSyk-AWkPb%PXX7Nal-YARtTpjCikii9nJxER19D6diSyk-AWkPb%anXX7Nal-YARtTpjCikii9nJxER19D6diSyk-AWkPb%aeXX7Nal-YARtTpjCikii9nJxER19D6diSyk-AWkPb%atXX7Nal-YARtTpjCikii9nJxER19D6diSyk-AWkPb%cnXX7Nal-YARtTpjCikii9nJxER19D6diSyk-AWkPb%ceXX7Nal-YARtTpjCikii9nJxER19D6diSyk-AWkPb%ctXX7Nal-YARtTpjCikii9nJxER19D6diSyk-AWkPb%G?XX7Nal-YARtTpjCikii9nJxER19D6diSyk-AWkPb%GSXX7Nal-YARtTpjCikii9nJxER19D6diSyk-AWkPb%GKXX7Nal-YARtTpjCikii9nJxER19D6diSyk-AWkPb%B'], expect.objectContaining({ cwd: '/path/to/repo' })); - expect(spyOnSpawn).toBeCalledWith('/path/to/git', ['diff', '--name-status', '--find-renames', '--diff-filter=AMDR', '-z', '1a2b3c4d5e6f1a2b3c4d5e6f1a2b3c4d5e6f1a2b^', '1a2b3c4d5e6f1a2b3c4d5e6f1a2b3c4d5e6f1a2b'], expect.objectContaining({ cwd: '/path/to/repo' })); - expect(spyOnSpawn).toBeCalledWith('/path/to/git', ['diff', '--numstat', '--find-renames', '--diff-filter=AMDR', '-z', '1a2b3c4d5e6f1a2b3c4d5e6f1a2b3c4d5e6f1a2b^', '1a2b3c4d5e6f1a2b3c4d5e6f1a2b3c4d5e6f1a2b'], expect.objectContaining({ cwd: '/path/to/repo' })); + expect(spyOnSpawn).toBeCalledWith('/path/to/git', ['diff', '--name-status', '--find-renames', '--diff-filter=AMDR', '-z', '1a2b3c4d5e6f1a2b3c4d5e6f1a2b3c4d5e6f1a2b^1', '1a2b3c4d5e6f1a2b3c4d5e6f1a2b3c4d5e6f1a2b'], expect.objectContaining({ cwd: '/path/to/repo' })); + expect(spyOnSpawn).toBeCalledWith('/path/to/git', ['diff', '--numstat', '--find-renames', '--diff-filter=AMDR', '-z', '1a2b3c4d5e6f1a2b3c4d5e6f1a2b3c4d5e6f1a2b^1', '1a2b3c4d5e6f1a2b3c4d5e6f1a2b3c4d5e6f1a2b'], expect.objectContaining({ cwd: '/path/to/repo' })); }); it('Should return the commit details (using git-graph.showSignatureStatus)', async () => { @@ -2805,7 +2805,7 @@ describe('DataSource', () => { onDidChangeConfiguration.emit({ affectsConfiguration: (section) => section === 'git-graph.showSignatureStatus' }); - const result = await dataSource.getCommitDetails('/path/to/repo', '1a2b3c4d5e6f1a2b3c4d5e6f1a2b3c4d5e6f1a2b', true); + const result = await dataSource.getCommitDetails('/path/to/repo', '1a2b3c4d5e6f1a2b3c4d5e6f1a2b3c4d5e6f1a2b', true, 1); // Assert expect(result).toStrictEqual({ @@ -2851,8 +2851,8 @@ describe('DataSource', () => { error: null }); expect(spyOnSpawn).toBeCalledWith('/path/to/git', ['-c', 'log.showSignature=false', 'show', '--quiet', '1a2b3c4d5e6f1a2b3c4d5e6f1a2b3c4d5e6f1a2b', '--format=%HXX7Nal-YARtTpjCikii9nJxER19D6diSyk-AWkPb%PXX7Nal-YARtTpjCikii9nJxER19D6diSyk-AWkPb%anXX7Nal-YARtTpjCikii9nJxER19D6diSyk-AWkPb%aeXX7Nal-YARtTpjCikii9nJxER19D6diSyk-AWkPb%atXX7Nal-YARtTpjCikii9nJxER19D6diSyk-AWkPb%cnXX7Nal-YARtTpjCikii9nJxER19D6diSyk-AWkPb%ceXX7Nal-YARtTpjCikii9nJxER19D6diSyk-AWkPb%ctXX7Nal-YARtTpjCikii9nJxER19D6diSyk-AWkPb%G?XX7Nal-YARtTpjCikii9nJxER19D6diSyk-AWkPb%GSXX7Nal-YARtTpjCikii9nJxER19D6diSyk-AWkPb%GKXX7Nal-YARtTpjCikii9nJxER19D6diSyk-AWkPb%B'], expect.objectContaining({ cwd: '/path/to/repo' })); - expect(spyOnSpawn).toBeCalledWith('/path/to/git', ['diff', '--name-status', '--find-renames', '--diff-filter=AMDR', '-z', '1a2b3c4d5e6f1a2b3c4d5e6f1a2b3c4d5e6f1a2b^', '1a2b3c4d5e6f1a2b3c4d5e6f1a2b3c4d5e6f1a2b'], expect.objectContaining({ cwd: '/path/to/repo' })); - expect(spyOnSpawn).toBeCalledWith('/path/to/git', ['diff', '--numstat', '--find-renames', '--diff-filter=AMDR', '-z', '1a2b3c4d5e6f1a2b3c4d5e6f1a2b3c4d5e6f1a2b^', '1a2b3c4d5e6f1a2b3c4d5e6f1a2b3c4d5e6f1a2b'], expect.objectContaining({ cwd: '/path/to/repo' })); + expect(spyOnSpawn).toBeCalledWith('/path/to/git', ['diff', '--name-status', '--find-renames', '--diff-filter=AMDR', '-z', '1a2b3c4d5e6f1a2b3c4d5e6f1a2b3c4d5e6f1a2b^1', '1a2b3c4d5e6f1a2b3c4d5e6f1a2b3c4d5e6f1a2b'], expect.objectContaining({ cwd: '/path/to/repo' })); + expect(spyOnSpawn).toBeCalledWith('/path/to/git', ['diff', '--numstat', '--find-renames', '--diff-filter=AMDR', '-z', '1a2b3c4d5e6f1a2b3c4d5e6f1a2b3c4d5e6f1a2b^1', '1a2b3c4d5e6f1a2b3c4d5e6f1a2b3c4d5e6f1a2b'], expect.objectContaining({ cwd: '/path/to/repo' })); }); it('Should return the commit details (without signature status if not available)', async () => { @@ -2869,7 +2869,7 @@ describe('DataSource', () => { path: '/path/to/git', version: '2.3.0' }); - const result = await dataSource.getCommitDetails('/path/to/repo', '1a2b3c4d5e6f1a2b3c4d5e6f1a2b3c4d5e6f1a2b', true); + const result = await dataSource.getCommitDetails('/path/to/repo', '1a2b3c4d5e6f1a2b3c4d5e6f1a2b3c4d5e6f1a2b', true, 1); // Assert expect(result).toStrictEqual({ @@ -2911,8 +2911,8 @@ describe('DataSource', () => { error: null }); expect(spyOnSpawn).toBeCalledWith('/path/to/git', ['-c', 'log.showSignature=false', 'show', '--quiet', '1a2b3c4d5e6f1a2b3c4d5e6f1a2b3c4d5e6f1a2b', '--format=%HXX7Nal-YARtTpjCikii9nJxER19D6diSyk-AWkPb%PXX7Nal-YARtTpjCikii9nJxER19D6diSyk-AWkPb%anXX7Nal-YARtTpjCikii9nJxER19D6diSyk-AWkPb%aeXX7Nal-YARtTpjCikii9nJxER19D6diSyk-AWkPb%atXX7Nal-YARtTpjCikii9nJxER19D6diSyk-AWkPb%cnXX7Nal-YARtTpjCikii9nJxER19D6diSyk-AWkPb%ceXX7Nal-YARtTpjCikii9nJxER19D6diSyk-AWkPb%ctXX7Nal-YARtTpjCikii9nJxER19D6diSyk-AWkPbXX7Nal-YARtTpjCikii9nJxER19D6diSyk-AWkPbXX7Nal-YARtTpjCikii9nJxER19D6diSyk-AWkPbXX7Nal-YARtTpjCikii9nJxER19D6diSyk-AWkPb%B'], expect.objectContaining({ cwd: '/path/to/repo' })); - expect(spyOnSpawn).toBeCalledWith('/path/to/git', ['diff', '--name-status', '--find-renames', '--diff-filter=AMDR', '-z', '1a2b3c4d5e6f1a2b3c4d5e6f1a2b3c4d5e6f1a2b^', '1a2b3c4d5e6f1a2b3c4d5e6f1a2b3c4d5e6f1a2b'], expect.objectContaining({ cwd: '/path/to/repo' })); - expect(spyOnSpawn).toBeCalledWith('/path/to/git', ['diff', '--numstat', '--find-renames', '--diff-filter=AMDR', '-z', '1a2b3c4d5e6f1a2b3c4d5e6f1a2b3c4d5e6f1a2b^', '1a2b3c4d5e6f1a2b3c4d5e6f1a2b3c4d5e6f1a2b'], expect.objectContaining({ cwd: '/path/to/repo' })); + expect(spyOnSpawn).toBeCalledWith('/path/to/git', ['diff', '--name-status', '--find-renames', '--diff-filter=AMDR', '-z', '1a2b3c4d5e6f1a2b3c4d5e6f1a2b3c4d5e6f1a2b^1', '1a2b3c4d5e6f1a2b3c4d5e6f1a2b3c4d5e6f1a2b'], expect.objectContaining({ cwd: '/path/to/repo' })); + expect(spyOnSpawn).toBeCalledWith('/path/to/git', ['diff', '--numstat', '--find-renames', '--diff-filter=AMDR', '-z', '1a2b3c4d5e6f1a2b3c4d5e6f1a2b3c4d5e6f1a2b^1', '1a2b3c4d5e6f1a2b3c4d5e6f1a2b3c4d5e6f1a2b'], expect.objectContaining({ cwd: '/path/to/repo' })); }); it('Should return the commit details (using git-graph.repository.useMailmap)', async () => { @@ -2928,7 +2928,7 @@ describe('DataSource', () => { onDidChangeConfiguration.emit({ affectsConfiguration: (section) => section === 'git-graph.repository.useMailmap' }); - const result = await dataSource.getCommitDetails('/path/to/repo', '1a2b3c4d5e6f1a2b3c4d5e6f1a2b3c4d5e6f1a2b', true); + const result = await dataSource.getCommitDetails('/path/to/repo', '1a2b3c4d5e6f1a2b3c4d5e6f1a2b3c4d5e6f1a2b', true, 1); // Assert expect(result).toStrictEqual({ @@ -2970,8 +2970,8 @@ describe('DataSource', () => { error: null }); expect(spyOnSpawn).toBeCalledWith('/path/to/git', ['-c', 'log.showSignature=false', 'show', '--quiet', '1a2b3c4d5e6f1a2b3c4d5e6f1a2b3c4d5e6f1a2b', '--format=%HXX7Nal-YARtTpjCikii9nJxER19D6diSyk-AWkPb%PXX7Nal-YARtTpjCikii9nJxER19D6diSyk-AWkPb%aNXX7Nal-YARtTpjCikii9nJxER19D6diSyk-AWkPb%aEXX7Nal-YARtTpjCikii9nJxER19D6diSyk-AWkPb%atXX7Nal-YARtTpjCikii9nJxER19D6diSyk-AWkPb%cNXX7Nal-YARtTpjCikii9nJxER19D6diSyk-AWkPb%cEXX7Nal-YARtTpjCikii9nJxER19D6diSyk-AWkPb%ctXX7Nal-YARtTpjCikii9nJxER19D6diSyk-AWkPbXX7Nal-YARtTpjCikii9nJxER19D6diSyk-AWkPbXX7Nal-YARtTpjCikii9nJxER19D6diSyk-AWkPbXX7Nal-YARtTpjCikii9nJxER19D6diSyk-AWkPb%B'], expect.objectContaining({ cwd: '/path/to/repo' })); - expect(spyOnSpawn).toBeCalledWith('/path/to/git', ['diff', '--name-status', '--find-renames', '--diff-filter=AMDR', '-z', '1a2b3c4d5e6f1a2b3c4d5e6f1a2b3c4d5e6f1a2b^', '1a2b3c4d5e6f1a2b3c4d5e6f1a2b3c4d5e6f1a2b'], expect.objectContaining({ cwd: '/path/to/repo' })); - expect(spyOnSpawn).toBeCalledWith('/path/to/git', ['diff', '--numstat', '--find-renames', '--diff-filter=AMDR', '-z', '1a2b3c4d5e6f1a2b3c4d5e6f1a2b3c4d5e6f1a2b^', '1a2b3c4d5e6f1a2b3c4d5e6f1a2b3c4d5e6f1a2b'], expect.objectContaining({ cwd: '/path/to/repo' })); + expect(spyOnSpawn).toBeCalledWith('/path/to/git', ['diff', '--name-status', '--find-renames', '--diff-filter=AMDR', '-z', '1a2b3c4d5e6f1a2b3c4d5e6f1a2b3c4d5e6f1a2b^1', '1a2b3c4d5e6f1a2b3c4d5e6f1a2b3c4d5e6f1a2b'], expect.objectContaining({ cwd: '/path/to/repo' })); + expect(spyOnSpawn).toBeCalledWith('/path/to/git', ['diff', '--numstat', '--find-renames', '--diff-filter=AMDR', '-z', '1a2b3c4d5e6f1a2b3c4d5e6f1a2b3c4d5e6f1a2b^1', '1a2b3c4d5e6f1a2b3c4d5e6f1a2b3c4d5e6f1a2b'], expect.objectContaining({ cwd: '/path/to/repo' })); }); it('Should return the commit details (handling unknown Git file status returned by git diff --name-status)', async () => { @@ -2981,7 +2981,7 @@ describe('DataSource', () => { mockGitSuccessOnce(['0 0 dir/deleted.txt', '1 1 dir/modified.txt', '2 3 ', 'dir/renamed-old.txt', 'dir/renamed-new.txt', ''].join('\0')); // Run - const result = await dataSource.getCommitDetails('/path/to/repo', '1a2b3c4d5e6f1a2b3c4d5e6f1a2b3c4d5e6f1a2b', true); + const result = await dataSource.getCommitDetails('/path/to/repo', '1a2b3c4d5e6f1a2b3c4d5e6f1a2b3c4d5e6f1a2b', true, 1); // Assert expect(result).toStrictEqual({ @@ -3009,8 +3009,8 @@ describe('DataSource', () => { error: null }); expect(spyOnSpawn).toBeCalledWith('/path/to/git', ['-c', 'log.showSignature=false', 'show', '--quiet', '1a2b3c4d5e6f1a2b3c4d5e6f1a2b3c4d5e6f1a2b', '--format=%HXX7Nal-YARtTpjCikii9nJxER19D6diSyk-AWkPb%PXX7Nal-YARtTpjCikii9nJxER19D6diSyk-AWkPb%anXX7Nal-YARtTpjCikii9nJxER19D6diSyk-AWkPb%aeXX7Nal-YARtTpjCikii9nJxER19D6diSyk-AWkPb%atXX7Nal-YARtTpjCikii9nJxER19D6diSyk-AWkPb%cnXX7Nal-YARtTpjCikii9nJxER19D6diSyk-AWkPb%ceXX7Nal-YARtTpjCikii9nJxER19D6diSyk-AWkPb%ctXX7Nal-YARtTpjCikii9nJxER19D6diSyk-AWkPbXX7Nal-YARtTpjCikii9nJxER19D6diSyk-AWkPbXX7Nal-YARtTpjCikii9nJxER19D6diSyk-AWkPbXX7Nal-YARtTpjCikii9nJxER19D6diSyk-AWkPb%B'], expect.objectContaining({ cwd: '/path/to/repo' })); - expect(spyOnSpawn).toBeCalledWith('/path/to/git', ['diff', '--name-status', '--find-renames', '--diff-filter=AMDR', '-z', '1a2b3c4d5e6f1a2b3c4d5e6f1a2b3c4d5e6f1a2b^', '1a2b3c4d5e6f1a2b3c4d5e6f1a2b3c4d5e6f1a2b'], expect.objectContaining({ cwd: '/path/to/repo' })); - expect(spyOnSpawn).toBeCalledWith('/path/to/git', ['diff', '--numstat', '--find-renames', '--diff-filter=AMDR', '-z', '1a2b3c4d5e6f1a2b3c4d5e6f1a2b3c4d5e6f1a2b^', '1a2b3c4d5e6f1a2b3c4d5e6f1a2b3c4d5e6f1a2b'], expect.objectContaining({ cwd: '/path/to/repo' })); + expect(spyOnSpawn).toBeCalledWith('/path/to/git', ['diff', '--name-status', '--find-renames', '--diff-filter=AMDR', '-z', '1a2b3c4d5e6f1a2b3c4d5e6f1a2b3c4d5e6f1a2b^1', '1a2b3c4d5e6f1a2b3c4d5e6f1a2b3c4d5e6f1a2b'], expect.objectContaining({ cwd: '/path/to/repo' })); + expect(spyOnSpawn).toBeCalledWith('/path/to/git', ['diff', '--numstat', '--find-renames', '--diff-filter=AMDR', '-z', '1a2b3c4d5e6f1a2b3c4d5e6f1a2b3c4d5e6f1a2b^1', '1a2b3c4d5e6f1a2b3c4d5e6f1a2b3c4d5e6f1a2b'], expect.objectContaining({ cwd: '/path/to/repo' })); }); it('Should return the commit details (handling unexpected response format returned by git diff --numstat)', async () => { @@ -3020,7 +3020,7 @@ describe('DataSource', () => { mockGitSuccessOnce(['0 0 dir/deleted.txt', '1 1', '2 3 ', 'dir/renamed-old.txt', 'dir/renamed-new.txt', ''].join('\0')); // Run - const result = await dataSource.getCommitDetails('/path/to/repo', '1a2b3c4d5e6f1a2b3c4d5e6f1a2b3c4d5e6f1a2b', true); + const result = await dataSource.getCommitDetails('/path/to/repo', '1a2b3c4d5e6f1a2b3c4d5e6f1a2b3c4d5e6f1a2b', true, 1); // Assert expect(result).toStrictEqual({ @@ -3062,8 +3062,8 @@ describe('DataSource', () => { error: null }); expect(spyOnSpawn).toBeCalledWith('/path/to/git', ['-c', 'log.showSignature=false', 'show', '--quiet', '1a2b3c4d5e6f1a2b3c4d5e6f1a2b3c4d5e6f1a2b', '--format=%HXX7Nal-YARtTpjCikii9nJxER19D6diSyk-AWkPb%PXX7Nal-YARtTpjCikii9nJxER19D6diSyk-AWkPb%anXX7Nal-YARtTpjCikii9nJxER19D6diSyk-AWkPb%aeXX7Nal-YARtTpjCikii9nJxER19D6diSyk-AWkPb%atXX7Nal-YARtTpjCikii9nJxER19D6diSyk-AWkPb%cnXX7Nal-YARtTpjCikii9nJxER19D6diSyk-AWkPb%ceXX7Nal-YARtTpjCikii9nJxER19D6diSyk-AWkPb%ctXX7Nal-YARtTpjCikii9nJxER19D6diSyk-AWkPbXX7Nal-YARtTpjCikii9nJxER19D6diSyk-AWkPbXX7Nal-YARtTpjCikii9nJxER19D6diSyk-AWkPbXX7Nal-YARtTpjCikii9nJxER19D6diSyk-AWkPb%B'], expect.objectContaining({ cwd: '/path/to/repo' })); - expect(spyOnSpawn).toBeCalledWith('/path/to/git', ['diff', '--name-status', '--find-renames', '--diff-filter=AMDR', '-z', '1a2b3c4d5e6f1a2b3c4d5e6f1a2b3c4d5e6f1a2b^', '1a2b3c4d5e6f1a2b3c4d5e6f1a2b3c4d5e6f1a2b'], expect.objectContaining({ cwd: '/path/to/repo' })); - expect(spyOnSpawn).toBeCalledWith('/path/to/git', ['diff', '--numstat', '--find-renames', '--diff-filter=AMDR', '-z', '1a2b3c4d5e6f1a2b3c4d5e6f1a2b3c4d5e6f1a2b^', '1a2b3c4d5e6f1a2b3c4d5e6f1a2b3c4d5e6f1a2b'], expect.objectContaining({ cwd: '/path/to/repo' })); + expect(spyOnSpawn).toBeCalledWith('/path/to/git', ['diff', '--name-status', '--find-renames', '--diff-filter=AMDR', '-z', '1a2b3c4d5e6f1a2b3c4d5e6f1a2b3c4d5e6f1a2b^1', '1a2b3c4d5e6f1a2b3c4d5e6f1a2b3c4d5e6f1a2b'], expect.objectContaining({ cwd: '/path/to/repo' })); + expect(spyOnSpawn).toBeCalledWith('/path/to/git', ['diff', '--numstat', '--find-renames', '--diff-filter=AMDR', '-z', '1a2b3c4d5e6f1a2b3c4d5e6f1a2b3c4d5e6f1a2b^1', '1a2b3c4d5e6f1a2b3c4d5e6f1a2b3c4d5e6f1a2b'], expect.objectContaining({ cwd: '/path/to/repo' })); }); it('Should return an error message thrown by git (when thrown by git show)', async () => { @@ -3073,7 +3073,7 @@ describe('DataSource', () => { mockGitSuccessOnce(['0 0 dir/deleted.txt', '1 1 dir/modified.txt', '2 3 ', 'dir/renamed-old.txt', 'dir/renamed-new.txt', ''].join('\0')); // Run - const result = await dataSource.getCommitDetails('/path/to/repo', '1a2b3c4d5e6f1a2b3c4d5e6f1a2b3c4d5e6f1a2b', true); + const result = await dataSource.getCommitDetails('/path/to/repo', '1a2b3c4d5e6f1a2b3c4d5e6f1a2b3c4d5e6f1a2b', true, 1); // Assert expect(result).toStrictEqual({ @@ -3089,7 +3089,7 @@ describe('DataSource', () => { mockGitSuccessOnce(['0 0 dir/deleted.txt', '1 1 dir/modified.txt', '2 3 ', 'dir/renamed-old.txt', 'dir/renamed-new.txt', ''].join('\0')); // Run - const result = await dataSource.getCommitDetails('/path/to/repo', '1a2b3c4d5e6f1a2b3c4d5e6f1a2b3c4d5e6f1a2b', true); + const result = await dataSource.getCommitDetails('/path/to/repo', '1a2b3c4d5e6f1a2b3c4d5e6f1a2b3c4d5e6f1a2b', true, 1); // Assert expect(result).toStrictEqual({ @@ -3105,7 +3105,7 @@ describe('DataSource', () => { mockGitThrowingErrorOnce(); // Run - const result = await dataSource.getCommitDetails('/path/to/repo', '1a2b3c4d5e6f1a2b3c4d5e6f1a2b3c4d5e6f1a2b', true); + const result = await dataSource.getCommitDetails('/path/to/repo', '1a2b3c4d5e6f1a2b3c4d5e6f1a2b3c4d5e6f1a2b', true, 1); // Assert expect(result).toStrictEqual({ From 893bb4c2c08d260c0aa96aeaf6fb5a31d185d902 Mon Sep 17 00:00:00 2001 From: Dan Arad Date: Sat, 20 Mar 2021 17:42:57 +0200 Subject: [PATCH 03/27] Add test for requesting 2nd parent --- tests/dataSource.test.ts | 53 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 53 insertions(+) diff --git a/tests/dataSource.test.ts b/tests/dataSource.test.ts index 12283570..424f648a 100644 --- a/tests/dataSource.test.ts +++ b/tests/dataSource.test.ts @@ -3066,6 +3066,59 @@ describe('DataSource', () => { expect(spyOnSpawn).toBeCalledWith('/path/to/git', ['diff', '--numstat', '--find-renames', '--diff-filter=AMDR', '-z', '1a2b3c4d5e6f1a2b3c4d5e6f1a2b3c4d5e6f1a2b^1', '1a2b3c4d5e6f1a2b3c4d5e6f1a2b3c4d5e6f1a2b'], expect.objectContaining({ cwd: '/path/to/repo' })); }); + it('Should return the commit details (and request diff to 2nd parent)', async () => { + // Setup + mockGitSuccessOnce('1a2b3c4d5e6f1a2b3c4d5e6f1a2b3c4d5e6f1a2bXX7Nal-YARtTpjCikii9nJxER19D6diSyk-AWkPba1b2c3d4e5f6a1b2c3d4e5f6a1b2c3d4e5f6a1b2XX7Nal-YARtTpjCikii9nJxER19D6diSyk-AWkPbTest AuthorXX7Nal-YARtTpjCikii9nJxER19D6diSyk-AWkPbtest-author@mhutchie.comXX7Nal-YARtTpjCikii9nJxER19D6diSyk-AWkPb1587559258XX7Nal-YARtTpjCikii9nJxER19D6diSyk-AWkPbTest CommitterXX7Nal-YARtTpjCikii9nJxER19D6diSyk-AWkPbtest-committer@mhutchie.comXX7Nal-YARtTpjCikii9nJxER19D6diSyk-AWkPb1587559259XX7Nal-YARtTpjCikii9nJxER19D6diSyk-AWkPbXX7Nal-YARtTpjCikii9nJxER19D6diSyk-AWkPbXX7Nal-YARtTpjCikii9nJxER19D6diSyk-AWkPbXX7Nal-YARtTpjCikii9nJxER19D6diSyk-AWkPbCommit Message.\r\nSecond Line.'); + mockGitSuccessOnce(['D', 'dir/deleted.txt', 'M', 'dir/modified.txt', 'R100', 'dir/renamed-old.txt', 'dir/renamed-new.txt', ''].join('\0')); + mockGitSuccessOnce(['0 0 dir/deleted.txt', '1 1 dir/modified.txt', '2 3 ', 'dir/renamed-old.txt', 'dir/renamed-new.txt', ''].join('\0')); + + // Run + const result = await dataSource.getCommitDetails('/path/to/repo', '1a2b3c4d5e6f1a2b3c4d5e6f1a2b3c4d5e6f1a2b', true, 2); + + // Assert + expect(result).toStrictEqual({ + commitDetails: { + hash: '1a2b3c4d5e6f1a2b3c4d5e6f1a2b3c4d5e6f1a2b', + parents: ['a1b2c3d4e5f6a1b2c3d4e5f6a1b2c3d4e5f6a1b2'], + author: 'Test Author', + authorEmail: 'test-author@mhutchie.com', + authorDate: 1587559258, + committer: 'Test Committer', + committerEmail: 'test-committer@mhutchie.com', + committerDate: 1587559259, + signature: null, + body: 'Commit Message.\nSecond Line.', + fileChanges: [ + { + additions: 0, + deletions: 0, + newFilePath: 'dir/deleted.txt', + oldFilePath: 'dir/deleted.txt', + type: 'D' + }, + { + additions: 1, + deletions: 1, + newFilePath: 'dir/modified.txt', + oldFilePath: 'dir/modified.txt', + type: 'M' + }, + { + additions: 2, + deletions: 3, + newFilePath: 'dir/renamed-new.txt', + oldFilePath: 'dir/renamed-old.txt', + type: 'R' + } + ] + }, + error: null + }); + expect(spyOnSpawn).toBeCalledWith('/path/to/git', ['-c', 'log.showSignature=false', 'show', '--quiet', '1a2b3c4d5e6f1a2b3c4d5e6f1a2b3c4d5e6f1a2b', '--format=%HXX7Nal-YARtTpjCikii9nJxER19D6diSyk-AWkPb%PXX7Nal-YARtTpjCikii9nJxER19D6diSyk-AWkPb%anXX7Nal-YARtTpjCikii9nJxER19D6diSyk-AWkPb%aeXX7Nal-YARtTpjCikii9nJxER19D6diSyk-AWkPb%atXX7Nal-YARtTpjCikii9nJxER19D6diSyk-AWkPb%cnXX7Nal-YARtTpjCikii9nJxER19D6diSyk-AWkPb%ceXX7Nal-YARtTpjCikii9nJxER19D6diSyk-AWkPb%ctXX7Nal-YARtTpjCikii9nJxER19D6diSyk-AWkPbXX7Nal-YARtTpjCikii9nJxER19D6diSyk-AWkPbXX7Nal-YARtTpjCikii9nJxER19D6diSyk-AWkPbXX7Nal-YARtTpjCikii9nJxER19D6diSyk-AWkPb%B'], expect.objectContaining({ cwd: '/path/to/repo' })); + expect(spyOnSpawn).toBeCalledWith('/path/to/git', ['diff', '--name-status', '--find-renames', '--diff-filter=AMDR', '-z', '1a2b3c4d5e6f1a2b3c4d5e6f1a2b3c4d5e6f1a2b^2', '1a2b3c4d5e6f1a2b3c4d5e6f1a2b3c4d5e6f1a2b'], expect.objectContaining({ cwd: '/path/to/repo' })); + expect(spyOnSpawn).toBeCalledWith('/path/to/git', ['diff', '--numstat', '--find-renames', '--diff-filter=AMDR', '-z', '1a2b3c4d5e6f1a2b3c4d5e6f1a2b3c4d5e6f1a2b^2', '1a2b3c4d5e6f1a2b3c4d5e6f1a2b3c4d5e6f1a2b'], expect.objectContaining({ cwd: '/path/to/repo' })); + }); + it('Should return an error message thrown by git (when thrown by git show)', async () => { // Setup mockGitThrowingErrorOnce(); From 07bee7abfefacd3d95abd4b828647221bad7b521 Mon Sep 17 00:00:00 2001 From: Dan Arad Date: Mon, 5 Apr 2021 13:49:04 +0300 Subject: [PATCH 04/27] Show parents on a single line --- web/main.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/web/main.ts b/web/main.ts index e092b90c..6f594d7f 100644 --- a/web/main.ts +++ b/web/main.ts @@ -2505,13 +2505,13 @@ class GitGraphView { const escapedParent = escapeHtml(parent); const isComparedToParent = parentIndex + 1 === expandedCommit.parentIndex; return typeof this.commitLookup[parent] === 'number' - ? `  ${isComparedToParent ? '' : ''}#${parentIndex + 1} ${abbrevCommit(escapedParent)}${isComparedToParent ? '' : ''}` + ? `  ${isComparedToParent ? '' : ''}${abbrevCommit(escapedParent)}${isComparedToParent ? '' : ''}` : escapedParent; - }).join('
') + }).join(', ') : 'None'; html += '' + 'Commit: ' + escapeHtml(commitDetails.hash) + '
' - + 'Parents:
' + parents + '
' + + 'Parents: ' + parents + '
' + 'Author: ' + escapeHtml(commitDetails.author) + (commitDetails.authorEmail !== '' ? ' <' + escapeHtml(commitDetails.authorEmail) + '>' : '') + '
' + (commitDetails.authorDate !== commitDetails.committerDate ? 'Author Date: ' + formatLongDate(commitDetails.authorDate) + '
' : '') + 'Committer: ' + escapeHtml(commitDetails.committer) + (commitDetails.committerEmail !== '' ? ' <' + escapeHtml(commitDetails.committerEmail) + '>' : '') + (commitDetails.signature !== null ? generateSignatureHtml(commitDetails.signature) : '') + '
' From 724c13468efefe7d86f690c0f566862ab7f77559 Mon Sep 17 00:00:00 2001 From: Dan Arad Date: Mon, 5 Apr 2021 15:54:33 +0300 Subject: [PATCH 05/27] Add context menu option for switching parents. A new class was added to each parent hash element in the form `parent-`, where id is the index of the parent. Then, in the context menu, we check if the class exists and if it does we add an action to allow viewing diff with the parent. --- web/main.ts | 23 ++++++++++++++++++++++- web/utils.ts | 1 + 2 files changed, 23 insertions(+), 1 deletion(-) diff --git a/web/main.ts b/web/main.ts index 6f594d7f..0d525eb5 100644 --- a/web/main.ts +++ b/web/main.ts @@ -2077,6 +2077,18 @@ class GitGraphView { } }; + const getParentIndexFromClass = (element: Element) => { + const eventTargetClasses = element.classList.value.split(' '); + const parentIndexClasses = eventTargetClasses.filter(className => className.startsWith(`${CLASS_PARENT}-`)); + + const isParentElement = parentIndexClasses.length === 1; + if(!isParentElement) { + return -1; + } + + return parseInt(parentIndexClasses[0].substring(`${CLASS_PARENT}-`.length)); + } + document.body.addEventListener('click', followInternalLink); document.body.addEventListener('contextmenu', (e: MouseEvent) => { @@ -2117,6 +2129,8 @@ class GitGraphView { isInDialog = true; } + const parentIndex = getParentIndexFromClass(eventTarget); + handledEvent(e); contextMenu.show([ [ @@ -2132,6 +2146,13 @@ class GitGraphView { visible: isInternalUrl, onClick: () => followInternalLink(e) }, + { + title: "View Changes with this Parent", + visible: isInternalUrl && parentIndex !== -1, + onClick: () => { + this.loadCommitDetails(this.expandedCommit!.commitElem!, parentIndex); + } + }, { title: 'Copy URL to Clipboard', visible: isExternalUrl, @@ -2505,7 +2526,7 @@ class GitGraphView { const escapedParent = escapeHtml(parent); const isComparedToParent = parentIndex + 1 === expandedCommit.parentIndex; return typeof this.commitLookup[parent] === 'number' - ? `  ${isComparedToParent ? '' : ''}${abbrevCommit(escapedParent)}${isComparedToParent ? '' : ''}` + ? `  ${isComparedToParent ? '' : ''}${abbrevCommit(escapedParent)}${isComparedToParent ? '' : ''}` : escapedParent; }).join(', ') : 'None'; diff --git a/web/utils.ts b/web/utils.ts index b4066463..2c8d8dbb 100644 --- a/web/utils.ts +++ b/web/utils.ts @@ -89,6 +89,7 @@ const CLASS_REF_TAG = 'tag'; const CLASS_SELECTED = 'selected'; const CLASS_TAG_LABELS_RIGHT_ALIGNED = 'tagLabelsRightAligned'; const CLASS_TRANSITION = 'transition'; +const CLASS_PARENT = 'parent'; const ID_EVENT_CAPTURE_ELEM = 'eventCaptureElem'; From 85a29d6f9a3c2c3a29533b42e22baff1ed37a2f3 Mon Sep 17 00:00:00 2001 From: Dan Arad Date: Mon, 5 Apr 2021 16:51:23 +0300 Subject: [PATCH 06/27] Refactor parent commit rendering for clarity --- web/main.ts | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-) diff --git a/web/main.ts b/web/main.ts index 0d525eb5..2bbcb4da 100644 --- a/web/main.ts +++ b/web/main.ts @@ -2524,10 +2524,21 @@ class GitGraphView { const parents = commitDetails.parents.length > 0 ? commitDetails.parents.map((parent, parentIndex) => { const escapedParent = escapeHtml(parent); + + const parentWasLoaded = typeof this.commitLookup[parent] === 'number'; const isComparedToParent = parentIndex + 1 === expandedCommit.parentIndex; - return typeof this.commitLookup[parent] === 'number' - ? `  ${isComparedToParent ? '' : ''}${abbrevCommit(escapedParent)}${isComparedToParent ? '' : ''}` - : escapedParent; + + let parentHtml = abbrevCommit(escapedParent); + if(parentWasLoaded) { + parentHtml = ` + ${parentHtml} + `; + } + if(isComparedToParent) { + parentHtml = `${parentHtml}`; + } + + return parentHtml; }).join(', ') : 'None'; html += '' From 5a2afcc28183942bac3aec0fd990533471bb2fdd Mon Sep 17 00:00:00 2001 From: Dan Arad Date: Mon, 5 Apr 2021 22:29:32 +0300 Subject: [PATCH 07/27] Add an icon to toggle parents --- web/main.ts | 8 ++++++++ web/utils.ts | 1 + 2 files changed, 9 insertions(+) diff --git a/web/main.ts b/web/main.ts index 2bbcb4da..73833d37 100644 --- a/web/main.ts +++ b/web/main.ts @@ -2564,6 +2564,7 @@ class GitGraphView { (codeReviewPossible ? '
' + SVG_ICONS.review + '
' : '') + (!expandedCommit.loading ? '
' + SVG_ICONS.fileTree + '
' + SVG_ICONS.fileList + '
' : '') + (externalDiffPossible ? '
' + SVG_ICONS.linkExternal + '
' : '') + + (expandedCommit.commitDetails && expandedCommit.commitDetails.parents.length > 1 ? '
' + SVG_ICONS.merge + '
' : '') + '
'; elem.innerHTML = isDocked ? html : '
' + html + ''; @@ -2635,6 +2636,13 @@ class GitGraphView { this.changeFileViewType(GG.FileViewType.List); }); + document.getElementById('cdvParentToggle')?.addEventListener('click', () => { + const parentIndex = this.expandedCommit!.parentIndex; + const parentNumber = this.expandedCommit!.commitDetails!.parents.length; + const nextParentIndex = parentIndex < parentNumber ? parentIndex + 1 : 1; + this.loadCommitDetails(this.expandedCommit!.commitElem!, nextParentIndex); + }); + if (codeReviewPossible) { this.renderCodeReviewBtn(); document.getElementById('cdvCodeReview')!.addEventListener('click', (e) => { diff --git a/web/utils.ts b/web/utils.ts index 2c8d8dbb..5f90a889 100644 --- a/web/utils.ts +++ b/web/utils.ts @@ -27,6 +27,7 @@ const SVG_ICONS = { openFolder: '', closedFolder: '', file: '', + merge: '', // The SVG icons below are custom made arrowDown: '', From 3a6bc5a0b017be6272351dcb2ca5dd3ca6455070 Mon Sep 17 00:00:00 2001 From: Dan Arad Date: Tue, 6 Apr 2021 12:38:53 +0300 Subject: [PATCH 08/27] Improve choice of next parent when toggling Added the `loadedParents` variable which is used to deduce how to render each parent and to make sure that toggling is done only between loaded parents. --- web/main.ts | 22 ++++++++++++++++------ 1 file changed, 16 insertions(+), 6 deletions(-) diff --git a/web/main.ts b/web/main.ts index 73833d37..6f342001 100644 --- a/web/main.ts +++ b/web/main.ts @@ -2492,6 +2492,7 @@ class GitGraphView { const commitOrder = this.getCommitOrder(expandedCommit.commitHash, expandedCommit.compareWithHash === null ? expandedCommit.commitHash : expandedCommit.compareWithHash); const codeReviewPossible = !expandedCommit.loading && commitOrder.to !== UNCOMMITTED; const externalDiffPossible = !expandedCommit.loading && (expandedCommit.compareWithHash !== null || this.commits[this.commitLookup[expandedCommit.commitHash]].parents.length > 0); + const loadedParents = expandedCommit.commitDetails?.parents.filter(parent => this.commitLookup[parent]) || []; if (elem === null) { elem = document.createElement(isDocked ? 'div' : 'tr'); @@ -2525,7 +2526,7 @@ class GitGraphView { ? commitDetails.parents.map((parent, parentIndex) => { const escapedParent = escapeHtml(parent); - const parentWasLoaded = typeof this.commitLookup[parent] === 'number'; + const parentWasLoaded = loadedParents.includes(parent); const isComparedToParent = parentIndex + 1 === expandedCommit.parentIndex; let parentHtml = abbrevCommit(escapedParent); @@ -2564,7 +2565,7 @@ class GitGraphView { (codeReviewPossible ? '
' + SVG_ICONS.review + '
' : '') + (!expandedCommit.loading ? '
' + SVG_ICONS.fileTree + '
' + SVG_ICONS.fileList + '
' : '') + (externalDiffPossible ? '
' + SVG_ICONS.linkExternal + '
' : '') + - (expandedCommit.commitDetails && expandedCommit.commitDetails.parents.length > 1 ? '
' + SVG_ICONS.merge + '
' : '') + + (loadedParents.length > 1 ? '
' + SVG_ICONS.merge + '
' : '') + '
'; elem.innerHTML = isDocked ? html : '
' + html + ''; @@ -2637,10 +2638,19 @@ class GitGraphView { }); document.getElementById('cdvParentToggle')?.addEventListener('click', () => { - const parentIndex = this.expandedCommit!.parentIndex; - const parentNumber = this.expandedCommit!.commitDetails!.parents.length; - const nextParentIndex = parentIndex < parentNumber ? parentIndex + 1 : 1; - this.loadCommitDetails(this.expandedCommit!.commitElem!, nextParentIndex); + const expandedCommit = this.expandedCommit!; + const parentIndex = expandedCommit.parentIndex; + const parents = expandedCommit.commitDetails!.parents; + + let nextParentIndex = parentIndex < parents.length ? parentIndex + 1 : 1; + while(!loadedParents.includes(parents[nextParentIndex - 1])) { + if(nextParentIndex === parentIndex) { + break; + } + nextParentIndex = nextParentIndex < parents.length ? parentIndex + 1 : 1; + } + + this.loadCommitDetails(expandedCommit.commitElem!, nextParentIndex); }); if (codeReviewPossible) { From 00b53d3abe0d25bcd0abceccf6cb2b9d885223d3 Mon Sep 17 00:00:00 2001 From: Dan Arad Date: Tue, 6 Apr 2021 12:56:31 +0300 Subject: [PATCH 09/27] Add support to toggling diffs between stash parents --- src/dataSource.ts | 9 ++++++--- src/gitGraphView.ts | 2 +- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/src/dataSource.ts b/src/dataSource.ts index 6640e25c..cadc6110 100644 --- a/src/dataSource.ts +++ b/src/dataSource.ts @@ -339,6 +339,7 @@ export class DataSource extends Disposable { * @param repo The path of the repository. * @param commitHash The hash of the commit open in the Commit Details View. * @param hasParents Does the commit have parents + * @param parentIndex The index of the commit parent * @returns The commit details. */ public getCommitDetails(repo: string, commitHash: string, hasParents: boolean, parentIndex: number): Promise { @@ -360,13 +361,15 @@ export class DataSource extends Disposable { * @param repo The path of the repository. * @param commitHash The hash of the stash commit open in the Commit Details View. * @param stash The stash. + * @param parentIndex the index of the stash parent * @returns The stash details. */ - public getStashDetails(repo: string, commitHash: string, stash: GitCommitStash): Promise { + public getStashDetails(repo: string, commitHash: string, stash: GitCommitStash, parentIndex: number): Promise { + const fromCommit = commitHash + `^${parentIndex}`; return Promise.all([ this.getCommitDetailsBase(repo, commitHash), - this.getDiffNameStatus(repo, stash.baseHash, commitHash), - this.getDiffNumStat(repo, stash.baseHash, commitHash), + this.getDiffNameStatus(repo, fromCommit, commitHash), + this.getDiffNumStat(repo, fromCommit, commitHash), stash.untrackedFilesHash !== null ? this.getDiffNameStatus(repo, stash.untrackedFilesHash, stash.untrackedFilesHash) : Promise.resolve([]), stash.untrackedFilesHash !== null ? this.getDiffNumStat(repo, stash.untrackedFilesHash, stash.untrackedFilesHash) : Promise.resolve([]) ]).then((results) => { diff --git a/src/gitGraphView.ts b/src/gitGraphView.ts index a9c5043e..b00ffdfb 100644 --- a/src/gitGraphView.ts +++ b/src/gitGraphView.ts @@ -233,7 +233,7 @@ export class GitGraphView extends Disposable { ? this.dataSource.getUncommittedDetails(msg.repo) : msg.stash === null ? this.dataSource.getCommitDetails(msg.repo, msg.commitHash, msg.hasParents, msg.parentIndex) - : this.dataSource.getStashDetails(msg.repo, msg.commitHash, msg.stash), + : this.dataSource.getStashDetails(msg.repo, msg.commitHash, msg.stash, msg.parentIndex), msg.avatarEmail !== null ? this.avatarManager.getAvatarImage(msg.avatarEmail) : Promise.resolve(null) ]); this.sendMessage({ From d39f5c4d589398792ca9129a2220babd0444cb56 Mon Sep 17 00:00:00 2001 From: Dan Arad Date: Tue, 6 Apr 2021 14:16:22 +0300 Subject: [PATCH 10/27] Fix bugs where parentIndex is not reset properly when a commit is clicked on. --- web/main.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/web/main.ts b/web/main.ts index 6f342001..45a53895 100644 --- a/web/main.ts +++ b/web/main.ts @@ -2014,7 +2014,7 @@ class GitGraphView { if (newHashIndex > -1) { handledEvent(e); const elem = findCommitElemWithId(getCommitElems(), newHashIndex); - if (elem !== null) this.loadCommitDetails(elem, this.expandedCommit.parentIndex); + if (elem !== null) this.loadCommitDetails(elem, DEFAULT_PARENT_INDEX); } } else if (e.key && (e.ctrlKey || e.metaKey)) { const key = e.key.toLowerCase(), keybindings = this.config.keybindings; @@ -2201,7 +2201,7 @@ class GitGraphView { this.loadCommitComparison(this.expandedCommit.commitElem, eventElem); } } else { - this.loadCommitDetails(eventElem, this.expandedCommit.parentIndex); + this.loadCommitDetails(eventElem, DEFAULT_PARENT_INDEX); } } else { this.loadCommitDetails(eventElem, DEFAULT_PARENT_INDEX); From a77c2ec25e25686b93f8e060eea93b306512caad Mon Sep 17 00:00:00 2001 From: Dan Arad Date: Tue, 6 Apr 2021 15:31:58 +0300 Subject: [PATCH 11/27] Fix lint errors --- web/main.ts | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/web/main.ts b/web/main.ts index 45a53895..5be0dcfe 100644 --- a/web/main.ts +++ b/web/main.ts @@ -2078,16 +2078,16 @@ class GitGraphView { }; const getParentIndexFromClass = (element: Element) => { - const eventTargetClasses = element.classList.value.split(' '); - const parentIndexClasses = eventTargetClasses.filter(className => className.startsWith(`${CLASS_PARENT}-`)); + const eventTargetClasses = element.classList.value.split(' '); + const parentIndexClasses = eventTargetClasses.filter(className => className.startsWith(`${CLASS_PARENT}-`)); - const isParentElement = parentIndexClasses.length === 1; - if(!isParentElement) { - return -1; - } + const isParentElement = parentIndexClasses.length === 1; + if(!isParentElement) { + return -1; + } - return parseInt(parentIndexClasses[0].substring(`${CLASS_PARENT}-`.length)); - } + return parseInt(parentIndexClasses[0].substring(`${CLASS_PARENT}-`.length)); + }; document.body.addEventListener('click', followInternalLink); @@ -2147,7 +2147,7 @@ class GitGraphView { onClick: () => followInternalLink(e) }, { - title: "View Changes with this Parent", + title: 'View Changes with this Parent', visible: isInternalUrl && parentIndex !== -1, onClick: () => { this.loadCommitDetails(this.expandedCommit!.commitElem!, parentIndex); From 439ca17bb50d3d900b0a9638b08d2f89b04211dc Mon Sep 17 00:00:00 2001 From: Dan Arad Date: Tue, 6 Apr 2021 15:32:21 +0300 Subject: [PATCH 12/27] Add parentIndex to existing tests; Add a test for second parent. --- tests/dataSource.test.ts | 79 ++++++++++++++++++++++++++++++++++------ 1 file changed, 68 insertions(+), 11 deletions(-) diff --git a/tests/dataSource.test.ts b/tests/dataSource.test.ts index 424f648a..40dd1681 100644 --- a/tests/dataSource.test.ts +++ b/tests/dataSource.test.ts @@ -3180,7 +3180,7 @@ describe('DataSource', () => { selector: 'refs/stash@{0}', baseHash: 'a1b2c3d4e5f6a1b2c3d4e5f6a1b2c3d4e5f6a1b2', untrackedFilesHash: null - }); + }, 1); // Assert expect(result).toStrictEqual({ @@ -3222,8 +3222,8 @@ describe('DataSource', () => { error: null }); expect(spyOnSpawn).toBeCalledWith('/path/to/git', ['-c', 'log.showSignature=false', 'show', '--quiet', '1a2b3c4d5e6f1a2b3c4d5e6f1a2b3c4d5e6f1a2b', '--format=%HXX7Nal-YARtTpjCikii9nJxER19D6diSyk-AWkPb%PXX7Nal-YARtTpjCikii9nJxER19D6diSyk-AWkPb%anXX7Nal-YARtTpjCikii9nJxER19D6diSyk-AWkPb%aeXX7Nal-YARtTpjCikii9nJxER19D6diSyk-AWkPb%atXX7Nal-YARtTpjCikii9nJxER19D6diSyk-AWkPb%cnXX7Nal-YARtTpjCikii9nJxER19D6diSyk-AWkPb%ceXX7Nal-YARtTpjCikii9nJxER19D6diSyk-AWkPb%ctXX7Nal-YARtTpjCikii9nJxER19D6diSyk-AWkPbXX7Nal-YARtTpjCikii9nJxER19D6diSyk-AWkPbXX7Nal-YARtTpjCikii9nJxER19D6diSyk-AWkPbXX7Nal-YARtTpjCikii9nJxER19D6diSyk-AWkPb%B'], expect.objectContaining({ cwd: '/path/to/repo' })); - expect(spyOnSpawn).toBeCalledWith('/path/to/git', ['diff', '--name-status', '--find-renames', '--diff-filter=AMDR', '-z', 'a1b2c3d4e5f6a1b2c3d4e5f6a1b2c3d4e5f6a1b2', '1a2b3c4d5e6f1a2b3c4d5e6f1a2b3c4d5e6f1a2b'], expect.objectContaining({ cwd: '/path/to/repo' })); - expect(spyOnSpawn).toBeCalledWith('/path/to/git', ['diff', '--numstat', '--find-renames', '--diff-filter=AMDR', '-z', 'a1b2c3d4e5f6a1b2c3d4e5f6a1b2c3d4e5f6a1b2', '1a2b3c4d5e6f1a2b3c4d5e6f1a2b3c4d5e6f1a2b'], expect.objectContaining({ cwd: '/path/to/repo' })); + expect(spyOnSpawn).toBeCalledWith('/path/to/git', ['diff', '--name-status', '--find-renames', '--diff-filter=AMDR', '-z', '1a2b3c4d5e6f1a2b3c4d5e6f1a2b3c4d5e6f1a2b^1', '1a2b3c4d5e6f1a2b3c4d5e6f1a2b3c4d5e6f1a2b'], expect.objectContaining({ cwd: '/path/to/repo' })); + expect(spyOnSpawn).toBeCalledWith('/path/to/git', ['diff', '--numstat', '--find-renames', '--diff-filter=AMDR', '-z', '1a2b3c4d5e6f1a2b3c4d5e6f1a2b3c4d5e6f1a2b^1', '1a2b3c4d5e6f1a2b3c4d5e6f1a2b3c4d5e6f1a2b'], expect.objectContaining({ cwd: '/path/to/repo' })); }); it('Should return the stash details (including untracked files)', async () => { @@ -3239,7 +3239,7 @@ describe('DataSource', () => { selector: 'refs/stash@{0}', baseHash: 'a1b2c3d4e5f6a1b2c3d4e5f6a1b2c3d4e5f6a1b2', untrackedFilesHash: 'c3d4e5f6a1b2c3d4e5f6a1b2c3d4e5f6a1b2c3d4' - }); + }, 1); // Assert expect(result).toStrictEqual({ @@ -3288,12 +3288,69 @@ describe('DataSource', () => { error: null }); expect(spyOnSpawn).toBeCalledWith('/path/to/git', ['-c', 'log.showSignature=false', 'show', '--quiet', '1a2b3c4d5e6f1a2b3c4d5e6f1a2b3c4d5e6f1a2b', '--format=%HXX7Nal-YARtTpjCikii9nJxER19D6diSyk-AWkPb%PXX7Nal-YARtTpjCikii9nJxER19D6diSyk-AWkPb%anXX7Nal-YARtTpjCikii9nJxER19D6diSyk-AWkPb%aeXX7Nal-YARtTpjCikii9nJxER19D6diSyk-AWkPb%atXX7Nal-YARtTpjCikii9nJxER19D6diSyk-AWkPb%cnXX7Nal-YARtTpjCikii9nJxER19D6diSyk-AWkPb%ceXX7Nal-YARtTpjCikii9nJxER19D6diSyk-AWkPb%ctXX7Nal-YARtTpjCikii9nJxER19D6diSyk-AWkPbXX7Nal-YARtTpjCikii9nJxER19D6diSyk-AWkPbXX7Nal-YARtTpjCikii9nJxER19D6diSyk-AWkPbXX7Nal-YARtTpjCikii9nJxER19D6diSyk-AWkPb%B'], expect.objectContaining({ cwd: '/path/to/repo' })); - expect(spyOnSpawn).toBeCalledWith('/path/to/git', ['diff', '--name-status', '--find-renames', '--diff-filter=AMDR', '-z', 'a1b2c3d4e5f6a1b2c3d4e5f6a1b2c3d4e5f6a1b2', '1a2b3c4d5e6f1a2b3c4d5e6f1a2b3c4d5e6f1a2b'], expect.objectContaining({ cwd: '/path/to/repo' })); - expect(spyOnSpawn).toBeCalledWith('/path/to/git', ['diff', '--numstat', '--find-renames', '--diff-filter=AMDR', '-z', 'a1b2c3d4e5f6a1b2c3d4e5f6a1b2c3d4e5f6a1b2', '1a2b3c4d5e6f1a2b3c4d5e6f1a2b3c4d5e6f1a2b'], expect.objectContaining({ cwd: '/path/to/repo' })); + expect(spyOnSpawn).toBeCalledWith('/path/to/git', ['diff', '--name-status', '--find-renames', '--diff-filter=AMDR', '-z', '1a2b3c4d5e6f1a2b3c4d5e6f1a2b3c4d5e6f1a2b^1', '1a2b3c4d5e6f1a2b3c4d5e6f1a2b3c4d5e6f1a2b'], expect.objectContaining({ cwd: '/path/to/repo' })); + expect(spyOnSpawn).toBeCalledWith('/path/to/git', ['diff', '--numstat', '--find-renames', '--diff-filter=AMDR', '-z', '1a2b3c4d5e6f1a2b3c4d5e6f1a2b3c4d5e6f1a2b^1', '1a2b3c4d5e6f1a2b3c4d5e6f1a2b3c4d5e6f1a2b'], expect.objectContaining({ cwd: '/path/to/repo' })); expect(spyOnSpawn).toBeCalledWith('/path/to/git', ['diff-tree', '--name-status', '-r', '--root', '--find-renames', '--diff-filter=AMDR', '-z', 'c3d4e5f6a1b2c3d4e5f6a1b2c3d4e5f6a1b2c3d4'], expect.objectContaining({ cwd: '/path/to/repo' })); expect(spyOnSpawn).toBeCalledWith('/path/to/git', ['diff-tree', '--numstat', '-r', '--root', '--find-renames', '--diff-filter=AMDR', '-z', 'c3d4e5f6a1b2c3d4e5f6a1b2c3d4e5f6a1b2c3d4'], expect.objectContaining({ cwd: '/path/to/repo' })); }); + it('Should return the stash details with diff to second parent', async () => { + // Setup + mockGitSuccessOnce('1a2b3c4d5e6f1a2b3c4d5e6f1a2b3c4d5e6f1a2bXX7Nal-YARtTpjCikii9nJxER19D6diSyk-AWkPba1b2c3d4e5f6a1b2c3d4e5f6a1b2c3d4e5f6a1b2 b2c3d4e5f6a1b2c3d4e5f6a1b2c3d4e5f6a1b2c3XX7Nal-YARtTpjCikii9nJxER19D6diSyk-AWkPbTest AuthorXX7Nal-YARtTpjCikii9nJxER19D6diSyk-AWkPbtest-author@mhutchie.comXX7Nal-YARtTpjCikii9nJxER19D6diSyk-AWkPb1587559258XX7Nal-YARtTpjCikii9nJxER19D6diSyk-AWkPbTest CommitterXX7Nal-YARtTpjCikii9nJxER19D6diSyk-AWkPbtest-committer@mhutchie.comXX7Nal-YARtTpjCikii9nJxER19D6diSyk-AWkPb1587559259XX7Nal-YARtTpjCikii9nJxER19D6diSyk-AWkPbXX7Nal-YARtTpjCikii9nJxER19D6diSyk-AWkPbXX7Nal-YARtTpjCikii9nJxER19D6diSyk-AWkPbXX7Nal-YARtTpjCikii9nJxER19D6diSyk-AWkPbCommit Message.\r\nSecond Line.'); + mockGitSuccessOnce(['D', 'dir/deleted.txt', 'M', 'dir/modified.txt', 'R100', 'dir/renamed-old.txt', 'dir/renamed-new.txt', ''].join('\0')); + mockGitSuccessOnce(['0 0 dir/deleted.txt', '1 1 dir/modified.txt', '2 3 ', 'dir/renamed-old.txt', 'dir/renamed-new.txt', ''].join('\0')); + + // Run + const result = await dataSource.getStashDetails('/path/to/repo', '1a2b3c4d5e6f1a2b3c4d5e6f1a2b3c4d5e6f1a2b', { + selector: 'refs/stash@{0}', + baseHash: 'a1b2c3d4e5f6a1b2c3d4e5f6a1b2c3d4e5f6a1b2', + untrackedFilesHash: null + }, 2); + + // Assert + expect(result).toStrictEqual({ + commitDetails: { + hash: '1a2b3c4d5e6f1a2b3c4d5e6f1a2b3c4d5e6f1a2b', + parents: ['a1b2c3d4e5f6a1b2c3d4e5f6a1b2c3d4e5f6a1b2', 'b2c3d4e5f6a1b2c3d4e5f6a1b2c3d4e5f6a1b2c3'], + author: 'Test Author', + authorEmail: 'test-author@mhutchie.com', + authorDate: 1587559258, + committer: 'Test Committer', + committerEmail: 'test-committer@mhutchie.com', + committerDate: 1587559259, + signature: null, + body: 'Commit Message.\nSecond Line.', + fileChanges: [ + { + additions: 0, + deletions: 0, + newFilePath: 'dir/deleted.txt', + oldFilePath: 'dir/deleted.txt', + type: 'D' + }, + { + additions: 1, + deletions: 1, + newFilePath: 'dir/modified.txt', + oldFilePath: 'dir/modified.txt', + type: 'M' + }, + { + additions: 2, + deletions: 3, + newFilePath: 'dir/renamed-new.txt', + oldFilePath: 'dir/renamed-old.txt', + type: 'R' + } + ] + }, + error: null + }); + expect(spyOnSpawn).toBeCalledWith('/path/to/git', ['-c', 'log.showSignature=false', 'show', '--quiet', '1a2b3c4d5e6f1a2b3c4d5e6f1a2b3c4d5e6f1a2b', '--format=%HXX7Nal-YARtTpjCikii9nJxER19D6diSyk-AWkPb%PXX7Nal-YARtTpjCikii9nJxER19D6diSyk-AWkPb%anXX7Nal-YARtTpjCikii9nJxER19D6diSyk-AWkPb%aeXX7Nal-YARtTpjCikii9nJxER19D6diSyk-AWkPb%atXX7Nal-YARtTpjCikii9nJxER19D6diSyk-AWkPb%cnXX7Nal-YARtTpjCikii9nJxER19D6diSyk-AWkPb%ceXX7Nal-YARtTpjCikii9nJxER19D6diSyk-AWkPb%ctXX7Nal-YARtTpjCikii9nJxER19D6diSyk-AWkPbXX7Nal-YARtTpjCikii9nJxER19D6diSyk-AWkPbXX7Nal-YARtTpjCikii9nJxER19D6diSyk-AWkPbXX7Nal-YARtTpjCikii9nJxER19D6diSyk-AWkPb%B'], expect.objectContaining({ cwd: '/path/to/repo' })); + expect(spyOnSpawn).toBeCalledWith('/path/to/git', ['diff', '--name-status', '--find-renames', '--diff-filter=AMDR', '-z', '1a2b3c4d5e6f1a2b3c4d5e6f1a2b3c4d5e6f1a2b^2', '1a2b3c4d5e6f1a2b3c4d5e6f1a2b3c4d5e6f1a2b'], expect.objectContaining({ cwd: '/path/to/repo' })); + expect(spyOnSpawn).toBeCalledWith('/path/to/git', ['diff', '--numstat', '--find-renames', '--diff-filter=AMDR', '-z', '1a2b3c4d5e6f1a2b3c4d5e6f1a2b3c4d5e6f1a2b^2', '1a2b3c4d5e6f1a2b3c4d5e6f1a2b3c4d5e6f1a2b'], expect.objectContaining({ cwd: '/path/to/repo' })); + }); + it('Should return an error message thrown by git (when thrown by git show)', async () => { // Setup mockGitThrowingErrorOnce(); @@ -3305,7 +3362,7 @@ describe('DataSource', () => { selector: 'refs/stash@{0}', baseHash: 'a1b2c3d4e5f6a1b2c3d4e5f6a1b2c3d4e5f6a1b2', untrackedFilesHash: null - }); + }, 1); // Assert expect(result).toStrictEqual({ @@ -3325,7 +3382,7 @@ describe('DataSource', () => { selector: 'refs/stash@{0}', baseHash: 'a1b2c3d4e5f6a1b2c3d4e5f6a1b2c3d4e5f6a1b2', untrackedFilesHash: null - }); + }, 1); // Assert expect(result).toStrictEqual({ @@ -3345,7 +3402,7 @@ describe('DataSource', () => { selector: 'refs/stash@{0}', baseHash: 'a1b2c3d4e5f6a1b2c3d4e5f6a1b2c3d4e5f6a1b2', untrackedFilesHash: null - }); + }, 1); // Assert expect(result).toStrictEqual({ @@ -3367,7 +3424,7 @@ describe('DataSource', () => { selector: 'refs/stash@{0}', baseHash: 'a1b2c3d4e5f6a1b2c3d4e5f6a1b2c3d4e5f6a1b2', untrackedFilesHash: 'c3d4e5f6a1b2c3d4e5f6a1b2c3d4e5f6a1b2c3d4' - }); + }, 1); // Assert expect(result).toStrictEqual({ @@ -3389,7 +3446,7 @@ describe('DataSource', () => { selector: 'refs/stash@{0}', baseHash: 'a1b2c3d4e5f6a1b2c3d4e5f6a1b2c3d4e5f6a1b2', untrackedFilesHash: 'c3d4e5f6a1b2c3d4e5f6a1b2c3d4e5f6a1b2c3d4' - }); + }, 1); // Assert expect(result).toStrictEqual({ From cb7fec6e827cbf032f304b64978d0ed0d61dae2c Mon Sep 17 00:00:00 2001 From: Dan Arad Date: Wed, 21 Apr 2021 14:36:00 +0300 Subject: [PATCH 13/27] Abbreviate before escaping and remove generated whitespace. --- web/main.ts | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/web/main.ts b/web/main.ts index 87d1d567..6f9d3dc0 100644 --- a/web/main.ts +++ b/web/main.ts @@ -2558,16 +2558,14 @@ class GitGraphView { const commitDetails = expandedCommit.commitDetails!; const parents = commitDetails.parents.length > 0 ? commitDetails.parents.map((parent, parentIndex) => { - const escapedParent = escapeHtml(parent); + const escapedParent = escapeHtml(abbrevCommit(parent)); const parentWasLoaded = loadedParents.includes(parent); const isComparedToParent = parentIndex + 1 === expandedCommit.parentIndex; - let parentHtml = abbrevCommit(escapedParent); + let parentHtml = escapedParent; if(parentWasLoaded) { - parentHtml = ` - ${parentHtml} - `; + parentHtml = `${parentHtml}`; } if(isComparedToParent) { parentHtml = `${parentHtml}`; From b6f8cf0f51fdbecdd6a6f8e0512f7695e8d5c518 Mon Sep 17 00:00:00 2001 From: Dan Arad Date: Wed, 21 Apr 2021 15:32:41 +0300 Subject: [PATCH 14/27] Change behavior of button to a context menu instead of toggle. --- web/main.ts | 36 +++++++++++++++++++++++++----------- 1 file changed, 25 insertions(+), 11 deletions(-) diff --git a/web/main.ts b/web/main.ts index 6f9d3dc0..0e019638 100644 --- a/web/main.ts +++ b/web/main.ts @@ -2597,7 +2597,7 @@ class GitGraphView { (codeReviewPossible ? '
' + SVG_ICONS.review + '
' : '') + (!expandedCommit.loading ? '
' + SVG_ICONS.fileTree + '
' + SVG_ICONS.fileList + '
' : '') + (externalDiffPossible ? '
' + SVG_ICONS.linkExternal + '
' : '') + - (loadedParents.length > 1 ? '
' + SVG_ICONS.merge + '
' : '') + + (loadedParents.length > 1 ? '
' + SVG_ICONS.merge + '
' : '') + '
'; elem.innerHTML = isDocked ? html : '
' + html + ''; @@ -2669,20 +2669,34 @@ class GitGraphView { this.changeFileViewType(GG.FileViewType.List); }); - document.getElementById('cdvParentToggle')?.addEventListener('click', () => { + document.getElementById('cdvChooseParent')?.addEventListener('click', (event) => { + // Prevent closing of context menu by the same click + event.stopPropagation(); + const expandedCommit = this.expandedCommit!; - const parentIndex = expandedCommit.parentIndex; + const currentParentIndex = expandedCommit.parentIndex; const parents = expandedCommit.commitDetails!.parents; - let nextParentIndex = parentIndex < parents.length ? parentIndex + 1 : 1; - while(!loadedParents.includes(parents[nextParentIndex - 1])) { - if(nextParentIndex === parentIndex) { - break; - } - nextParentIndex = nextParentIndex < parents.length ? parentIndex + 1 : 1; - } + const contextMenuItems = parents.map((parent, index) => { + const parentIndex = index + 1; + const parentCommit = this.commitLookup[parent] !== undefined ? this.commits[this.commitLookup[parent]] : undefined; + const subject = parentCommit?.message.split('\n')[0]; + + return { + title: `[${parentIndex}] ${abbrevCommit(parent)}${subject && `: ${subject}`}`, + visible: parentIndex !== currentParentIndex && loadedParents.includes(parent), + onClick: () => {this.loadCommitDetails(expandedCommit.commitElem!, parentIndex);} + }; + }); + + const target: ContextMenuTarget & CommitTarget = { + type: TargetType.CommitDetailsView, + hash: expandedCommit.commitHash, + index: this.commitLookup[expandedCommit.commitHash], + elem: document.getElementById('cdvChooseParent')! + }; - this.loadCommitDetails(expandedCommit.commitElem!, nextParentIndex); + contextMenu.show([contextMenuItems], false, target, event, this.isCdvDocked() ? document.body : this.viewElem); }); if (codeReviewPossible) { From ce741d72a081b39a9201abc45a659b2453c4d997 Mon Sep 17 00:00:00 2001 From: Dan Arad Date: Wed, 21 Apr 2021 17:42:25 +0300 Subject: [PATCH 15/27] Remove `loadedParents`. Go back to using the parents. We'll need to show an error message in cases where links of parents that aren't loaded are clicked. --- web/main.ts | 21 +++++++++------------ 1 file changed, 9 insertions(+), 12 deletions(-) diff --git a/web/main.ts b/web/main.ts index 0e019638..e274ca28 100644 --- a/web/main.ts +++ b/web/main.ts @@ -2526,7 +2526,6 @@ class GitGraphView { const commitOrder = this.getCommitOrder(expandedCommit.commitHash, expandedCommit.compareWithHash === null ? expandedCommit.commitHash : expandedCommit.compareWithHash); const codeReviewPossible = !expandedCommit.loading && commitOrder.to !== UNCOMMITTED; const externalDiffPossible = !expandedCommit.loading && (expandedCommit.compareWithHash !== null || this.commits[this.commitLookup[expandedCommit.commitHash]].parents.length > 0); - const loadedParents = expandedCommit.commitDetails?.parents.filter(parent => this.commitLookup[parent]) || []; if (elem === null) { elem = document.createElement(isDocked ? 'div' : 'tr'); @@ -2558,15 +2557,11 @@ class GitGraphView { const commitDetails = expandedCommit.commitDetails!; const parents = commitDetails.parents.length > 0 ? commitDetails.parents.map((parent, parentIndex) => { - const escapedParent = escapeHtml(abbrevCommit(parent)); - - const parentWasLoaded = loadedParents.includes(parent); + const escapedParent = escapeHtml(parent); + const escapedAbbreviatedParent = escapeHtml(abbrevCommit(parent)); const isComparedToParent = parentIndex + 1 === expandedCommit.parentIndex; - let parentHtml = escapedParent; - if(parentWasLoaded) { - parentHtml = `${parentHtml}`; - } + let parentHtml = `${escapedAbbreviatedParent}`; if(isComparedToParent) { parentHtml = `${parentHtml}`; } @@ -2597,7 +2592,7 @@ class GitGraphView { (codeReviewPossible ? '
' + SVG_ICONS.review + '
' : '') + (!expandedCommit.loading ? '
' + SVG_ICONS.fileTree + '
' + SVG_ICONS.fileList + '
' : '') + (externalDiffPossible ? '
' + SVG_ICONS.linkExternal + '
' : '') + - (loadedParents.length > 1 ? '
' + SVG_ICONS.merge + '
' : '') + + (expandedCommit.commitDetails && expandedCommit.commitDetails.parents.length > 1 ? '
' + SVG_ICONS.merge + '
' : '') + '
'; elem.innerHTML = isDocked ? html : '
' + html + ''; @@ -2683,9 +2678,11 @@ class GitGraphView { const subject = parentCommit?.message.split('\n')[0]; return { - title: `[${parentIndex}] ${abbrevCommit(parent)}${subject && `: ${subject}`}`, - visible: parentIndex !== currentParentIndex && loadedParents.includes(parent), - onClick: () => {this.loadCommitDetails(expandedCommit.commitElem!, parentIndex);} + title: `[${parentIndex}] ${abbrevCommit(parent)}${subject ? `: ${subject}` : ''}`, + visible: parentIndex !== currentParentIndex, + onClick: () => { + this.loadCommitDetails(expandedCommit.commitElem!, parentIndex); + } }; }); From 9d08f717f53cccdd7ed4e03fbbb0ea536987eb54 Mon Sep 17 00:00:00 2001 From: Dan Arad Date: Wed, 21 Apr 2021 17:56:41 +0300 Subject: [PATCH 16/27] Add error message when commit cannot be found --- web/main.ts | 34 +++++++++++++++++++++++++--------- 1 file changed, 25 insertions(+), 9 deletions(-) diff --git a/web/main.ts b/web/main.ts index e274ca28..51446999 100644 --- a/web/main.ts +++ b/web/main.ts @@ -2094,15 +2094,18 @@ class GitGraphView { const value = unescapeHtml((e.target).dataset.value!); switch ((e.target).dataset.type!) { case 'commit': - if (typeof this.commitLookup[value] === 'number' && (this.expandedCommit === null || this.expandedCommit.commitHash !== value || this.expandedCommit.compareWithHash !== null)) { - const elem = findCommitElemWithId(getCommitElems(), this.commitLookup[value]); - if (elem === null) return; - if (e.ctrlKey || e.metaKey) { - if(this.expandedCommit !== null && this.expandedCommit.commitElem !== null && this.expandedCommit.commitDetails !== null) { - const parentIndex = this.expandedCommit.commitDetails.parents.indexOf(value) + 1; - this.loadCommitDetails(this.expandedCommit.commitElem, parentIndex); + if (e.ctrlKey || e.metaKey) { + if(this.expandedCommit !== null && this.expandedCommit.commitElem !== null && this.expandedCommit.commitDetails !== null) { + const parentIndex = this.expandedCommit.commitDetails.parents.indexOf(value) + 1; + this.loadCommitDetails(this.expandedCommit.commitElem, parentIndex); + } + } else { + if (this.expandedCommit === null || this.expandedCommit.commitHash !== value || this.expandedCommit.compareWithHash !== null) { + const elem = findCommitElemWithId(getCommitElems(), this.commitLookup[value] || null); + if (elem === null) { + this.showErrorOnNonLoadedCommit(); + return; } - } else { this.loadCommitDetails(elem, DEFAULT_PARENT_INDEX); } } @@ -2357,7 +2360,10 @@ class GitGraphView { public loadCommitDetails(commitElem: HTMLElement, parentIndex: number) { const commit = this.getCommitOfElem(commitElem); - if (commit === null) return; + if (commit === null) { + this.showErrorOnNonLoadedCommit(); + return; + } this.closeCommitDetails(false); this.saveExpandedCommitLoading(parseInt(commitElem.dataset.id!), commit.hash, commitElem, null, null); @@ -2366,6 +2372,16 @@ class GitGraphView { this.requestCommitDetails(commit.hash, false, parentIndex); } + private showErrorOnNonLoadedCommit() { + dialog.showError('The commit could not be found in the loaded commits.
' + + 'This can happen for several reasons:' + + '
    ' + + '
  1. The commit is further down the tree and hasn\'t been loaded yet. Try loading more commits.
    2. ' + + '
  2. Filtering has been applied. Try removing any filters.
    3. ' + + '
  3. You are trying to see a commit present in the reflog, but not in the normal log (e.g. stash parents). Turn "Include Commits Mentioned By Reflogs" on in the extension settings.
    4. ' + + '
', null, null, null); + } + public closeCommitDetails(saveAndRender: boolean) { const expandedCommit = this.expandedCommit; if (expandedCommit === null) return; From 25ae0b14b28a4154b06047b9c1403280280ef9c6 Mon Sep 17 00:00:00 2001 From: Dan Arad Date: Wed, 21 Apr 2021 18:09:56 +0300 Subject: [PATCH 17/27] Add option to load more commits if commit is not found --- web/main.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/web/main.ts b/web/main.ts index 51446999..6b2b4831 100644 --- a/web/main.ts +++ b/web/main.ts @@ -2373,13 +2373,15 @@ class GitGraphView { } private showErrorOnNonLoadedCommit() { + const actionName = this.moreCommitsAvailable ? 'Load More Commits' : null; + dialog.showError('The commit could not be found in the loaded commits.
' + 'This can happen for several reasons:' + '
    ' + '
  1. The commit is further down the tree and hasn\'t been loaded yet. Try loading more commits.
    2. ' + '
  2. Filtering has been applied. Try removing any filters.
    3. ' + '
  3. You are trying to see a commit present in the reflog, but not in the normal log (e.g. stash parents). Turn "Include Commits Mentioned By Reflogs" on in the extension settings.
    4. ' + - '
', null, null, null); + '', null, actionName, () => {this.loadMoreCommits();}); } public closeCommitDetails(saveAndRender: boolean) { From c9915f2f95d1534a11fe865ce3dc64954b61a8e8 Mon Sep 17 00:00:00 2001 From: Dan Arad Date: Wed, 21 Apr 2021 18:30:02 +0300 Subject: [PATCH 18/27] Escape titles of context menu items --- web/main.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/web/main.ts b/web/main.ts index 6b2b4831..cde6da9b 100644 --- a/web/main.ts +++ b/web/main.ts @@ -2696,7 +2696,7 @@ class GitGraphView { const subject = parentCommit?.message.split('\n')[0]; return { - title: `[${parentIndex}] ${abbrevCommit(parent)}${subject ? `: ${subject}` : ''}`, + title: escapeHtml(`[${parentIndex}] ${abbrevCommit(parent)}${subject ? `: ${subject}` : ''}`), visible: parentIndex !== currentParentIndex, onClick: () => { this.loadCommitDetails(expandedCommit.commitElem!, parentIndex); From 4e08379b53c4445d6706373a1658d5196fe90ed0 Mon Sep 17 00:00:00 2001 From: Dan Arad Date: Sun, 25 Apr 2021 15:52:29 +0300 Subject: [PATCH 19/27] Fix tests syntactically --- tests/gitGraphView.test.ts | 3 +++ 1 file changed, 3 insertions(+) diff --git a/tests/gitGraphView.test.ts b/tests/gitGraphView.test.ts index fb64254f..42360937 100644 --- a/tests/gitGraphView.test.ts +++ b/tests/gitGraphView.test.ts @@ -959,6 +959,7 @@ describe('GitGraphView', () => { repo: '/path/to/repo', commitHash: '1a2b3c4d5e6f1a2b3c4d5e6f1a2b3c4d5e6f1a2b', hasParents: true, + parentIndex: 1, stash: null, avatarEmail: 'user@mhutchie.com', refresh: false @@ -996,6 +997,7 @@ describe('GitGraphView', () => { repo: '/path/to/repo', commitHash: utils.UNCOMMITTED, hasParents: true, + parentIndex: 1, stash: null, avatarEmail: null, refresh: false @@ -1040,6 +1042,7 @@ describe('GitGraphView', () => { repo: '/path/to/repo', commitHash: '1a2b3c4d5e6f1a2b3c4d5e6f1a2b3c4d5e6f1a2b', hasParents: true, + parentIndex: 1, stash: stash, avatarEmail: null, refresh: false From 5227d5ea2f09babe114cafcdca911ff1a8fd122d Mon Sep 17 00:00:00 2001 From: Dan Arad Date: Sun, 25 Apr 2021 16:03:40 +0300 Subject: [PATCH 20/27] Replace template literals with string concatenation for consistency --- web/main.ts | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/web/main.ts b/web/main.ts index cde6da9b..700bf8f3 100644 --- a/web/main.ts +++ b/web/main.ts @@ -2115,15 +2115,17 @@ class GitGraphView { }; const getParentIndexFromClass = (element: Element) => { + const parentClassPreamble = CLASS_PARENT + '-'; + const eventTargetClasses = element.classList.value.split(' '); - const parentIndexClasses = eventTargetClasses.filter(className => className.startsWith(`${CLASS_PARENT}-`)); + const parentIndexClasses = eventTargetClasses.filter(className => className.startsWith(parentClassPreamble)); const isParentElement = parentIndexClasses.length === 1; if(!isParentElement) { return -1; } - return parseInt(parentIndexClasses[0].substring(`${CLASS_PARENT}-`.length)); + return parseInt(parentIndexClasses[0].substring(parentClassPreamble.length)); }; document.body.addEventListener('click', followInternalLink); @@ -2579,9 +2581,9 @@ class GitGraphView { const escapedAbbreviatedParent = escapeHtml(abbrevCommit(parent)); const isComparedToParent = parentIndex + 1 === expandedCommit.parentIndex; - let parentHtml = `${escapedAbbreviatedParent}`; + let parentHtml = '' + escapedAbbreviatedParent + ''; if(isComparedToParent) { - parentHtml = `${parentHtml}`; + parentHtml = '' + parentHtml + ''; } return parentHtml; @@ -2696,7 +2698,7 @@ class GitGraphView { const subject = parentCommit?.message.split('\n')[0]; return { - title: escapeHtml(`[${parentIndex}] ${abbrevCommit(parent)}${subject ? `: ${subject}` : ''}`), + title: escapeHtml('[' + parentIndex + '] ' + abbrevCommit(parent) + (subject ? ': ' + subject : '')), visible: parentIndex !== currentParentIndex, onClick: () => { this.loadCommitDetails(expandedCommit.commitElem!, parentIndex); From 59e671a62d4055148cec7252ada15f7b22050b34 Mon Sep 17 00:00:00 2001 From: Dan Arad Date: Sun, 25 Apr 2021 16:30:56 +0300 Subject: [PATCH 21/27] Fix tests --- tests/gitGraphView.test.ts | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/tests/gitGraphView.test.ts b/tests/gitGraphView.test.ts index 42360937..5fab2a31 100644 --- a/tests/gitGraphView.test.ts +++ b/tests/gitGraphView.test.ts @@ -967,13 +967,14 @@ describe('GitGraphView', () => { // Assert await waitForExpect(() => { - expect(spyOnGetCommitDetails).toHaveBeenCalledWith('/path/to/repo', '1a2b3c4d5e6f1a2b3c4d5e6f1a2b3c4d5e6f1a2b', true); + expect(spyOnGetCommitDetails).toHaveBeenCalledWith('/path/to/repo', '1a2b3c4d5e6f1a2b3c4d5e6f1a2b3c4d5e6f1a2b', true, 1); expect(spyOnGetAvatarImage).toHaveBeenCalledWith('user@mhutchie.com'); expect(spyOnGetCodeReview).toHaveBeenCalledWith('/path/to/repo', '1a2b3c4d5e6f1a2b3c4d5e6f1a2b3c4d5e6f1a2b'); expect(messages).toStrictEqual([ { command: 'commitDetails', commitDetails: null, + parentIndex: 1, avatar: getAvatarImageResolvedValue, codeReview: getCodeReviewResolvedValue, refresh: false, @@ -1012,6 +1013,7 @@ describe('GitGraphView', () => { { command: 'commitDetails', commitDetails: null, + parentIndex: 1, avatar: null, codeReview: null, refresh: false, @@ -1050,13 +1052,14 @@ describe('GitGraphView', () => { // Assert await waitForExpect(() => { - expect(spyOnGetStashDetails).toHaveBeenCalledWith('/path/to/repo', '1a2b3c4d5e6f1a2b3c4d5e6f1a2b3c4d5e6f1a2b', stash); + expect(spyOnGetStashDetails).toHaveBeenCalledWith('/path/to/repo', '1a2b3c4d5e6f1a2b3c4d5e6f1a2b3c4d5e6f1a2b', stash, 1); expect(spyOnGetAvatarImage).not.toHaveBeenCalled(); expect(spyOnGetCodeReview).toHaveBeenCalledWith('/path/to/repo', '1a2b3c4d5e6f1a2b3c4d5e6f1a2b3c4d5e6f1a2b'); expect(messages).toStrictEqual([ { command: 'commitDetails', commitDetails: null, + parentIndex: 1, avatar: null, codeReview: getCodeReviewResolvedValue, refresh: false, From 5d079543b91a6251a63221f66b89908cff6cfc39 Mon Sep 17 00:00:00 2001 From: Dan Arad Date: Sun, 25 Apr 2021 16:56:56 +0300 Subject: [PATCH 22/27] Make error message show suggestions conditionally. Currently filtering is assumed to always be on. Pending maintainer's decision. --- web/main.ts | 33 ++++++++++++++++++++++++++------- 1 file changed, 26 insertions(+), 7 deletions(-) diff --git a/web/main.ts b/web/main.ts index 700bf8f3..a2a3ccb6 100644 --- a/web/main.ts +++ b/web/main.ts @@ -2376,14 +2376,33 @@ class GitGraphView { private showErrorOnNonLoadedCommit() { const actionName = this.moreCommitsAvailable ? 'Load More Commits' : null; + const filteringActive = true; + const reflogCommitsNotShown = !getIncludeCommitsMentionedByReflogs(this.gitRepos[this.currentRepo].includeCommitsMentionedByReflogs); - dialog.showError('The commit could not be found in the loaded commits.
' + - 'This can happen for several reasons:' + - '
    ' + - '
  1. The commit is further down the tree and hasn\'t been loaded yet. Try loading more commits.
    2. ' + - '
  2. Filtering has been applied. Try removing any filters.
    3. ' + - '
  3. You are trying to see a commit present in the reflog, but not in the normal log (e.g. stash parents). Turn "Include Commits Mentioned By Reflogs" on in the extension settings.
    4. ' + - '
', null, actionName, () => {this.loadMoreCommits();}); + let detailedMessage: string | null; + + if(!this.moreCommitsAvailable && !filteringActive && !reflogCommitsNotShown) { + detailedMessage = null; + } else { + detailedMessage = 'Possible causes:\n'; + + if(this.moreCommitsAvailable) { + detailedMessage += '\n• The commit is further down the tree and hasn\'t been loaded yet. Try loading more commits.\n'; + } + + if(filteringActive) { + detailedMessage += '\n• Filtering has been applied. Try removing any filters.\n'; + } + + if(reflogCommitsNotShown) { + detailedMessage += '\n• You are trying to see a commit present in the reflog, but not in the normal log (e.g. stash parents). Turn "Include Commits Mentioned By Reflogs" on in the extension settings.\n'; + } + + detailedMessage += '\n'; + } + + dialog.showError('The commit could not be found in the loaded commits.', + detailedMessage, actionName, () => {this.loadMoreCommits();}); } public closeCommitDetails(saveAndRender: boolean) { From 87559930c5b8c69995376ad7a989667e129459bb Mon Sep 17 00:00:00 2001 From: Dan Arad Date: Sun, 25 Apr 2021 17:15:05 +0300 Subject: [PATCH 23/27] Remove more template literals --- src/dataSource.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/dataSource.ts b/src/dataSource.ts index f4b0f660..a41bb17a 100644 --- a/src/dataSource.ts +++ b/src/dataSource.ts @@ -346,7 +346,7 @@ export class DataSource extends Disposable { * @returns The commit details. */ public getCommitDetails(repo: string, commitHash: string, hasParents: boolean, parentIndex: number): Promise { - const fromCommit = commitHash + (hasParents ? `^${parentIndex}` : ''); + const fromCommit = commitHash + (hasParents ? '^' + parentIndex : ''); return Promise.all([ this.getCommitDetailsBase(repo, commitHash), this.getDiffNameStatus(repo, fromCommit, commitHash), @@ -368,7 +368,7 @@ export class DataSource extends Disposable { * @returns The stash details. */ public getStashDetails(repo: string, commitHash: string, stash: GitCommitStash, parentIndex: number): Promise { - const fromCommit = commitHash + `^${parentIndex}`; + const fromCommit = commitHash + '^' + parentIndex; return Promise.all([ this.getCommitDetailsBase(repo, commitHash), this.getDiffNameStatus(repo, fromCommit, commitHash), From 6097b1e7987f83a2a855699a3dc6b46852a2ed93 Mon Sep 17 00:00:00 2001 From: Dan Arad Date: Sun, 25 Apr 2021 17:16:15 +0300 Subject: [PATCH 24/27] Use data attribute instead of a class to mark parent index. --- web/main.ts | 18 +++++++----------- web/utils.ts | 1 - 2 files changed, 7 insertions(+), 12 deletions(-) diff --git a/web/main.ts b/web/main.ts index a2a3ccb6..a5ba8f8f 100644 --- a/web/main.ts +++ b/web/main.ts @@ -2114,18 +2114,14 @@ class GitGraphView { } }; - const getParentIndexFromClass = (element: Element) => { - const parentClassPreamble = CLASS_PARENT + '-'; + const getParentIndexFromElement = (element: Element) => { + const parentIndexAsString = (element as HTMLElement).dataset.parent; - const eventTargetClasses = element.classList.value.split(' '); - const parentIndexClasses = eventTargetClasses.filter(className => className.startsWith(parentClassPreamble)); - - const isParentElement = parentIndexClasses.length === 1; - if(!isParentElement) { + if(parentIndexAsString === undefined) { return -1; } - return parseInt(parentIndexClasses[0].substring(parentClassPreamble.length)); + return parseInt(parentIndexAsString); }; document.body.addEventListener('click', followInternalLink); @@ -2168,7 +2164,7 @@ class GitGraphView { isInDialog = true; } - const parentIndex = getParentIndexFromClass(eventTarget); + const parentIndex = getParentIndexFromElement(eventTarget); handledEvent(e); contextMenu.show([ @@ -2187,7 +2183,7 @@ class GitGraphView { }, { title: 'View Changes with this Parent', - visible: isInternalUrl && parentIndex !== -1, + visible: isInternalUrl && parentIndex !== undefined, onClick: () => { this.loadCommitDetails(this.expandedCommit!.commitElem!, parentIndex); } @@ -2600,7 +2596,7 @@ class GitGraphView { const escapedAbbreviatedParent = escapeHtml(abbrevCommit(parent)); const isComparedToParent = parentIndex + 1 === expandedCommit.parentIndex; - let parentHtml = '' + escapedAbbreviatedParent + ''; + let parentHtml = '' + escapedAbbreviatedParent + ''; if(isComparedToParent) { parentHtml = '' + parentHtml + ''; } diff --git a/web/utils.ts b/web/utils.ts index 5f90a889..9bdfb30b 100644 --- a/web/utils.ts +++ b/web/utils.ts @@ -90,7 +90,6 @@ const CLASS_REF_TAG = 'tag'; const CLASS_SELECTED = 'selected'; const CLASS_TAG_LABELS_RIGHT_ALIGNED = 'tagLabelsRightAligned'; const CLASS_TRANSITION = 'transition'; -const CLASS_PARENT = 'parent'; const ID_EVENT_CAPTURE_ELEM = 'eventCaptureElem'; From 4908075fae9856056aa613a3e904c2abd626130e Mon Sep 17 00:00:00 2001 From: Dan Arad Date: Sun, 25 Apr 2021 17:20:22 +0300 Subject: [PATCH 25/27] Refactor error message creation, by assuming that the line about filtering is always present. --- web/main.ts | 27 +++++++++------------------ 1 file changed, 9 insertions(+), 18 deletions(-) diff --git a/web/main.ts b/web/main.ts index a5ba8f8f..71c10d77 100644 --- a/web/main.ts +++ b/web/main.ts @@ -2372,31 +2372,22 @@ class GitGraphView { private showErrorOnNonLoadedCommit() { const actionName = this.moreCommitsAvailable ? 'Load More Commits' : null; - const filteringActive = true; const reflogCommitsNotShown = !getIncludeCommitsMentionedByReflogs(this.gitRepos[this.currentRepo].includeCommitsMentionedByReflogs); - let detailedMessage: string | null; + let detailedMessage = 'Possible causes:\n'; - if(!this.moreCommitsAvailable && !filteringActive && !reflogCommitsNotShown) { - detailedMessage = null; - } else { - detailedMessage = 'Possible causes:\n'; - - if(this.moreCommitsAvailable) { - detailedMessage += '\n• The commit is further down the tree and hasn\'t been loaded yet. Try loading more commits.\n'; - } - - if(filteringActive) { - detailedMessage += '\n• Filtering has been applied. Try removing any filters.\n'; - } + detailedMessage += '\n• Filtering has been applied. Try removing any filters.\n'; - if(reflogCommitsNotShown) { - detailedMessage += '\n• You are trying to see a commit present in the reflog, but not in the normal log (e.g. stash parents). Turn "Include Commits Mentioned By Reflogs" on in the extension settings.\n'; - } + if(this.moreCommitsAvailable) { + detailedMessage += '\n• The commit is further down the tree and hasn\'t been loaded yet. Try loading more commits.\n'; + } - detailedMessage += '\n'; + if(reflogCommitsNotShown) { + detailedMessage += '\n• You are trying to see a commit present in the reflog, but not in the normal log (e.g. stash parents). Turn "Include Commits Mentioned By Reflogs" on in the extension settings.\n'; } + detailedMessage += '\n'; + dialog.showError('The commit could not be found in the loaded commits.', detailedMessage, actionName, () => {this.loadMoreCommits();}); } From 929350eb42bf62ce600c1b2383e772ed2af91da5 Mon Sep 17 00:00:00 2001 From: Dan Arad Date: Sun, 25 Apr 2021 17:27:20 +0300 Subject: [PATCH 26/27] Change button title to match the action better. Switch from "Toggle Parent" to "View Changes with an Alternate Parent...". --- web/main.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/web/main.ts b/web/main.ts index 71c10d77..8e16d4ad 100644 --- a/web/main.ts +++ b/web/main.ts @@ -2618,7 +2618,7 @@ class GitGraphView { (codeReviewPossible ? '
' + SVG_ICONS.review + '
' : '') + (!expandedCommit.loading ? '
' + SVG_ICONS.fileTree + '
' + SVG_ICONS.fileList + '
' : '') + (externalDiffPossible ? '
' + SVG_ICONS.linkExternal + '
' : '') + - (expandedCommit.commitDetails && expandedCommit.commitDetails.parents.length > 1 ? '
' + SVG_ICONS.merge + '
' : '') + + (expandedCommit.commitDetails && expandedCommit.commitDetails.parents.length > 1 ? '
' + SVG_ICONS.merge + '
' : '') + '
'; elem.innerHTML = isDocked ? html : '
' + html + ''; From 6d9336167054dc78cc7e563804b93e94ce34c18a Mon Sep 17 00:00:00 2001 From: Dan Arad Date: Sun, 25 Apr 2021 17:27:54 +0300 Subject: [PATCH 27/27] Change context menu to always show all parents, and put a check mark by the current parent. --- web/main.ts | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/web/main.ts b/web/main.ts index 8e16d4ad..69839bab 100644 --- a/web/main.ts +++ b/web/main.ts @@ -2705,7 +2705,8 @@ class GitGraphView { return { title: escapeHtml('[' + parentIndex + '] ' + abbrevCommit(parent) + (subject ? ': ' + subject : '')), - visible: parentIndex !== currentParentIndex, + visible: true, + checked: parentIndex === currentParentIndex, onClick: () => { this.loadCommitDetails(expandedCommit.commitElem!, parentIndex); } @@ -2719,7 +2720,7 @@ class GitGraphView { elem: document.getElementById('cdvChooseParent')! }; - contextMenu.show([contextMenuItems], false, target, event, this.isCdvDocked() ? document.body : this.viewElem); + contextMenu.show([contextMenuItems], true, target, event, this.isCdvDocked() ? document.body : this.viewElem); }); if (codeReviewPossible) {