fix: map kebab-case first-parent arg to camelCase for git-client v5 (#4704)#4706
Conversation
…onventional-changelog#4704) minimist parses `--first-parent` into `{ 'first-parent': true }`, but @conventional-changelog/git-client expects camelCase (`firstParent`) as of conventional-changelog/conventional-changelog#1450 Also strips the `_` array that minimist always produces, preventing it from being passed through to the git client.
Review Summary by QodoMap kebab-case first-parent to camelCase for git-client v5
WalkthroughsDescription• Maps kebab-case first-parent argument to camelCase firstParent • Removes minimist's _ array from git client options • Ensures compatibility with @conventional-changelog/git-client v5 Diagramflowchart LR
A["minimist parses --first-parent"] -->|"kebab-case: first-parent"| B["Destructure and map"]
B -->|"Remove _ array"| C["Extract first-parent"]
C -->|"Convert to camelCase"| D["firstParent for git-client"]
File Changes1. @commitlint/read/src/read.ts
|
Code Review by Qodo
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
| const { _, ...parsedArgs } = minimist(gitLogArgs.split(" ")); | ||
| gitOptions = { | ||
| ...minimist(gitLogArgs.split(" ")), | ||
| ...parsedArgs, | ||
| firstParent: parsedArgs["first-parent"], | ||
| from, |
There was a problem hiding this comment.
1. Firstparent overwritten to undefined 🐞 Bug ≡ Correctness
In getCommitMessages(), firstParent is always assigned from parsedArgs["first-parent"] after spreading parsedArgs, which overwrites any parsedArgs.firstParent value (e.g., from --firstParent) with undefined. This makes --git-log-args parsing silently drop a valid firstParent setting and always injects a firstParent property when gitLogArgs is set.
Agent Prompt
### Issue description
`firstParent` is always overwritten with `parsedArgs["first-parent"]`, which clobbers an existing `parsedArgs.firstParent` value and also adds `firstParent: undefined` whenever `gitLogArgs` is provided.
### Issue Context
`--git-log-args` is an arbitrary space-separated string. The code should map kebab-case `first-parent` to camelCase `firstParent` **only when present**, and should not overwrite an existing `firstParent` value.
### Fix Focus Areas
- @commitlint/read/src/read.ts[72-81]
### Suggested approach
- Extract `first-parent` via object rest (`const { ['first-parent']: firstParentKebab, ...rest } = parsedArgs`) so the kebab key is removed from the spread.
- Build `gitOptions` with:
- `...rest`
- `firstParent` set **only if** `firstParentKebab !== undefined`, otherwise preserve any existing `rest.firstParent`.
- `from`/`to` last as today.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
There was a problem hiding this comment.
Pull request overview
Fixes commitlint’s --git-log-args="--first-parent" behavior by ensuring options parsed from minimist are compatible with @conventional-changelog/git-client v5 (camelCase firstParent) and by preventing minimist’s _ array from being spread into git options.
Changes:
- Parse
gitLogArgswithminimistand exclude the_positional-args array from the options spread. - Map
--first-parent(kebab-case) tofirstParent(camelCase) when building git options.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| let gitOptions: GitOptions = { from, to }; | ||
| if (gitLogArgs) { | ||
| const { _, ...parsedArgs } = minimist(gitLogArgs.split(" ")); |
There was a problem hiding this comment.
const { _, ...parsedArgs } = … introduces an unused local (_). With noUnusedLocals: true in the repo TS config, this will fail type-check/build unless _ is used or removed. Consider deleting ._ from the minimist result instead of destructuring it, or explicitly marking the binding as used (e.g. void _) so the compiler doesn’t flag it.
| const { _, ...parsedArgs } = minimist(gitLogArgs.split(" ")); | |
| const parsedArgs = minimist(gitLogArgs.split(" ")); | |
| delete parsedArgs._; |
| firstParent: parsedArgs["first-parent"], | ||
| from, | ||
| to, | ||
| }; |
There was a problem hiding this comment.
firstParent is always set from parsedArgs["first-parent"]. If the user passes the already-camel-cased --firstParent, minimist will set parsedArgs.firstParent = true but parsedArgs["first-parent"] will be undefined, and this code will overwrite firstParent to undefined in gitOptions. Update the mapping so firstParent is only derived from the kebab-case key when it’s actually present, and avoid clobbering an existing firstParent value.
| firstParent: parsedArgs["first-parent"], | |
| from, | |
| to, | |
| }; | |
| from, | |
| to, | |
| }; | |
| if (Object.prototype.hasOwnProperty.call(parsedArgs, "first-parent")) { | |
| gitOptions.firstParent = parsedArgs["first-parent"]; | |
| } |
| gitOptions = { | ||
| ...minimist(gitLogArgs.split(" ")), | ||
| ...parsedArgs, | ||
| firstParent: parsedArgs["first-parent"], |
There was a problem hiding this comment.
After mapping "first-parent" to firstParent, parsedArgs still contains the original "first-parent" key and it will be spread into gitOptions. Since git-raw-commits/git-client v5 dropped support for arbitrary git log args, it’s better to remove the kebab-case key after mapping to avoid passing unknown options downstream.
| if (gitLogArgs) { | ||
| const { _, ...parsedArgs } = minimist(gitLogArgs.split(" ")); | ||
| gitOptions = { | ||
| ...minimist(gitLogArgs.split(" ")), | ||
| ...parsedArgs, | ||
| firstParent: parsedArgs["first-parent"], | ||
| from, | ||
| to, | ||
| }; |
There was a problem hiding this comment.
This change adds new behavior for gitLogArgs parsing (dropping minimist’s _ and mapping --first-parent to firstParent), but @commitlint/read/src/read.test.ts currently only covers --skip. Add a test case that verifies --git-log-args='--first-parent' results in first-parent history only (or at least that the correct option is passed through), to prevent regressions like #4704.
|
Thanks! Please have a look at the AI feedback, tackle if valid. |
Description
This PR was made to fix #4704
When
--first-parentis passed via--git-log-args, minimist parses it into{ 'first-parent': true, _: [] }. However,@conventional-changelog/git-clientv5 expects the camelCase keyfirstParent. This change:_array that minimist always adds, so it isn't spread into the git client options.first-parentkey to the camelCasefirstParentkey expected by the git client.Motivation and Context
@conventional-changelog/git-clientre-addedfirstParentsupport in conventional-changelog/conventional-changelog#1450, but it expects a camelCase key. Since minimist parses--first-parentinto kebab-case ({ 'first-parent': true }), commitlint needs to map it tofirstParentfor the git client to recognise it.Usage examples
How Has This Been Tested?
--first-parentis correctly mapped tofirstParentand passed through to the git client._array from minimist is no longer spread into git options.Types of changes
Checklist: