Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

#384 View alternate parent changes at merge commits #488

Open
wants to merge 30 commits into
base: develop
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 20 commits
Commits
Show all changes
30 commits
Select commit Hold shift + click to select a range
fe3de81
Select the parent with which to view the diff
dan1994 Mar 20, 2021
90f7b53
Fix existing tests
dan1994 Mar 20, 2021
893bb4c
Add test for requesting 2nd parent
dan1994 Mar 20, 2021
783d6b8
Merge remote-tracking branch 'upstream/develop' into 384-View-alterna…
dan1994 Apr 5, 2021
07bee7a
Show parents on a single line
dan1994 Apr 5, 2021
724c134
Add context menu option for switching parents.
dan1994 Apr 5, 2021
85a29d6
Refactor parent commit rendering for clarity
dan1994 Apr 5, 2021
5a2afcc
Add an icon to toggle parents
dan1994 Apr 5, 2021
3a6bc5a
Improve choice of next parent when toggling
dan1994 Apr 6, 2021
00b53d3
Add support to toggling diffs between stash
dan1994 Apr 6, 2021
d39f5c4
Fix bugs where parentIndex is not reset properly
dan1994 Apr 6, 2021
a77c2ec
Fix lint errors
dan1994 Apr 6, 2021
439ca17
Add parentIndex to existing tests; Add a test for
dan1994 Apr 6, 2021
15f475d
View alternate parent changes at merge commits
dan1994 Apr 21, 2021
cb7fec6
Abbreviate before escaping and remove generated
dan1994 Apr 21, 2021
b6f8cf0
Change behavior of button to a context menu
dan1994 Apr 21, 2021
ce741d7
Remove `loadedParents`.
dan1994 Apr 21, 2021
9d08f71
Add error message when commit cannot be found
dan1994 Apr 21, 2021
25ae0b1
Add option to load more commits if commit is not
dan1994 Apr 21, 2021
c9915f2
Escape titles of context menu items
dan1994 Apr 21, 2021
304601d
Merge remote-tracking branch 'upstream/develop' into 384-View-alterna…
dan1994 Apr 25, 2021
4e08379
Fix tests syntactically
dan1994 Apr 25, 2021
5227d5e
Replace template literals with string
dan1994 Apr 25, 2021
59e671a
Fix tests
dan1994 Apr 25, 2021
5d07954
Make error message show suggestions conditionally.
dan1994 Apr 25, 2021
8755993
Remove more template literals
dan1994 Apr 25, 2021
6097b1e
Use data attribute instead of a class to mark
dan1994 Apr 25, 2021
4908075
Refactor error message creation, by assuming
dan1994 Apr 25, 2021
929350e
Change button title to match the action better.
dan1994 Apr 25, 2021
6d93361
Change context menu to always show all parents,
dan1994 Apr 25, 2021
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 8 additions & 5 deletions src/dataSource.ts
Original file line number Diff line number Diff line change
Expand Up @@ -342,10 +342,11 @@ 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): Promise<GitCommitDetailsData> {
const fromCommit = commitHash + (hasParents ? '^' : '');
public getCommitDetails(repo: string, commitHash: string, hasParents: boolean, parentIndex: number): Promise<GitCommitDetailsData> {
const fromCommit = commitHash + (hasParents ? `^${parentIndex}` : '');
return Promise.all([
this.getCommitDetailsBase(repo, commitHash),
this.getDiffNameStatus(repo, fromCommit, commitHash),
Expand All @@ -363,13 +364,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<GitCommitDetailsData> {
public getStashDetails(repo: string, commitHash: string, stash: GitCommitStash, parentIndex: number): Promise<GitCommitDetailsData> {
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) => {
Expand Down
5 changes: 3 additions & 2 deletions src/gitGraphView.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.getStashDetails(msg.repo, msg.commitHash, msg.stash),
? this.dataSource.getCommitDetails(msg.repo, msg.commitHash, msg.hasParents, msg.parentIndex)
: this.dataSource.getStashDetails(msg.repo, msg.commitHash, msg.stash, msg.parentIndex),
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
Expand Down
2 changes: 2 additions & 0 deletions src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -662,13 +662,15 @@ 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;
}
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;
Expand Down
182 changes: 146 additions & 36 deletions tests/dataSource.test.ts

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion web/findWidget.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}
}
Expand Down
1 change: 1 addition & 0 deletions web/global.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
133 changes: 111 additions & 22 deletions web/main.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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.');
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -746,6 +747,7 @@ class GitGraphView {
index: index,
commitHash: commitHash,
commitElem: commitElem,
parentIndex: DEFAULT_PARENT_INDEX,
compareWithHash: compareWithHash,
compareWithElem: compareWithElem,
commitDetails: null,
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -2046,7 +2048,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, DEFAULT_PARENT_INDEX);
}
} else if (e.key && (e.ctrlKey || e.metaKey)) {
const key = e.key.toLowerCase(), keybindings = this.config.keybindings;
Expand Down Expand Up @@ -2092,15 +2094,38 @@ class GitGraphView {
const value = unescapeHtml((<HTMLElement>e.target).dataset.value!);
switch ((<HTMLElement>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) this.loadCommitDetails(elem);
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;
}
this.loadCommitDetails(elem, DEFAULT_PARENT_INDEX);
}
}
break;
}
}
};

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) => {
Expand Down Expand Up @@ -2141,6 +2166,8 @@ class GitGraphView {
isInDialog = true;
}

const parentIndex = getParentIndexFromClass(eventTarget);

handledEvent(e);
contextMenu.show([
[
Expand All @@ -2156,6 +2183,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,
Expand Down Expand Up @@ -2204,10 +2238,10 @@ class GitGraphView {
this.loadCommitComparison(this.expandedCommit.commitElem, eventElem);
}
} else {
this.loadCommitDetails(eventElem);
this.loadCommitDetails(eventElem, DEFAULT_PARENT_INDEX);
}
} else {
this.loadCommitDetails(eventElem);
this.loadCommitDetails(eventElem, DEFAULT_PARENT_INDEX);
}
}
});
Expand Down Expand Up @@ -2324,15 +2358,30 @@ class GitGraphView {

/* Commit Details View */

public loadCommitDetails(commitElem: HTMLElement) {
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);
commitElem.classList.add(CLASS_COMMIT_DETAILS_OPEN);
this.renderCommitDetailsView(false);
this.requestCommitDetails(commit.hash, false);
this.requestCommitDetails(commit.hash, false, parentIndex);
}

