[UI] hide refs/pull/*/head branchs on Commit graph

[a1012112796]

• Gitea version (or commit ref):
• Git version:
• Operating system:
• Database (use [x]):
  • PostgreSQL
  • MySQL
  • MSSQL
  • SQLite
• Can you reproduce the bug at https://try.gitea.io:
  • Yes (provide example URL)
  • No
  • Not relevant
• Log gist:

Description

As title

[lunny]

The file will still be used when you review the pull request. So I don't think it should be deleted. But we should hide them from commits graph.

[lunny]

I have said above. Once refs/pull/10/head deleted, the pull request view will be broken. So it cannot be deleted.

[a1012112796]

sorry. now I see, you are right. I will change the title.

[worthy7]

I just wanted to come and bump this up, I only just found the graph, I wanted a way to show collegues and this looks great, but all the deleted branch information is not needed (that's why it was squash merged).

Anyway, vote to hide them.

[zeripath]

Ideally we need another toggle [Show PRs] and a way of selecting branches/prs to show the commits from. As ever the UI is the most difficult bit here. The backend code changes would be very simple, (see attached patch.)

Probably want something like:

https://fomantic-ui.com/modules/dropdown.html#clearable-selection

coupled with a remote search functionality:

https://fomantic-ui.com/modules/dropdown.html#return-all-choices-remotely

The below is a patch that would add exclude-pr-refs and (possibly many) branch parameters to be passed to the commitgraph endpoint to allow for graphs to be selected between refs. It actually allows for things like A...B and A..B - there's a small amount of sanity check to prevent branches with '--' prefixes causing trouble and unknown branches will just drop to all. There's also the option of passing in file parameters to test against particular files.

Patch

From 2cbec04ae646ebbc98130d7a1e881a7ad97da6b3 Mon Sep 17 00:00:00 2001
From: Andrew Thornton <art27@cantab.net>
Date: Mon, 7 Sep 2020 11:04:11 +0100
Subject: [PATCH] Add backend support for excluding PRs, selecting branches and
 files.

Signed-off-by: Andrew Thornton <art27@cantab.net>
---
 modules/context/pagination.go  |  6 ++++++
 modules/context/repo.go        | 12 ++++++++++++
 modules/git/commit.go          | 22 +++++++++++++++++-----
 modules/git/repo.go            |  2 +-
 modules/git/repo_commit.go     |  6 +++---
 modules/gitgraph/graph.go      | 33 ++++++++++++++++++++++++++-------
 modules/gitgraph/graph_test.go |  2 +-
 routers/repo/commit.go         | 34 +++++++++++++++++++++++++++++-----
 8 files changed, 95 insertions(+), 22 deletions(-)

diff --git a/modules/context/pagination.go b/modules/context/pagination.go
index 9a6ad0b5c..a6638f408 100644
--- a/modules/context/pagination.go
+++ b/modules/context/pagination.go
@@ -37,6 +37,12 @@ func (p *Pagination) AddParam(ctx *Context, paramKey string, ctxKey string) {
 	p.urlParams = append(p.urlParams, urlParam)
 }
 
+// AddParamString adds a string parameter directly
+func (p *Pagination) AddParamString(key string, value string) {
+	urlParam := fmt.Sprintf("%s=%v", url.QueryEscape(key), url.QueryEscape(value))
+	p.urlParams = append(p.urlParams, urlParam)
+}
+
 // GetParams returns the configured URL params
 func (p *Pagination) GetParams() template.URL {
 	return template.URL(strings.Join(p.urlParams, "&"))
diff --git a/modules/context/repo.go b/modules/context/repo.go
index 4aac0c05a..03b19711a 100644
--- a/modules/context/repo.go
+++ b/modules/context/repo.go
@@ -157,6 +157,18 @@ func (r *Repository) GetCommitsCount() (int64, error) {
 	})
 }
 
+// GetCommitGraphsCount returns cached commit count for current view
+func (r *Repository) GetCommitGraphsCount(excludePRrefs bool, branches []string, files []string) (int64, error) {
+	cacheKey := fmt.Sprintf("commits-count-%d-graph-%t-%s-%s", r.Repository.ID, excludePRrefs, branches, files)
+
+	return cache.GetInt64(cacheKey, func() (int64, error) {
+		if len(branches) == 0 {
+			return git.AllCommitsCount(r.Repository.RepoPath(), excludePRrefs, files...)
+		}
+		return git.CommitsCountFiles(r.Repository.RepoPath(), branches, files)
+	})
+}
+
 // BranchNameSubURL sub-URL for the BranchName field
 func (r *Repository) BranchNameSubURL() string {
 	switch {
diff --git a/modules/git/commit.go b/modules/git/commit.go
index 6d2bc2b02..d33f0bc08 100644
--- a/modules/git/commit.go
+++ b/modules/git/commit.go
@@ -262,8 +262,19 @@ func CommitChangesWithArgs(repoPath string, args []string, opts CommitChangesOpt
 }
 
 // AllCommitsCount returns count of all commits in repository
-func AllCommitsCount(repoPath string) (int64, error) {
-	stdout, err := NewCommand("rev-list", "--all", "--count").RunInDir(repoPath)
+func AllCommitsCount(repoPath string, excludePRrefs bool, files ...string) (int64, error) {
+	args := []string{"--all", "--count"}
+	if excludePRrefs {
+		args = append([]string{"--exclude=refs/pull/*"}, args...)
+	}
+	cmd := NewCommand("rev-list")
+	cmd.AddArguments(args...)
+	if len(files) > 0 {
+		cmd.AddArguments("--")
+		cmd.AddArguments(files...)
+	}
+
+	stdout, err := cmd.RunInDir(repoPath)
 	if err != nil {
 		return 0, err
 	}
@@ -271,7 +282,8 @@ func AllCommitsCount(repoPath string) (int64, error) {
 	return strconv.ParseInt(strings.TrimSpace(stdout), 10, 64)
 }
 
-func commitsCount(repoPath string, revision, relpath []string) (int64, error) {
+// CommitsCountFiles returns number of total commits of until given revision.
+func CommitsCountFiles(repoPath string, revision, relpath []string) (int64, error) {
 	cmd := NewCommand("rev-list", "--count")
 	cmd.AddArguments(revision...)
 	if len(relpath) > 0 {
@@ -288,8 +300,8 @@ func commitsCount(repoPath string, revision, relpath []string) (int64, error) {
 }
 
 // CommitsCount returns number of total commits of until given revision.
-func CommitsCount(repoPath, revision string) (int64, error) {
-	return commitsCount(repoPath, []string{revision}, []string{})
+func CommitsCount(repoPath string, revision ...string) (int64, error) {
+	return CommitsCountFiles(repoPath, revision, []string{})
 }
 
 // CommitsCount returns number of total commits of until current revision.
diff --git a/modules/git/repo.go b/modules/git/repo.go
index 644ff0928..ae370d3da 100644
--- a/modules/git/repo.go
+++ b/modules/git/repo.go
@@ -49,7 +49,7 @@ const prettyLogFormat = `--pretty=format:%H`
 
 // GetAllCommitsCount returns count of all commits in repository
 func (repo *Repository) GetAllCommitsCount() (int64, error) {
-	return AllCommitsCount(repo.Path)
+	return AllCommitsCount(repo.Path, false)
 }
 
 func (repo *Repository) parsePrettyFormatLogToList(logs []byte) (*list.List, error) {
diff --git a/modules/git/repo_commit.go b/modules/git/repo_commit.go
index 45745c808..f46ba0cbb 100644
--- a/modules/git/repo_commit.go
+++ b/modules/git/repo_commit.go
@@ -318,7 +318,7 @@ func (repo *Repository) FileChangedBetweenCommits(filename, id1, id2 string) (bo
 
 // FileCommitsCount return the number of files at a revison
 func (repo *Repository) FileCommitsCount(revision, file string) (int64, error) {
-	return commitsCount(repo.Path, []string{revision}, []string{file})
+	return CommitsCountFiles(repo.Path, []string{revision}, []string{file})
 }
 
 // CommitsByFileAndRange return the commits according revison file and the page
@@ -413,11 +413,11 @@ func (repo *Repository) CommitsBetweenIDs(last, before string) (*list.List, erro
 
 // CommitsCountBetween return numbers of commits between two commits
 func (repo *Repository) CommitsCountBetween(start, end string) (int64, error) {
-	count, err := commitsCount(repo.Path, []string{start + "..." + end}, []string{})
+	count, err := CommitsCountFiles(repo.Path, []string{start + "..." + end}, []string{})
 	if err != nil && strings.Contains(err.Error(), "no merge base") {
 		// future versions of git >= 2.28 are likely to return an error if before and last have become unrelated.
 		// previously it would return the results of git rev-list before last so let's try that...
-		return commitsCount(repo.Path, []string{start, end}, []string{})
+		return CommitsCountFiles(repo.Path, []string{start, end}, []string{})
 	}
 
 	return count, err
diff --git a/modules/gitgraph/graph.go b/modules/gitgraph/graph.go
index 257e4f3af..2f58fd346 100644
--- a/modules/gitgraph/graph.go
+++ b/modules/gitgraph/graph.go
@@ -17,23 +17,42 @@ import (
 )
 
 // GetCommitGraph return a list of commit (GraphItems) from all branches
-func GetCommitGraph(r *git.Repository, page int, maxAllowedColors int) (*Graph, error) {
+func GetCommitGraph(r *git.Repository, page int, maxAllowedColors int, excludePRrefs bool, branches, files []string) (*Graph, error) {
 	format := "DATA:%d|%H|%ad|%an|%ae|%h|%s"
 
 	if page == 0 {
 		page = 1
 	}
 
-	graphCmd := git.NewCommand("log")
-	graphCmd.AddArguments("--graph",
-		"--date-order",
-		"--all",
+	args := make([]string, 0, 10+len(branches)+len(files))
+
+	args = append(args, "--graph", "--date-order")
+
+	if excludePRrefs {
+		args = append(args, "--exclude=refs/pull/*")
+	}
+
+	if len(branches) == 0 {
+		args = append(args, "--all")
+	}
+
+	args = append(args,
 		"-C",
 		"-M",
 		fmt.Sprintf("-n %d", setting.UI.GraphMaxCommitNum*page),
 		"--date=iso",
-		fmt.Sprintf("--pretty=format:%s", format),
-	)
+		fmt.Sprintf("--pretty=format:%s", format))
+
+	if len(branches) > 0 {
+		args = append(args, branches...)
+	}
+	args = append(args, "--")
+	if len(files) > 0 {
+		args = append(args, files...)
+	}
+
+	graphCmd := git.NewCommand("log")
+	graphCmd.AddArguments(args...)
 	graph := NewGraph()
 
 	stderr := new(strings.Builder)
diff --git a/modules/gitgraph/graph_test.go b/modules/gitgraph/graph_test.go
index ca9d653ce..d25174154 100644
--- a/modules/gitgraph/graph_test.go
+++ b/modules/gitgraph/graph_test.go
@@ -22,7 +22,7 @@ func BenchmarkGetCommitGraph(b *testing.B) {
 	defer currentRepo.Close()
 
 	for i := 0; i < b.N; i++ {
-		graph, err := GetCommitGraph(currentRepo, 1, 0)
+		graph, err := GetCommitGraph(currentRepo, 1, 0, false, nil, nil)
 		if err != nil {
 			b.Error("Could get commit graph")
 		}
diff --git a/routers/repo/commit.go b/routers/repo/commit.go
index d9547cc51..2ae4425d3 100644
--- a/routers/repo/commit.go
+++ b/routers/repo/commit.go
@@ -95,6 +95,16 @@ func Graph(ctx *context.Context) {
 		mode = "color"
 	}
 	ctx.Data["Mode"] = mode
+	excludePRrefs := ctx.QueryBool("exclude-pr-refs")
+	branches := ctx.QueryStrings("branch")
+	realBranches := make([]string, len(branches))
+	copy(realBranches, branches)
+	for i, branch := range realBranches {
+		if strings.HasPrefix(branch, "--") {
+			realBranches[i] = "refs/heads/" + branch
+		}
+	}
+	files := ctx.QueryStrings("file")
 
 	commitsCount, err := ctx.Repo.GetCommitsCount()
 	if err != nil {
@@ -102,15 +112,21 @@ func Graph(ctx *context.Context) {
 		return
 	}
 
-	allCommitsCount, err := ctx.Repo.GitRepo.GetAllCommitsCount()
+	graphCommitsCount, err := ctx.Repo.GetCommitGraphsCount(excludePRrefs, realBranches, files)
 	if err != nil {
-		ctx.ServerError("GetAllCommitsCount", err)
-		return
+		log.Warn("GetCommitGraphsCount error for generate graph exclude prs: %t branches: %s in %-v, Will Ignore branches and try again. Underlying Error: %v", excludePRrefs, branches, ctx.Repo.Repository, err)
+		realBranches = []string{}
+		branches = []string{}
+		graphCommitsCount, err = ctx.Repo.GetCommitGraphsCount(excludePRrefs, realBranches, files)
+		if err != nil {
+			ctx.ServerError("GetCommitGraphsCount", err)
+			return
+		}
 	}
 
 	page := ctx.QueryInt("page")
 
-	graph, err := gitgraph.GetCommitGraph(ctx.Repo.GitRepo, page, 0)
+	graph, err := gitgraph.GetCommitGraph(ctx.Repo.GitRepo, page, 0, excludePRrefs, realBranches, files)
 	if err != nil {
 		ctx.ServerError("GetCommitGraph", err)
 		return
@@ -121,8 +137,16 @@ func Graph(ctx *context.Context) {
 	ctx.Data["Reponame"] = ctx.Repo.Repository.Name
 	ctx.Data["CommitCount"] = commitsCount
 	ctx.Data["Branch"] = ctx.Repo.BranchName
-	paginator := context.NewPagination(int(allCommitsCount), setting.UI.GraphMaxCommitNum, page, 5)
+	ctx.Data["ExcludePRRefs"] = excludePRrefs
+	paginator := context.NewPagination(int(graphCommitsCount), setting.UI.GraphMaxCommitNum, page, 5)
 	paginator.AddParam(ctx, "mode", "Mode")
+	paginator.AddParam(ctx, "exclude-pr-refs", "ExcludePRRefs")
+	for _, branch := range branches {
+		paginator.AddParamString("branch", branch)
+	}
+	for _, file := range files {
+		paginator.AddParamString("file", file)
+	}
 	ctx.Data["Page"] = paginator
 	ctx.HTML(200, tplGraph)
 }
-- 
2.25.1

[6543]

I have said above. Once refs/pull/10/head deleted, the pull request view will be broken. So it cannot be deleted.

and you can not migrate pull requests either ...