Skip to content
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

Fix CSV diff panic if the file was deleted from both branches #22947

Closed
wants to merge 1 commit into from

Conversation

wxiaoguang
Copy link
Contributor

@wxiaoguang wxiaoguang commented Feb 17, 2023

Close #22946

If a CSV file was deleted from both branches, then baseReader and headReader are nil, no need to "diff" them.
The UI will show a "deleted" change correctly.

ps: This is a quick fix. If you want to see the "deleted content", it needs more work to fine tune. I think it's good enough for a fix, since the file has been deleted from both side already.

image

@lunny lunny added the type/bug label Feb 17, 2023
@lunny lunny added this to the 1.19.0 milestone Feb 17, 2023
@lunny lunny added the outdated/backport/v1.18 This PR should be backported to Gitea 1.18 label Feb 17, 2023
@lunny lunny requested a review from KN4CK3R February 17, 2023 03:54
@GiteaBot GiteaBot added the lgtm/need 1 This PR needs approval from one additional maintainer to be merged. label Feb 17, 2023
Copy link
Contributor

@kdumontnu kdumontnu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See: #22949

@wxiaoguang
Copy link
Contributor Author

Thank you, I think the complete fix is better. Close this one.

@wxiaoguang wxiaoguang closed this Feb 17, 2023
@wxiaoguang wxiaoguang deleted the fix-csv-diff branch February 17, 2023 04:07
@lunny lunny removed this from the 1.19.0 milestone Feb 17, 2023
lunny pushed a commit that referenced this pull request Feb 20, 2023
Replaces: #22947
Fixes #22946
Probably related to #19530

Basically, many of the diffs were broken because they were comparing to
the base commit, where a 3-dot diff should be comparing to the [last
common
ancestor](https://matthew-brett.github.io/pydagogue/git_diff_dots.html).

This should have an integration test so that we don’t run into this
issue again.

---------

Co-authored-by: Jonathan Tran <jonnytran@gmail.com>
@delvh delvh removed the outdated/backport/v1.18 This PR should be backported to Gitea 1.18 label Feb 21, 2023
@go-gitea go-gitea locked and limited conversation to collaborators May 3, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
lgtm/need 1 This PR needs approval from one additional maintainer to be merged. type/bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Diff breaks if csv deleted from base and head
5 participants