fix(pulls): respect diff.orderFile in diff file tree (#38566) (#38578)

Backport #38566 by @eliroca

Co-authored-by: Elisei Roca <eroca@suse.de>
Co-authored-by: wxiaoguang <wxiaoguang@gmail.com>

modules/git/config.go

@@ -91,7 +91,7 @@ func syncGitConfig(ctx context.Context) (err error) {
 		}
 	}
 
-	// By default partial clones are disabled, enable them from git v2.22
+	// By default, partial clones are disabled, enable them from git v2.22
 	if !setting.Git.DisablePartialClone && DefaultFeatures().CheckVersionAtLeast("2.22") {
 		if err = configSet(ctx, "uploadpack.allowfilter", "true"); err != nil {
 			return err
@@ -114,9 +114,23 @@ func syncGitConfig(ctx context.Context) (err error) {
 		}
 	}
 
+	GlobalConfig = &GlobalConfigStruct{}
+	// HINT: GIT-DIFF-TREE-UI-CONFIG: Git's bug: git-diff-tree loads config with /* no "diff" UI options */ (since 20 years ago).
+	// https://github.com/git/git/blame/5d2e7709234afea1b6ddb25cd4f60d3d5fb3c200/builtin/diff-tree.c#L127
+	// Although document and manual say that "git-diff-tree" supports "diff.orderfile" option, but it is not actually supported.
+	// So we need to apply the diff.orderfile explicitly in our code.
+	GlobalConfig.DiffOrderFile, _ = configGet(ctx, "diff.orderfile")
 	return nil
 }
 
