-
Notifications
You must be signed in to change notification settings - Fork 825
simplify status base line #12290
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
base: master
Are you sure you want to change the base?
simplify status base line #12290
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR updates the but status output to simplify the merge-base (“status base”) line by removing extra annotations, and refreshes CLI snapshot/test expectations accordingly.
Changes:
- Removes the
(common base)label from the merge-base line in human-readable status output. - Stops appending the “(checked …)” suffix to the merge-base line.
- Adjusts merge-base commit message formatting (first line only, shorter truncation), updating snapshots and JSON test expectations.
Reviewed changes
Copilot reviewed 3 out of 9 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| crates/but/src/command/legacy/status/mod.rs | Removes “(common base)” and the merge-base “checked …” suffix; changes merge-base commit message extraction/truncation. |
| crates/but/tests/but/command/status.rs | Updates expected JSON message value for merge-base commit output. |
| crates/but/tests/but/journey.rs | Updates journey test expected status output to match simplified merge-base line. |
| crates/but/tests/but/command/absorb.rs | Updates expected status output in absorb command snapshots to match simplified merge-base line. |
| crates/but/tests/but/snapshots/from-workspace/status01-verbose.stdout.term.svg | Updates terminal SVG snapshot for simplified merge-base line. |
| crates/but/tests/but/command/snapshots/status/two-worktrees/status-with-worktrees.stdout.term.svg | Updates terminal SVG snapshot for simplified merge-base line. |
| crates/but/tests/but/command/snapshots/status/two-worktrees/status-with-worktrees-verbose.stdout.term.svg | Updates terminal SVG snapshot for simplified merge-base line. |
| crates/but/tests/but/command/snapshots/status/remote-and-local-files.stdout.term.svg | Updates terminal SVG snapshot for simplified merge-base line. |
| crates/but/tests/but/command/snapshots/status/long-cli-ids.stdout.term.svg | Updates terminal SVG snapshot for simplified merge-base line. |
| let message = base_commit_decoded | ||
| .message | ||
| .to_string() | ||
| .replace('\n', " ") | ||
| .lines() | ||
| .next() | ||
| .unwrap_or("") | ||
| .chars() | ||
| .take(50) | ||
| .take(40) | ||
| .collect::<String>(); |
Copilot
AI
Feb 8, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
CommonMergeBase.message is used for both human output and the JSON merge_base.message (via build_workspace_status_json). Changing the extraction to lines().next() and truncating to 40 chars alters the JSON output semantics (and reduces information vs the prior 50-char, newline-flattened version). Consider storing an unmodified/full commit message in CommonMergeBase (or at least keep the previous 50-char behavior for JSON) and applying truncation/newline normalization only when rendering the human CLI line.
| writeln!( | ||
| out, | ||
| "{} {} (common base) [{}] {} {}{}", | ||
| "{} {} [{}] {} {}", | ||
| if upstream_state.is_some() { "├╯" } else { "┴" }, | ||
| common_merge_base_data.common_merge_base.dimmed(), | ||
| common_merge_base_data.target_name.green().bold(), | ||
| common_merge_base_data.commit_date.dimmed(), | ||
| common_merge_base_data.message, | ||
| if upstream_state.is_none() { | ||
| last_checked_text.dimmed().to_string() | ||
| } else { | ||
| String::new() | ||
| } | ||
| )?; |
Copilot
AI
Feb 8, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The PR description says to remove the "checked …" string, but this change only removes it from the merge-base line. last_checked_text is still appended to upstream output in this module (e.g., upstream summary/detailed sections). Either remove it there as well for consistency with the description, or adjust the PR description/scope to clarify it’s only the merge-base line.
Remove '(common base)' label, truncate commit message to 40 chars from first line only, and move last-checked timestamp to only appear on the upstream line when there are upstream commits.
Keep the unmodified commit message in CommonMergeBase so JSON output (via build_workspace_status_json) retains the original content. Previously the code normalized and truncated messages before storing them in CommonMergeBase, which changed JSON semantics and lost information. This change stores full_message in CommonMergeBase and applies newline normalization and a 40-char truncation only when rendering the human CLI line.
b3deec6 to
9c15875
Compare
remove the “checked ago…” and “(common base)” strings