feat(lanes): make history-diff args optional and remove EXPERIMENTAL marks#10192
feat(lanes): make history-diff args optional and remove EXPERIMENTAL marks#10192davidfirst merged 7 commits intomasterfrom
Conversation
…marks Allow `bit lane history-diff` to work with 0, 1, or 2 arguments: - no args: diff latest history entry against its predecessor - one arg: diff the given entry against its predecessor - two args: explicit from/to (backward compatible) Also removes EXPERIMENTAL from all lane history subcommands.
There was a problem hiding this comment.
Pull request overview
Updates the bit lane history-diff CLI to support 0/1/2 positional arguments (defaulting to “latest vs predecessor” when omitted) and removes “EXPERIMENTAL” labeling from lane-history-related command descriptions.
Changes:
- Add
LaneHistory.getHistoryIds()helper for consumers needing the list of history entry IDs. - Update
bit lane history-diffto accept optional positional args and infer from/to IDs when omitted. - Remove
EXPERIMENTAL.prefix fromlane history,lane checkout, andlane revertcommand descriptions.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| scopes/scope/objects/models/lane-history.ts | Adds a helper to retrieve lane history entry IDs. |
| scopes/lanes/lanes/lane.cmd.ts | Updates CLI descriptions to remove “EXPERIMENTAL” labeling. |
| scopes/lanes/diff/lane-history-diff.cmd.ts | Implements optional argument behavior and updates help text for lane history-diff. |
|
| it('with no args, should diff the latest entry against its predecessor', () => { | ||
| const output = helper.command.runCmd('bit lane history-diff'); | ||
| expect(output).to.have.string('comp1'); | ||
| expect(output).to.have.string('comp2'); | ||
| }); | ||
|
|
||
| it('with one arg (latest id), should diff that entry against its predecessor', () => { | ||
| const latestId = historyEntries[historyEntries.length - 1].id; | ||
| const output = helper.command.runCmd(`bit lane history-diff ${latestId}`); | ||
| expect(output).to.have.string('comp1'); | ||
| expect(output).to.have.string('comp2'); | ||
| }); | ||
|
|
||
| it('with one arg (first id), should throw since it has no predecessor', () => { | ||
| const firstId = historyEntries[0].id; | ||
| expect(() => helper.command.runCmd(`bit lane history-diff ${firstId}`)).to.throw('unable to find a predecessor'); | ||
| }); |
There was a problem hiding this comment.
The new e2e suite covers the 0-arg and 1-arg forms, but it doesn’t cover the 2-argument behavior (bit lane history-diff <from> <to>). Please add a test that passes two explicit history IDs (e.g., first snap id -> second snap id) and asserts the diff output, so the new CLI contract is fully exercised.
| name = 'history [lane-name]'; | ||
| description = 'EXPERIMENTAL. show lane history, default to the current lane'; | ||
| description = 'show lane history, default to the current lane'; | ||
| extendedDescription = `list from the oldest to the newest history items`; | ||
| alias = ''; |
There was a problem hiding this comment.
LaneHistoryCmd claims it lists history items “from the oldest to the newest”, but the implementation still iterates with Object.keys(history), which is not guaranteed to be chronological after LaneHistory.merge() (spread merge can change insertion order). To make the help/behavior match the PR’s “sorted by date” intent, build the list using laneHistory.getHistoryIds() (or explicitly sort by item.log.date) before formatting output/JSON.
Allow
bit lane history-diffto work with 0, 1, or 2 arguments:Also sorts history IDs by date for reliable chronological ordering after merges, and removes EXPERIMENTAL from all lane history subcommands (
history,history-diff,history-checkout,history-revert).