Skip to content

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