Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
github-actions
bot
added
rust
There was a problem hiding this comment.
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>(); |
There was a problem hiding this comment.
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() | ||
| } | ||
| )?; |
There was a problem hiding this comment.
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.
schacon
force-pushed
the
simplify-status-base-line
branch
from
b3deec6 to
9c15875
Compare
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.
schacon
force-pushed
the
simplify-status-base-line
branch
from
9c15875 to
d99a8b4
Compare
| writeln!( | ||
| out, | ||
| "{} {} (common base) [{}] {} {}{}", | ||
| "{} {} [{}] {} {}", |
There was a problem hiding this comment.
This output format change will affect other status snapshot fixtures beyond the ones updated in this PR. A repo-wide search still finds status snapshots containing the old "(common base)"/trailing-space baseline (e.g. crates/but/tests/but/command/snapshots/status/long-file-cli-ids-are-aligned.stdout.term.svg, crates/but/tests/but/snapshots/from-workspace/status01.stdout.term.svg), which will likely fail tests unless updated.
| "{} {} [{}] {} {}", | |
| "{} {} (common base) [{}] {} {} ", |
| writeln!( | ||
| out, | ||
| "{} {} (common base) [{}] {} {}{}", | ||
| "{} {} [{}] {} {}", | ||
| if upstream_state.is_some() { "├╯" } else { "┴" }, | ||
| common_merge_base_data.common_merge_base.dimmed(), |
There was a problem hiding this comment.
PR description says to remove the "checked ago…" string, but the (checked …) text is still included in upstream status output via last_checked_text (only removed from the base line here). Either remove it from the upstream output as well, or clarify the PR description/scope.
1 participant
remove the “checked ago…” and “(common base)” strings