fix(compare): show the diff when a comparison has no commits - #39247
fix(compare): show the diff when a comparison has no commits#392474RH1T3CT0R7 wants to merge 2 commits into
Conversation
The compare page only rendered the diff box when there was at least one commit. A direct comparison whose head is an ancestor of its base, which is where the Compare link of a force-push comment leads after commits were dropped, has no commits but a non-empty diff, so the page showed "There are no differences to show." instead of the diff. Show the diff box whenever there are commits or a diff, keep the "no differences" message for when there is neither, and let the commits header read "0 Commits" in that case.
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
Assisted-by: Claude Code |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7c25968d80
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| // Direct compare of two commits where the head (add-csv) is an ancestor of the base (remove-files-b). | ||
| // This is what the "Compare" link of a force-push comment points to after the push has dropped | ||
| // commits: "base..head" lists no commits, but the diff is not empty and must still be shown. |
There was a problem hiding this comment.
Replace the narrative test comment
This three-line block narrates the scenario and the change rather than providing only a short, why-focused note; remove it or reduce it to a terse rationale to comply with the repository's explicit comment policy.
AGENTS.md reference: AGENTS.md:L10-L10
Useful? React with 👍 / 👎.
|
|
||
| // 'link_hi' and 'test.csv' are restored, 'test.txt' is deleted | ||
| inspectCompare(t, htmlDoc, 3, []string{"link_hi", "test.csv", "test.txt"}) | ||
| assert.NotContains(t, resp.Body.String(), translation.NewLocale("en-US").TrString("repo.commits.nothing_to_compare")) |
There was a problem hiding this comment.
Assert against the parsed document or saved response
After NewHTMLParser(t, resp.Body) reads the response recorder's bytes.Buffer through EOF, resp.Body.String() contains only the unread remainder and is empty. Consequently this assertion passes even if the page still renders the nothing_to_compare message, so inspect htmlDoc or save the response string before parsing.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Right, the parser had already drained the recorder. 6658822 asserts against the parsed document's text instead; with the template fix reverted this assertion now fails with the "no differences" message in the output, so it is no longer vacuous.
The compare page only rendered the diff box when the comparison had at least one commit, and otherwise printed "There are no differences to show." A direct comparison whose head is an ancestor of its base hits that case:
base..headlists no commits, yet the diff is not empty. This is where the Compare link on a force-push comment leads after the push dropped commits, so the link opened an empty page.The diff box is now shown whenever there are commits or a non-empty diff, and the "no differences" message only appears when there is neither. The commits table header, which was hidden together with the diff box before, reads "0 Commits" in that case. An integration test on the repo20 fixture covers it.
Fixes #36390