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.

Sign up for GitHub

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

Jump to bottom

Revisions (second try) #13313

Closed
wants to merge 7 commits into from
Closed

Revisions (second try) #13313

wants to merge 7 commits into from

Conversation

ghost
Copy link

@ghost ghost commented Oct 26, 2020 •
edited by ghost
Loading

Second try of: #12665

Fixes #12604
Fixes #5254
Fixes #12800

For the range-diff page to work the interdiff (part of the patchtools package) is required on the server.

@ghost ghost marked this pull request as draft October 26, 2020 11:44
@ghost ghost force-pushed the revisions branch 2 times, most recently from 0878ec7 to db73765 Compare October 26, 2020 12:59
@ghost
Copy link
Author

ghost commented Oct 26, 2020

Somebody could please add interdiff to the test environment? @lunny

@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Oct 26, 2020
@ghost ghost marked this pull request as ready for review October 26, 2020 13:15
@05storm26
Pull request revisions.
2193204
@05storm26
GetBranchName shouldn't return non-branchname refs like refs/pull/1/head
5107174
@05storm26
Fix file diff box header missalignment.
c26bb2d
@05storm26
Fix GetDiffShortStat. It now correctly returns the stat between two r…
3979e2a 
…evs.

When using the base...head pattern git diff works out the common merge
base and then compare the mergebase to the head revision. This results
in incorrect stats if the you are comparing a y shaped branches
e.g.: sets of revisions. For example on the pull requests view changes
tab when you select since revision, when the last revision just changed
the commit msg the returned content is correctly empty (because there
is no file content change) but the stat is not because the comparasion
is not made between the actual revision heads but their merge head.

Regural /compare/base...head comparisions which also use this function
are not affected by this change because they *themselves* work out the
common merge base and they use *that* to call this function. So they
were doing this anyway instead of relying on this function to compute
the common merge base.

I checked all paths (I think...) that leads to this function and all
calls to this seem to be correct after this change.
@05storm26
Fix the use of GetDiffShortStat.
abdd41c
@05storm26
Add "\ No newline at end of file" parsing. Show when newline is missi…
f088317 
…ng using symbols.

Fix section description being printed incorrectly. '@ -1,4...' instead
of '@@ -1,4...'. Added a test for this.
@05storm26
Create a page for comparing patchsets using 'git range-diff'. Fixes g…
fd43e72 
…o-gitea#12800

Also add warnings if a '/compare/base...head' is y shaped and offer to
show the raw difference (where the inverse of new commits on base are
included in the diff) and also offer to compare the y shaped junction
with range-diff.

The rangediff page uses the interdiff tool to work out the changes
between corresponding commits.
@lafriks
Copy link
Member

lafriks commented Oct 26, 2020

Can something like be reused/extended https://github.com/kordianbruck/interdiff to not introduce dependency on something that everybody would need to install as it does not come by default on most OS

@ghost
Copy link
Author

ghost commented Oct 26, 2020

@lafriks Maybe. I tried it with this:

diff1.txt:

diff --git a/asdf b/asdf
deleted file mode 100644
index 8bd6648..0000000
--- a/asdf
+++ /dev/null
@@ -1 +0,0 @@
-asdf
diff --git a/binder b/binder
new file mode 100644
index 0000000..048e138
--- /dev/null
+++ b/binder
@@ -0,0 +1 @@
+binder
diff --git a/uuu b/uuu
new file mode 100644
index 0000000..1802629
--- /dev/null
+++ b/uuu
@@ -0,0 +1,5 @@
+asdf2
+
+ttt
+
+

diff2.txt:

diff --git a/asdf b/asdf
deleted file mode 100644
index 8bd6648..0000000
--- a/asdf
+++ /dev/null
@@ -1 +0,0 @@
-asdf
diff --git a/binder b/binder
new file mode 100644
index 0000000..048e138
--- /dev/null
+++ b/binder
@@ -0,0 +1 @@
+binder
diff --git a/mmmm b/mmmm
new file mode 100644
index 0000000..1802629
--- /dev/null
+++ b/mmmm
@@ -0,0 +1,5 @@
+asdf
+
+ttt
+
+

Interdiff says:

interdiff diff1.txt diff2.txt:

reverted:
--- b/uuu
+++ /dev/null
@@ -1,5 +0,0 @@
-asdf2
-
-ttt
-
-
only in patch2:
unchanged:
--- /dev/null
+++ b/mmmm
@@ -0,0 +1,5 @@
+asdf
+
+ttt
+
+

Which is not bad.

That function says:

--- /dev/null
+++ /dev/null
--- b/binder
+++ b/binder
--- b/uuu
+++ b/mmmm
@@ -1,5 +1,5 @@
-asdf2
-
-ttt
-
-
+asdf
+
+ttt
+
+

@ghost
Copy link
Author

ghost commented Oct 27, 2020 •
edited by ghost
Loading

@lafriks I was playing with these go interdiff implementations and honestly implementing interdiff is probably easier than I initially thought. I will change this to use a go implementation of interdiff instead of the patchutils one. Until that is done I will put this PR back into draft mode.

@ghost ghost marked this pull request as draft October 27, 2020 14:36
@ghost
Copy link
Author

ghost commented Nov 2, 2020

Hm. The version here, works somewhat good: https://github.com/google/go-patchutils. But it still doesn't deal with extra lines created by git and the a/filename b/filename thing correctly. The repo requires the google contributor whatever to be signed to contribute so... not ideal.

@lafriks
Copy link
Member

lafriks commented Nov 4, 2020

As it is only single file we could move it into Gitea modules and change it to work as we need

@ghost
Copy link
Author

ghost commented Nov 4, 2020

@lafriks Yes. I am doing it.

I am changing it to track the diff --git extra lines like "rename from" and "rename to" and to output instead of a regural interdiff a diff --git format.

I have done some part of it but it is not yet ready.

@lafriks lafriks added type/changelog Adds the changelog for a new Gitea version type/feature Completely new functionality. Can only be merged if feature freeze is not active. labels Nov 5, 2020
@lafriks lafriks modified the milestones: 1.12.6, 1.14.0 Nov 5, 2020
@lunny lunny modified the milestones: 1.14.0, 1.15.0 Jan 27, 2021
@techknowlogick techknowlogick modified the milestones: 1.15.0, 1.16.0 Jun 15, 2021
@lunny lunny modified the milestones: 1.16.0, 1.17.0 Nov 9, 2021
@lunny lunny removed this from the 1.17.0 milestone Apr 14, 2022
@lunny lunny mentioned this pull request Dec 19, 2022
Closed
@lunny
Copy link
Member

lunny commented Mar 27, 2023

Closed as the author disappeared.

@lunny lunny closed this Mar 27, 2023
@lunny lunny mentioned this pull request Jul 14, 2023
Open
@go-gitea go-gitea locked as resolved and limited conversation to collaborators Aug 1, 2023
@delvh delvh removed the type/changelog Adds the changelog for a new Gitea version label Oct 7, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers
No reviews
Assignees
No one assigned
Labels
lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. type/feature Completely new functionality. Can only be merged if feature freeze is not active.
Projects
None yet
Milestone
No milestone
6 participants
@lafriks @lunny @techknowlogick @GiteaBot @delvh @05storm26