git fetch: add --verbose flag to show abandoned commit details

[sjawhar]

Summary

Closes #3081

Abandoned commits are now shown with their change IDs and descriptions when fetching, matching the jj abandon output format.

Example output:

bookmark: main@origin [updated] tracked
Abandoned 2 commits that are no longer reachable:
  qpvuntsm 12345678 local work that was rebased
  kkmpptxz 87654321 another local commit

Changes

• Abandoned commit details are now shown by default (limited to 10 commits)
• Uses print_updated_commits() with commit_summary_template() for consistent formatting
• When --quiet is specified, no abandoned commit information is shown

Test Plan

• All 40 git fetch tests pass
• Verified --quiet suppresses output
• Clippy passes

[google-cla]

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

[PhilipMetzger]

please remove the pluralization again, also I don't think that you can attribute the authorship to Claude since that will fail the CLA check.

Also the project has no AI policy yet, but it'd be helpful to know if you understand the code change you submitted.

[joyously]

What does it do when the global --quiet flag is also present?

[sjawhar]

I have reviewed the code but I'm new to this project, so I won't claim to know all the implications downstream. 😅

[sjawhar]

When --quiet is specified, no abandoned commit information is shown (neither the count nor the details). The code checks ui.status_formatter() which returns None in quiet mode.

[PhilipMetzger]

LG, thanks for your contribution. You need Yuya's approval though.

[sjawhar]

Done! Thanks for the quick review.

[sjawhar]

Sorry, still getting the hang of things 😅

[sjawhar]

Restored the detailed: bool parameter as requested. The signature is now:

pub fn print_git_import_stats(
    ui: &Ui,
    tx: Option<&WorkspaceCommandTransaction<'_>>,
    stats: &GitImportStats,
    detailed: bool,
)

Call sites are now clearer:

• print_git_import_stats(ui, Some(&tx), &stats, true)?; for detailed output
• print_git_import_stats(ui, None, &stats, false)?; for summary only

[sjawhar]

Oops, sorry, I clicked request re-review but haven't fully done a self-review yet. Sorry for all the back and forth, but thank you for helping me get my bearing. I love jj!!

[sjawhar]

Done! I removed the test.

I'm curious - how do you think about when a new test is needed vs. when existing test coverage is sufficient? I initially added the test because it felt like a "new behavior" that deserved explicit coverage, but I can see now that the existing tests already exercise the same code path and verify the output.

Would love any tips on developing better intuition for this - trying to make future contributions smoother! 😅

[martinvonz]

Nit: Please move (or copy) most of the PR description to the commit description so the information is available to anyone looking at the commit in the future (including if we move the repo off of GitHub)

[martinvonz]

I'm curious - how do you think about when a new test is needed vs. when existing test coverage is sufficient?

As Yuya said, it's not needed because it's covered by existing tests. That means that if we have a regression in this code, we will notice it because those tests will fail. However, it's important that the tests don't just happen to test this functionality, because then the test might change in the future to no longer test this code. In this case, there are several tests that cover, and in particular the test_git_fetch_removed_bookmark() seems likely to always involve abandoning some commits, so I'm not worried that we will lose the testing.