+func configGet(ctx context.Context, key string) (string, error) {
+	stdout, _, err := gitcmd.NewCommand("config", "--global", "--get").AddDynamicArguments(key).RunStdString(ctx)
+	if err != nil && !gitcmd.IsErrorExitCode(err, 1) {
+		return "", fmt.Errorf("failed to get git config %s, err: %w", key, err)
+	}
+	return strings.TrimRight(stdout, "\r\n"), nil
+}
+
 func configSet(ctx context.Context, key, value string) error {
 	stdout, _, err := gitcmd.NewCommand("config", "--global", "--get").
 		AddDynamicArguments(key).

modules/git/git.go

@@ -36,7 +36,14 @@ type Features struct {
 	SupportGitMergeTree        bool           // >= 2.40 // we also need "--merge-base"
 }
 
-var defaultFeatures *Features
+type GlobalConfigStruct struct {
+	DiffOrderFile string
+}
+
+var (
+	defaultFeatures *Features
+	GlobalConfig    *GlobalConfigStruct
+)
 
 func (f *Features) CheckVersionAtLeast(atLeast string) bool {
 	return f.gitVersion.Compare(version.Must(version.NewVersion(atLeast))) >= 0

services/gitdiff/git_diff_tree.go

@@ -60,6 +60,11 @@ func runGitDiffTree(ctx context.Context, gitRepo *git.Repository, useMergeBase b
 	cmd := gitcmd.NewCommand("diff-tree", "--raw", "-r", "--root").
 		AddOptionFormat("--find-renames=%s", setting.Git.DiffRenameSimilarityThreshold)
 
+	// HINT: GIT-DIFF-TREE-UI-CONFIG: apply the diff.orderfile explicitly
+	if git.GlobalConfig.DiffOrderFile != "" {
+		cmd.AddOptionFormat("-O%s", git.GlobalConfig.DiffOrderFile)
+	}
+
 	if useMergeBase {
 		cmd.AddArguments("--merge-base")
 	}

services/gitdiff/git_diff_tree_test.go

@@ -4,10 +4,13 @@
 package gitdiff
 
 import (
+	"os"
+	"path/filepath"
 	"strings"
 	"testing"
 
 	"gitea.dev/modules/git"
+	"gitea.dev/modules/git/gitcmd"
 
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
@@ -217,6 +220,43 @@ func TestGitDiffTree(t *testing.T) {
 	}
 }
 
+func TestGitDiffTreeRespectsDiffOrderFile(t *testing.T) {
+	gitRepo, err := git.OpenRepository(t.Context(), "../../modules/git/tests/repos/repo5_pulls")
+	require.NoError(t, err)
+	defer gitRepo.Close()
+
+	testDiffTree := func(t *testing.T) (filePaths []string) {
+		t.Helper()
+		diffTree, err := GetDiffTree(t.Context(), gitRepo, false, "72866af952e98d02a73003501836074b286a78f6", "d8e0bbb45f200e67d9a784ce55bd90821af45ebd")
+		require.NoError(t, err)
+		for _, f := range diffTree.Files {
+			filePaths = append(filePaths, f.HeadPath)
+		}
+		return filePaths
+	}
+
+	t.Run("NoDiffOrderFile", func(t *testing.T) {
+		assert.Equal(t, []string{"LICENSE", "README.md"}, testDiffTree(t))
+	})
+
+	t.Run("GlobalDiffOrderFile", func(t *testing.T) {
+		diffOrderFilePath := filepath.Join(t.TempDir(), "test-diff-order.txt")
+		err = os.WriteFile(diffOrderFilePath, []byte("README.md\nLICENSE\n"), 0o644)
+		require.NoError(t, err)
+
+		_, _, err = gitcmd.NewCommand("config", "set", "--global").AddDynamicArguments("diff.orderFile", diffOrderFilePath).RunStdString(t.Context())
+		require.NoError(t, err)
+		require.NoError(t, git.InitFull())
+		defer func() {
+			_, _, err = gitcmd.NewCommand("config", "unset", "--global").AddDynamicArguments("diff.orderFile").RunStdString(t.Context())
+			require.NoError(t, err)
+			require.NoError(t, git.InitFull())
+		}()
+
+		assert.Equal(t, []string{"README.md", "LICENSE"}, testDiffTree(t))
+	})
+}
+
 func TestParseGitDiffTree(t *testing.T) {
 	test := []struct {
 		Name      string

templates/repo/diff/stats.tmpl

@@ -6,8 +6,8 @@
 {{if or .Addition .Deletion}}
 <div class="flex-text-block tw-flex-shrink-0 tw-text-[13px] {{if .Classes}}{{.Classes}}{{end}}">
 	<span>
-		{{if .Addition}}<span class="tw-text-diff-added-fg">+{{.Addition}}</span>{{end}}
-		{{if .Deletion}}<span class="tw-text-diff-removed-fg">-{{.Deletion}}</span>{{end}}
+		{{if .Addition}}<strong class="tw-text-diff-added-fg">+{{.Addition}}</strong>{{end}}
+		{{if .Deletion}}<strong class="tw-text-diff-removed-fg">-{{.Deletion}}</strong>{{end}}
 	</span>
 	<span class="diff-stats-bar" data-tooltip-content="{{ctx.Locale.Tr "repo.diff.stats_desc_file" (Eval .Addition "+" .Deletion) .Addition .Deletion}}">
 		{{/* if the denominator is zero, then the float result is "width: NaNpx", as before, it just works */}}

templates/repo/issue/filter_actions.tmpl

@@ -44,7 +44,7 @@
 			</span>
 			{{svg "octicon-triangle-down" 14 "dropdown icon"}}
 			<div class="menu">
-				<div class="item issue-action" data-element-id="0" data-url="{{$.Link}}/milestone">
+				<div class="item issue-action" data-element-id="" data-url="{{$.Link}}/milestone">
 				{{ctx.Locale.Tr "repo.issues.action_milestone_no_select"}}
 				</div>
 				{{if .OpenMilestones}}
@@ -75,7 +75,7 @@
 			</span>
 			{{svg "octicon-triangle-down" 14 "dropdown icon"}}
 			<div class="menu">
-				<div class="item issue-action" data-element-id="0" data-url="{{$.Link}}/projects">
+				<div class="item issue-action" data-element-id="" data-url="{{$.Link}}/projects">
 				{{ctx.Locale.Tr "repo.issues.new.clear_projects"}}
 				</div>
 				{{if .OpenProjects}}
@@ -113,9 +113,6 @@
 				<div class="item issue-action" data-action="clear" data-url="{{$.Link}}/assignee">
 					{{ctx.Locale.Tr "repo.issues.new.clear_assignees"}}
 				</div>
-				<div class="item issue-action" data-element-id="0" data-url="{{$.Link}}/assignee">
-					{{ctx.Locale.Tr "repo.issues.action_assignee_no_select"}}
-				</div>
 				{{range .Assignees}}
 					<div class="item issue-action" data-element-id="{{.ID}}" data-url="{{$.RepoLink}}/issues/assignee">
 						{{ctx.AvatarUtils.Avatar . 20}} {{.GetDisplayName}}

web_src/css/themes/theme-gitea-dark.css

@@ -163,14 +163,14 @@ gitea-theme-meta-info {
   --color-grey-light: #898d96;
   --color-gold: #b1983b;
   --color-white: #ffffff;
-  --color-diff-added-fg: #87ab63;
+  --color-diff-added-fg: #93b373;
   --color-diff-added-linenum-bg: #274227;
   --color-diff-added-row-bg: #203224;
   --color-diff-added-row-border: #314a37;
   --color-diff-added-word-bg: #3c653c;
   --color-diff-moved-row-bg: #818044;
   --color-diff-moved-row-border: #bcca6f;
-  --color-diff-removed-fg: #cc4848;
+  --color-diff-removed-fg: #fb5f5b;
   --color-diff-removed-linenum-bg: #482121;
   --color-diff-removed-row-bg: #301e1e;
   --color-diff-removed-row-border: #634343;
@@ -270,7 +270,7 @@ gitea-theme-meta-info {
   --color-syntax-keyword: #ff8854;
   --color-syntax-bool: #25bbc9;
   --color-syntax-control: #dd9e17;
-  --color-syntax-name: #c7a618;
+  --color-syntax-name: #fabd2f;
   --color-syntax-type: #eb8cb3;
   --color-syntax-number: #63b2dd;
   --color-syntax-operator: #ff8854;

web_src/css/themes/theme-gitea-light-protanopia-deuteranopia.css

@@ -8,11 +8,11 @@ gitea-theme-meta-info {
 
 /* red/green colorblind-friendly colors */
 :root {
-  --color-diff-added-fg: #2185d0;
+  --color-diff-added-fg: #0860cc;
   --color-diff-added-linenum-bg: #b6e3ff;
   --color-diff-added-row-bg: #ddf4ff;
   --color-diff-added-word-bg: #b6e3ff;
-  --color-diff-removed-fg: #fc6500;
+  --color-diff-removed-fg: #a84400;
   --color-diff-removed-linenum-bg: #ffd8b5;
   --color-diff-removed-row-bg: #fff1e5;
   --color-diff-removed-word-bg: #ffd8b5;

web_src/css/themes/theme-gitea-light-tritanopia.css

@@ -8,7 +8,7 @@ gitea-theme-meta-info {
 
 /* blue/yellow colorblind-friendly colors */
 :root {
-  --color-diff-added-fg: #2185d0;
+  --color-diff-added-fg: #0860cc;
   --color-diff-added-linenum-bg: #b6e3ff;
   --color-diff-added-row-bg: #ddf4ff;
   --color-diff-added-word-bg: #b6e3ff;

web_src/css/themes/theme-gitea-light.css

@@ -163,14 +163,14 @@ gitea-theme-meta-info {
   --color-grey-light: #7c838a;
   --color-gold: #a1882b;
   --color-white: #ffffff;
-  --color-diff-added-fg: #21ba45;
+  --color-diff-added-fg: #177231;
   --color-diff-added-linenum-bg: #d1f8d9;
   --color-diff-added-row-bg: #e6ffed;
   --color-diff-added-row-border: #e6ffed;
   --color-diff-added-word-bg: #acf2bd;
   --color-diff-moved-row-bg: #f1f8d1;
   --color-diff-moved-row-border: #d0e27f;
-  --color-diff-removed-fg: #db2828;
+  --color-diff-removed-fg: #c61f2b;
   --color-diff-removed-linenum-bg: #ffcecb;
   --color-diff-removed-row-bg: #ffeef0;
   --color-diff-removed-row-border: #f1c0c0;

web_src/js/features/repo-issue-list.ts

@@ -57,18 +57,12 @@ function initRepoIssueListCheckboxes() {
 
       const url = el.getAttribute('data-url')!;
       let action = el.getAttribute('data-action')!;
-      let elementId = el.getAttribute('data-element-id')!;
+      const elementId = el.getAttribute('data-element-id')!;
       const issueIDList: string[] = Array.from(document.querySelectorAll('.issue-checkbox:checked'), (el) => (el.getAttribute('data-issue-id')!));
       const issueIDs = issueIDList.join(',');
       if (!issueIDs) return;
 
-      // for assignee
-      if (elementId === '0' && url.endsWith('/assignee')) {
-        elementId = '';
-        action = 'clear';
-      }
-
-      // for toggle
+      // for label toggle
       if (action === 'toggle' && e.altKey) {
         action = 'toggle-alt';
       }