private showErrorOnNonLoadedCommit() {
const actionName = this.moreCommitsAvailable ? 'Load More Commits' : null;

dialog.showError('The commit could not be found in the loaded commits.<br>' +
dan1994 marked this conversation as resolved.
Show resolved Hide resolved
'This can happen for several reasons:' +
'<ol>' +
dan1994 marked this conversation as resolved.
Show resolved Hide resolved
'<li>The commit is further down the tree and hasn\'t been loaded yet. Try loading more commits.</li>' +
'<li>Filtering has been applied. Try removing any filters.</li>' +
'<li>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.</li>' +
'</ol>', null, actionName, () => {this.loadMoreCommits();});
}

public closeCommitDetails(saveAndRender: boolean) {
Expand Down Expand Up @@ -2362,7 +2411,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;

Expand All @@ -2377,6 +2426,7 @@ class GitGraphView {
expandedCommit.fileTree = fileTree;
GitGraphView.closeCdvContextMenuIfOpen(expandedCommit);
}
expandedCommit.parentIndex = parentIndex;
expandedCommit.avatar = avatar;
expandedCommit.codeReview = codeReview;
if (!refresh) {
Expand Down Expand Up @@ -2455,7 +2505,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);
}
Expand Down Expand Up @@ -2524,11 +2574,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);
return typeof this.commitLookup[parent] === 'number'
? '<span class="' + CLASS_INTERNAL_URL + '" data-type="commit" data-value="' + escapedParent + '" tabindex="-1">' + escapedParent + '</span>'
: escapedParent;
const escapedAbbreviatedParent = escapeHtml(abbrevCommit(parent));
const isComparedToParent = parentIndex + 1 === expandedCommit.parentIndex;

let parentHtml = `<span class="${CLASS_INTERNAL_URL} ${CLASS_PARENT}-${parentIndex + 1}" data-type="commit" data-value="${escapedParent}" tabindex="-1">${escapedAbbreviatedParent}</span>`;
dan1994 marked this conversation as resolved.
Show resolved Hide resolved
if(isComparedToParent) {
parentHtml = `<b>${parentHtml}</b>`;
}

return parentHtml;
}).join(', ')
: 'None';
html += '<span class="cdvSummaryTop' + (expandedCommit.avatar !== null ? ' withAvatar' : '') + '"><span class="cdvSummaryTopRow"><span class="cdvSummaryKeyValues">'
Expand All @@ -2554,6 +2610,7 @@ class GitGraphView {
(codeReviewPossible ? '<div id="cdvCodeReview" class="cdvControlBtn">' + SVG_ICONS.review + '</div>' : '') +
(!expandedCommit.loading ? '<div id="cdvFileViewTypeTree" class="cdvControlBtn cdvFileViewTypeBtn" title="File Tree View">' + SVG_ICONS.fileTree + '</div><div id="cdvFileViewTypeList" class="cdvControlBtn cdvFileViewTypeBtn" title="File List View">' + SVG_ICONS.fileList + '</div>' : '') +
(externalDiffPossible ? '<div id="cdvExternalDiff" class="cdvControlBtn">' + SVG_ICONS.linkExternal + '</div>' : '') +
(expandedCommit.commitDetails && expandedCommit.commitDetails.parents.length > 1 ? '<div id="cdvChooseParent" class="cdvControlBtn" title="Toggle Parent">' + SVG_ICONS.merge + '</div>' : '') +
dan1994 marked this conversation as resolved.
Show resolved Hide resolved
'</div><div class="cdvHeightResize"></div>';

elem.innerHTML = isDocked ? html : '<td><div class="cdvHeightResize"></div></td><td colspan="' + (this.getNumColumns() - 1) + '">' + html + '</td>';
Expand Down Expand Up @@ -2625,6 +2682,38 @@ class GitGraphView {
this.changeFileViewType(GG.FileViewType.List);
});

document.getElementById('cdvChooseParent')?.addEventListener('click', (event) => {
// Prevent closing of context menu by the same click
event.stopPropagation();

const expandedCommit = this.expandedCommit!;
const currentParentIndex = expandedCommit.parentIndex;
const parents = expandedCommit.commitDetails!.parents;

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 {
dan1994 marked this conversation as resolved.
Show resolved Hide resolved
title: escapeHtml(`[${parentIndex}] ${abbrevCommit(parent)}${subject ? `: ${subject}` : ''}`),
visible: parentIndex !== currentParentIndex,
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')!
};

contextMenu.show([contextMenuItems], false, target, event, this.isCdvDocked() ? document.body : this.viewElem);
});

if (codeReviewPossible) {
this.renderCodeReviewBtn();
document.getElementById('cdvCodeReview')!.addEventListener('click', (e) => {
Expand Down Expand Up @@ -3170,7 +3259,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);
Expand Down
Loading