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 all 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.

10 changes: 8 additions & 2 deletions tests/gitGraphView.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -959,20 +959,22 @@ describe('GitGraphView', () => {
repo: '/path/to/repo',
commitHash: '1a2b3c4d5e6f1a2b3c4d5e6f1a2b3c4d5e6f1a2b',
hasParents: true,
parentIndex: 1,
stash: null,
avatarEmail: '[email protected]',
refresh: false
});

// Assert
await waitForExpect(() => {
expect(spyOnGetCommitDetails).toHaveBeenCalledWith('/path/to/repo', '1a2b3c4d5e6f1a2b3c4d5e6f1a2b3c4d5e6f1a2b', true);
expect(spyOnGetCommitDetails).toHaveBeenCalledWith('/path/to/repo', '1a2b3c4d5e6f1a2b3c4d5e6f1a2b3c4d5e6f1a2b', true, 1);
expect(spyOnGetAvatarImage).toHaveBeenCalledWith('[email protected]');
expect(spyOnGetCodeReview).toHaveBeenCalledWith('/path/to/repo', '1a2b3c4d5e6f1a2b3c4d5e6f1a2b3c4d5e6f1a2b');
expect(messages).toStrictEqual([
{
command: 'commitDetails',
commitDetails: null,
parentIndex: 1,
avatar: getAvatarImageResolvedValue,
codeReview: getCodeReviewResolvedValue,
refresh: false,
Expand All @@ -996,6 +998,7 @@ describe('GitGraphView', () => {
repo: '/path/to/repo',
commitHash: utils.UNCOMMITTED,
hasParents: true,
parentIndex: 1,
stash: null,
avatarEmail: null,
refresh: false
Expand All @@ -1010,6 +1013,7 @@ describe('GitGraphView', () => {
{
command: 'commitDetails',
commitDetails: null,
parentIndex: 1,
avatar: null,
codeReview: null,
refresh: false,
Expand Down Expand Up @@ -1040,20 +1044,22 @@ describe('GitGraphView', () => {
repo: '/path/to/repo',
commitHash: '1a2b3c4d5e6f1a2b3c4d5e6f1a2b3c4d5e6f1a2b',
hasParents: true,
parentIndex: 1,
stash: stash,
avatarEmail: null,
refresh: false
});

// 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,
Expand Down
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
Loading