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

Bug/#4497 Unable to Cherry Pick Merge Commit Solved #4944

Merged
Show file tree
Hide file tree
Changes from 17 commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
13d85b6
Parser and Logic For Parent Commit Added
RounakJoshi09 Oct 11, 2023
3118c7c
Unit Test Cases Added
RounakJoshi09 Oct 13, 2023
6e5cd2b
All Unit Tests Passing
RounakJoshi09 Oct 14, 2023
4051b42
documentation added, Tests Fixed
RounakJoshi09 Oct 14, 2023
0a0bd0a
Merge branch 'develop' into bug/#4497_unable-to-cherrypick-merge-commit
RounakJoshi09 Oct 14, 2023
d8500f9
Suggested Changes FOR PR DONE
RounakJoshi09 Oct 18, 2023
6eee897
Merge branch 'bug/#4497_unable-to-cherrypick-merge-commit' of https:/…
RounakJoshi09 Oct 18, 2023
d65191f
Merge branch 'develop' into bug/#4497_unable-to-cherrypick-merge-commit
RounakJoshi09 Oct 19, 2023
827808d
Merge Conflict Resolved
RounakJoshi09 Oct 19, 2023
f92ad63
Merge branch 'develop' into bug/#4497_unable-to-cherrypick-merge-commit
RounakJoshi09 Oct 22, 2023
58e9e56
e2e test case added
RounakJoshi09 Oct 22, 2023
b0cfdcc
Documentation Modified New Ex Added
RounakJoshi09 Oct 23, 2023
122e592
Merge branch 'develop' into bug/#4497_unable-to-cherrypick-merge-commit
RounakJoshi09 Oct 23, 2023
ca96c0f
Linting Issue Fixed
RounakJoshi09 Oct 23, 2023
b461efa
Merge branch 'bug/#4497_unable-to-cherrypick-merge-commit' of https:/…
RounakJoshi09 Oct 23, 2023
7f1e0ab
Updated gitgraph.md
RounakJoshi09 Oct 23, 2023
5834818
Linting Issue Fixed
RounakJoshi09 Oct 23, 2023
aadf533
Error Hash Removed
RounakJoshi09 Nov 17, 2023
c19fa12
Merge branch 'develop' into bug/#4497_unable-to-cherrypick-merge-commit
RounakJoshi09 Nov 17, 2023
c6bf908
Merge branch 'develop' into bug/#4497_unable-to-cherrypick-merge-commit
RounakJoshi09 Dec 3, 2023
31a1de1
Condition of Parent Id Without Merge Commit Added
RounakJoshi09 Dec 3, 2023
d22ee8d
fix: Check if parentCommit is provided
sidharthv96 Dec 4, 2023
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
17 changes: 17 additions & 0 deletions cypress/integration/rendering/gitGraph.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -701,4 +701,21 @@ gitGraph TB:
{}
);
});
it('34: should render a simple gitgraph with cherry pick merge commit', () => {
imgSnapshotTest(
`gitGraph
commit id: "ZERO"
branch feature
branch release
checkout feature
commit id: "A"
commit id: "B"
checkout main
merge feature id: "M"
checkout release
cherry-pick id: "M" parent:"B"
`,
{}
);
});
});
60 changes: 34 additions & 26 deletions docs/syntax/gitgraph.md
Original file line number Diff line number Diff line change
Expand Up @@ -366,41 +366,49 @@ A few important rules to note here are:
1. You need to provide the `id` for an existing commit to be cherry-picked. If given commit id does not exist it will result in an error. For this, make use of the `commit id:$value` format of declaring commits. See the examples from above.
2. The given commit must not exist on the current branch. The cherry-picked commit must always be a different branch than the current branch.
3. Current branch must have at least one commit, before you can cherry-pick, otherwise it will cause an error is throw.
4. When cherry-picking a merge commit, providing a parent commit ID is mandatory. If the parent attribute is omitted or an invalid parent commit ID is provided, an error will be thrown.
5. The specified parent commit must be an immediate parent of the merge commit being cherry-picked.

Let see an example:

```mermaid-example
gitGraph
commit id: "ZERO"
branch develop
commit id:"A"
checkout main
commit id:"ONE"
checkout develop
commit id:"B"
checkout main
commit id:"TWO"
cherry-pick id:"A"
commit id:"THREE"
checkout develop
commit id:"C"
commit id: "ZERO"
branch develop
branch release
commit id:"A"
checkout main
commit id:"ONE"
checkout develop
commit id:"B"
checkout main
merge develop id:"MERGE"
commit id:"TWO"
checkout release
cherry-pick id:"MERGE" parent:"B"
commit id:"THREE"
checkout develop
commit id:"C"
```

```mermaid
gitGraph
commit id: "ZERO"
branch develop
commit id:"A"
checkout main
commit id:"ONE"
checkout develop
commit id:"B"
checkout main
commit id:"TWO"
cherry-pick id:"A"
commit id:"THREE"
checkout develop
commit id:"C"
commit id: "ZERO"
branch develop
branch release
commit id:"A"
checkout main
commit id:"ONE"
checkout develop
commit id:"B"
checkout main
merge develop id:"MERGE"
commit id:"TWO"
checkout release
cherry-pick id:"MERGE" parent:"B"
commit id:"THREE"
checkout develop
commit id:"C"
```

## Gitgraph specific configuration options
Expand Down
45 changes: 31 additions & 14 deletions packages/mermaid/src/diagrams/git/gitGraphAst.js
Original file line number Diff line number Diff line change
Expand Up @@ -255,11 +255,12 @@
log.debug('in mergeBranch');
};

export const cherryPick = function (sourceId, targetId, tag) {
export const cherryPick = function (sourceId, targetId, tag, parentCommitId) {
log.debug('Entering cherryPick:', sourceId, targetId, tag);
sourceId = common.sanitizeText(sourceId, getConfig());
targetId = common.sanitizeText(targetId, getConfig());
tag = common.sanitizeText(tag, getConfig());
parentCommitId = common.sanitizeText(parentCommitId, getConfig());

if (!sourceId || commits[sourceId] === undefined) {
let error = new Error(
Expand All @@ -274,21 +275,33 @@
};
throw error;
}

let sourceCommit = commits[sourceId];
let sourceCommitBranch = sourceCommit.branch;
if (sourceCommit.type === commitType.MERGE) {
let error = new Error(
'Incorrect usage of "cherryPick". Source commit should not be a merge commit'
);
error.hash = {
text: 'cherryPick ' + sourceId + ' ' + targetId,
token: 'cherryPick ' + sourceId + ' ' + targetId,
line: '1',
loc: { first_line: 1, last_line: 1, first_column: 1, last_column: 1 },
expected: ['cherry-pick abc'],
};
throw error;
if (!parentCommitId) {
let error = new Error(

Check warning on line 282 in packages/mermaid/src/diagrams/git/gitGraphAst.js

View check run for this annotation

Codecov / codecov/patch

packages/mermaid/src/diagrams/git/gitGraphAst.js#L282

Added line #L282 was not covered by tests
'Incorrect usage of cherry-pick: If the source commit is a merge commit, an immediate parent commit must be specified.'
);
error.hash = {

Check warning on line 285 in packages/mermaid/src/diagrams/git/gitGraphAst.js

View check run for this annotation

Codecov / codecov/patch

packages/mermaid/src/diagrams/git/gitGraphAst.js#L285

Added line #L285 was not covered by tests
text: `cherryPick ${sourceId} ${targetId}`,
token: `cherryPick ${sourceId} ${targetId}`,
line: '1',
loc: { first_line: 1, last_line: 1, first_column: 1, last_column: 1 },
sidharthv96 marked this conversation as resolved.
Show resolved Hide resolved
};
throw error;

Check warning on line 291 in packages/mermaid/src/diagrams/git/gitGraphAst.js

View check run for this annotation

Codecov / codecov/patch

packages/mermaid/src/diagrams/git/gitGraphAst.js#L291

Added line #L291 was not covered by tests
}
if (!(Array.isArray(sourceCommit.parents) && sourceCommit.parents.includes(parentCommitId))) {
let error = new Error(

Check warning on line 294 in packages/mermaid/src/diagrams/git/gitGraphAst.js

View check run for this annotation

Codecov / codecov/patch

packages/mermaid/src/diagrams/git/gitGraphAst.js#L294

Added line #L294 was not covered by tests
'Invalid operation: The specified parent commit is not an immediate parent of the cherry-picked commit.'
);
error.hash = {

Check warning on line 297 in packages/mermaid/src/diagrams/git/gitGraphAst.js

View check run for this annotation

Codecov / codecov/patch

packages/mermaid/src/diagrams/git/gitGraphAst.js#L297

Added line #L297 was not covered by tests
text: `cherryPick ${sourceId} ${targetId}`,
token: `cherryPick ${sourceId} ${targetId}`,
line: '1',
loc: { first_line: 1, last_line: 1, first_column: 1, last_column: 1 },
};
throw error;

Check warning on line 303 in packages/mermaid/src/diagrams/git/gitGraphAst.js

View check run for this annotation

Codecov / codecov/patch

packages/mermaid/src/diagrams/git/gitGraphAst.js#L303

Added line #L303 was not covered by tests
}
}
if (!targetId || commits[targetId] === undefined) {
// cherry-pick source commit to current branch
Expand Down Expand Up @@ -327,7 +340,11 @@
parents: [head == null ? null : head.id, sourceCommit.id],
branch: curBranch,
type: commitType.CHERRY_PICK,
tag: tag ?? 'cherry-pick:' + sourceCommit.id,
tag:
tag ??
`cherry-pick:${sourceCommit.id}${
sourceCommit.type === commitType.MERGE ? `|parent:${parentCommitId}` : ''
}`,
};
head = commit;
commits[commit.id] = commit;
Expand Down
139 changes: 139 additions & 0 deletions packages/mermaid/src/diagrams/git/gitGraphParserV2.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -673,6 +673,145 @@ describe('when parsing a gitGraph', function () {
expect(commits[cherryPickCommitID].branch).toBe('main');
});

it('should support cherry-picking of merge commits', function () {
const str = `gitGraph
commit id: "ZERO"
branch feature
branch release
checkout feature
commit id: "A"
commit id: "B"
checkout main
merge feature id: "M"
checkout release
cherry-pick id: "M" parent:"B"
`;

parser.parse(str);
const commits = parser.yy.getCommits();
const cherryPickCommitID = Object.keys(commits)[4];
expect(commits[cherryPickCommitID].tag).toBe('cherry-pick:M|parent:B');
expect(commits[cherryPickCommitID].branch).toBe('release');
});

it('should support cherry-picking of merge commits with tag', function () {
const str = `gitGraph
commit id: "ZERO"
branch feature
branch release
checkout feature
commit id: "A"
commit id: "B"
checkout main
merge feature id: "M"
checkout release
cherry-pick id: "M" parent:"ZERO" tag: "v1.0"
`;

parser.parse(str);
const commits = parser.yy.getCommits();
const cherryPickCommitID = Object.keys(commits)[4];
expect(commits[cherryPickCommitID].tag).toBe('v1.0');
expect(commits[cherryPickCommitID].branch).toBe('release');
});

it('should support cherry-picking of merge commits with additional commit', function () {
const str = `gitGraph
commit id: "ZERO"
branch feature
branch release
checkout feature
commit id: "A"
commit id: "B"
checkout main
merge feature id: "M"
checkout release
commit id: "C"
cherry-pick id: "M" tag: "v2.1:ZERO" parent:"ZERO"
commit id: "D"
`;

parser.parse(str);
const commits = parser.yy.getCommits();
const cherryPickCommitID = Object.keys(commits)[5];
expect(commits[cherryPickCommitID].tag).toBe('v2.1:ZERO');
expect(commits[cherryPickCommitID].branch).toBe('release');
});

it('should support cherry-picking of merge commits with empty tag', function () {
const str = `gitGraph
commit id: "ZERO"
branch feature
branch release
checkout feature
commit id: "A"
commit id: "B"
checkout main
merge feature id: "M"
checkout release
commit id: "C"
cherry-pick id:"M" parent: "ZERO" tag:""
commit id: "D"
cherry-pick id:"M" tag:"" parent: "B"
`;

parser.parse(str);
const commits = parser.yy.getCommits();
const cherryPickCommitID = Object.keys(commits)[5];
const cherryPickCommitID2 = Object.keys(commits)[7];
expect(commits[cherryPickCommitID].tag).toBe('');
expect(commits[cherryPickCommitID2].tag).toBe('');
expect(commits[cherryPickCommitID].branch).toBe('release');
});

it('should fail cherry-picking of merge commits if the parent of merge commits is not specified', function () {
expect(() =>
parser
.parse(
`gitGraph
commit id: "ZERO"
branch feature
branch release
checkout feature
commit id: "A"
commit id: "B"
checkout main
merge feature id: "M"
checkout release
commit id: "C"
cherry-pick id:"M"
`
)
.toThrow(
'Incorrect usage of cherry-pick: If the source commit is a merge commit, an immediate parent commit must be specified.'
)
);
});

it('should fail cherry-picking of merge commits when the parent provided is not an immediate parent of cherry picked commit', function () {
expect(() =>
parser
.parse(
`gitGraph
commit id: "ZERO"
branch feature
branch release
checkout feature
commit id: "A"
commit id: "B"
checkout main
merge feature id: "M"
checkout release
commit id: "C"
cherry-pick id:"M" parent: "A"
`
)
.toThrow(
'Invalid operation: The specified parent commit is not an immediate parent of the cherry-picked commit.'
)
);
});

it('should throw error when try to branch existing branch: main', function () {
const str = `gitGraph
commit
Expand Down
12 changes: 10 additions & 2 deletions packages/mermaid/src/diagrams/git/parser/gitGraph.jison
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ branch(?=\s|$) return 'BRANCH';
"order:" return 'ORDER';
merge(?=\s|$) return 'MERGE';
cherry\-pick(?=\s|$) return 'CHERRY_PICK';
"parent:" return 'PARENT_COMMIT'
// "reset" return 'RESET';
checkout(?=\s|$) return 'CHECKOUT';
"LR" return 'DIR';
Expand Down Expand Up @@ -109,10 +110,17 @@ branchStatement

cherryPickStatement
: CHERRY_PICK COMMIT_ID STR {yy.cherryPick($3, '', undefined)}
| CHERRY_PICK COMMIT_ID STR PARENT_COMMIT STR {yy.cherryPick($3, '', undefined,$5)}
| CHERRY_PICK COMMIT_ID STR COMMIT_TAG STR {yy.cherryPick($3, '', $5)}
| CHERRY_PICK COMMIT_ID STR COMMIT_TAG EMPTYSTR {yy.cherryPick($3, '', '')}
| CHERRY_PICK COMMIT_ID STR PARENT_COMMIT STR COMMIT_TAG STR {yy.cherryPick($3, '', $7,$5)}
| CHERRY_PICK COMMIT_ID STR COMMIT_TAG STR PARENT_COMMIT STR {yy.cherryPick($3, '', $5,$7)}
nirname marked this conversation as resolved.
Show resolved Hide resolved
| CHERRY_PICK COMMIT_TAG STR COMMIT_ID STR {yy.cherryPick($5, '', $3)}
| CHERRY_PICK COMMIT_TAG EMPTYSTR COMMIT_ID STR {yy.cherryPick($3, '', '')}
| CHERRY_PICK COMMIT_TAG EMPTYSTR COMMIT_ID STR {yy.cherryPick($5, '', '')}
| CHERRY_PICK COMMIT_ID STR COMMIT_TAG EMPTYSTR {yy.cherryPick($3, '', '')}
| CHERRY_PICK COMMIT_ID STR PARENT_COMMIT STR COMMIT_TAG EMPTYSTR {yy.cherryPick($3, '', '',$5)}
| CHERRY_PICK COMMIT_ID STR COMMIT_TAG EMPTYSTR PARENT_COMMIT STR {yy.cherryPick($3, '', '',$7)}
| CHERRY_PICK COMMIT_TAG STR COMMIT_ID STR PARENT_COMMIT STR {yy.cherryPick($5, '', $3,$7)}
| CHERRY_PICK COMMIT_TAG EMPTYSTR COMMIT_ID STR PARENT_COMMIT STR{yy.cherryPick($5, '', '',$7)}
;

mergeStatement
Expand Down
31 changes: 18 additions & 13 deletions packages/mermaid/src/docs/syntax/gitgraph.md
Original file line number Diff line number Diff line change
Expand Up @@ -244,24 +244,29 @@ A few important rules to note here are:
1. You need to provide the `id` for an existing commit to be cherry-picked. If given commit id does not exist it will result in an error. For this, make use of the `commit id:$value` format of declaring commits. See the examples from above.
2. The given commit must not exist on the current branch. The cherry-picked commit must always be a different branch than the current branch.
3. Current branch must have at least one commit, before you can cherry-pick, otherwise it will cause an error is throw.
4. When cherry-picking a merge commit, providing a parent commit ID is mandatory. If the parent attribute is omitted or an invalid parent commit ID is provided, an error will be thrown.
5. The specified parent commit must be an immediate parent of the merge commit being cherry-picked.

Let see an example:

```mermaid-example
gitGraph
commit id: "ZERO"
branch develop
commit id:"A"
checkout main
commit id:"ONE"
checkout develop
commit id:"B"
checkout main
commit id:"TWO"
cherry-pick id:"A"
commit id:"THREE"
checkout develop
commit id:"C"
commit id: "ZERO"
branch develop
branch release
commit id:"A"
checkout main
commit id:"ONE"
checkout develop
commit id:"B"
checkout main
merge develop id:"MERGE"
commit id:"TWO"
checkout release
cherry-pick id:"MERGE" parent:"B"
commit id:"THREE"
checkout develop
commit id:"C"
RounakJoshi09 marked this conversation as resolved.
Show resolved Hide resolved
```

## Gitgraph specific configuration options
Expand Down
Loading