peezy-tech/jojo
2
0
Fork 0
mirror of https://codeberg.org/forgejo/forgejo.git synced 2026-05-13 06:20:24 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 3

fix: display orphan branches separately in commit graph (#10484)

Browse source 
Fixes #2705.
Fixes #7635.

This PR fixes the commit graph showing false connections for orphan/root commits. Connection lines are now shown only when a parent/child relationship exists. Visible relationships are determined using `git log`'s `%P` output by the new `ComputeGlyphConnectivity` function. The SVG template is adapted to render vertical lines conditionally.

Unit tests for `ComputeGlyphConnectivity` cover regular linear commit history, orphan commits, merge commits, and non-commit glyphs (`|`, `/`, `\`). Unit tests also cover the changes to the `git log` parsing. The SVG template was verified manually.

Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/10484
Reviewed-by: Gusted <gusted@noreply.codeberg.org>
Co-authored-by: Bram Hagens <bram@bramh.me>
Co-committed-by: Bram Hagens <bram@bramh.me>
This commit is contained in:
Bram Hagens 2025-12-29 02:53:54 +01:00 • committed by Gusted
parent 81cce58a48
commit 60fb59a7a0
5 changed files with 255 additions and 29 deletions
Download patch file Download diff file Expand all files Collapse all files

5
services/repository/gitgraph/graph.go
View file

@ -16,7 +16,7 @@ import (
// GetCommitGraph return a list of commit (GraphItems) from all branches
func GetCommitGraph(r *git.Repository, page, maxAllowedColors int, hidePRRefs bool, branches, files []string) (*Graph, error) {
format := "DATA:%D|%H|%aD|%h|%s"
format := "DATA:%P|%D|%H|%aD|%h|%s"
if page == 0 {
page = 1
@ -112,5 +112,8 @@ func GetCommitGraph(r *git.Repository, page, maxAllowedColors int, hidePRRefs bo
}); err != nil {
return graph, err
}
graph.ComputeGlyphConnectivity()
return graph, nil
}

127
services/repository/gitgraph/graph_models.go
View file

@ -15,6 +15,7 @@ import (
git_model "forgejo.org/models/git"
repo_model "forgejo.org/models/repo"
user_model "forgejo.org/models/user"
"forgejo.org/modules/container"
"forgejo.org/modules/git"
"forgejo.org/modules/log"
)
@ -27,18 +28,20 @@ func NewGraph() *Graph {
Column: -1,
}
graph.Flows = map[int64]*Flow{}
graph.continuationAbove = map[[2]int]bool{}
return graph
}
// Graph represents a collection of flows
type Graph struct {
Flows map[int64]*Flow
Commits []*Commit
MinRow int
MinColumn int
MaxRow int
MaxColumn int
relationCommit *Commit
Flows map[int64]*Flow
Commits []*Commit
MinRow int
MinColumn int
MaxRow int
MaxColumn int
relationCommit *Commit
continuationAbove map[[2]int]bool
}
// Width returns the width of the graph
@ -51,6 +54,69 @@ func (graph *Graph) Height() int {
return graph.MaxRow - graph.MinRow + 1
}
// ComputeGlyphConnectivity sets ConnectsUp/ConnectsDown for commit glyphs based on parent/child relationships
func (graph *Graph) ComputeGlyphConnectivity() {
revs := make(container.Set[string])
revByPos := make(map[[2]int]string)
for _, c := range graph.Commits {
if c.Rev != "" {
revs.Add(c.Rev)
revByPos[[2]int{c.Row, c.Column}] = c.Rev
}
}
connectsDown := make(container.Set[string])
connectsUp := make(container.Set[string])
// Commits with parents connect down (even if parent is on another page)
// Commits with visible children connect up
for _, c := range graph.Commits {
if len(c.ParentHashes) > 0 {
connectsDown.Add(c.Rev)
}
for _, parentHash := range c.ParentHashes {
if revs.Contains(parentHash) {
connectsUp.Add(parentHash)
}
}
}
// Commits with a non-commit glyph above also connect up
for _, flow := range graph.Flows {
for _, g := range flow.Glyphs {
if g.Glyph != '*' {
pos := [2]int{g.Row + 1, g.Column}
if rev, exists := revByPos[pos]; exists {
connectsUp.Add(rev)
}
}
}
}
// Commits with continuation from previous page connect up
for pos := range graph.continuationAbove {
if rev, exists := revByPos[pos]; exists {
connectsUp.Add(rev)
}
}
for _, flow := range graph.Flows {
for i := range flow.Glyphs {
glyph := &flow.Glyphs[i]
if glyph.Glyph == '*' {
pos := [2]int{glyph.Row, glyph.Column}
if rev, exists := revByPos[pos]; exists {
glyph.ConnectsUp = connectsUp.Contains(rev)
glyph.ConnectsDown = connectsDown.Contains(rev)
}
} else {
glyph.ConnectsUp = true
glyph.ConnectsDown = true
}
}
}
}
// AddGlyph adds glyph to flows
func (graph *Graph) AddGlyph(row, column int, flowID int64, color int, glyph byte) {
flow, ok := graph.Flows[flowID]
@ -175,17 +241,19 @@ func (flow *Flow) AddGlyph(row, column int, glyph byte) {
}
flow.Glyphs = append(flow.Glyphs, Glyph{
row,
column,
glyph,
Row: row,
Column: column,
Glyph: glyph,
})
}
// Glyph represents a coordinate and glyph
type Glyph struct {
Row int
Column int
Glyph byte
Row int
Column int
Glyph byte
ConnectsUp bool
ConnectsDown bool
}
// RelationCommit represents an empty relation commit
@ -195,28 +263,36 @@ var RelationCommit = &Commit{
// NewCommit creates a new commit from a provided line
func NewCommit(row, column int, line []byte) (*Commit, error) {
data := bytes.SplitN(line, []byte("|"), 5)
if len(data) < 5 {
data := bytes.SplitN(line, []byte("|"), 6)
if len(data) < 6 {
return nil, fmt.Errorf("malformed data section on line %d with commit: %s", row, string(line))
}
// Format is a slight modification from RFC1123Z
t, err := time.Parse("Mon, _2 Jan 2006 15:04:05 -0700", string(data[2]))
t, err := time.Parse("Mon, _2 Jan 2006 15:04:05 -0700", string(data[3]))
if err != nil {
return nil, fmt.Errorf("could not parse date of commit: %w", err)
}
var parents []string
for p := range bytes.FieldsSeq(data[0]) {
parents = append(parents, string(p))
}
return &Commit{
Row: row,
Column: column,
// 0 matches git log --pretty=format:%d => ref names, like the --decorate option of git-log(1)
Refs: newRefsFromRefNames(data[0]),
// 1 matches git log --pretty=format:%H => commit hash
Rev: string(data[1]),
// 2 matches git log --pretty=format:%aD => author date, RFC2822 style
// 0 matches git log --pretty=format:%P => parent hashes
ParentHashes: parents,
// 1 matches git log --pretty=format:%d => ref names, like the --decorate option of git-log(1)
Refs: newRefsFromRefNames(data[1]),
// 2 matches git log --pretty=format:%H => commit hash
Rev: string(data[2]),
// 3 matches git log --pretty=format:%aD => author date, RFC2822 style
Date: t,
// 3 matches git log --pretty=format:%h => abbreviated commit hash
ShortRev: string(data[3]),
// 4 matches git log --pretty=format:%s => subject
Subject: string(data[4]),
// 4 matches git log --pretty=format:%h => abbreviated commit hash
ShortRev: string(data[4]),
// 5 matches git log --pretty=format:%s => subject
Subject: string(data[5]),
}, nil
}
@ -254,6 +330,7 @@ type Commit struct {
Date time.Time
ShortRev string
Subject string
ParentHashes []string
}
// OnlyRelation returns whether this a relation only commit

139
services/repository/gitgraph/graph_test.go
View file

@ -10,6 +10,9 @@ import (
"testing"
"forgejo.org/modules/git"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)
func BenchmarkGetCommitGraph(b *testing.B) {
@ -32,7 +35,7 @@ func BenchmarkGetCommitGraph(b *testing.B) {
}
func BenchmarkParseCommitString(b *testing.B) {
testString := "* DATA:|4e61bacab44e9b4730e44a6615d04098dd3a8eaf|2016-12-20 21:10:41 +0100|4e61bac|Add route for graph"
testString := "* DATA:abc123||4e61bacab44e9b4730e44a6615d04098dd3a8eaf|Tue, 20 Dec 2016 21:10:41 +0100|4e61bac|Add route for graph"
parser := &Parser{}
parser.Reset()
@ -241,7 +244,7 @@ func TestParseGlyphs(t *testing.T) {
}
func TestCommitStringParsing(t *testing.T) {
dataFirstPart := "* DATA:|4e61bacab44e9b4730e44a6615d04098dd3a8eaf|Tue, 20 Dec 2016 21:10:41 +0100|4e61bac|"
dataFirstPart := "* DATA:abc123||4e61bacab44e9b4730e44a6615d04098dd3a8eaf|Tue, 20 Dec 2016 21:10:41 +0100|4e61bac|"
tests := []struct {
shouldPass bool
testName string
@ -269,6 +272,138 @@ func TestCommitStringParsing(t *testing.T) {
}
}
func TestNewCommitParentHashes(t *testing.T) {
tests := []struct {
name string
data string
expectedParents []string
}{
{
name: "no parents (orphan)",
data: "||4e61bacab44e9b4730e44a6615d04098dd3a8eaf|Tue, 20 Dec 2016 21:10:41 +0100|4e61bac|subject",
expectedParents: nil,
},
{
name: "single parent",
data: "abc123||4e61bacab44e9b4730e44a6615d04098dd3a8eaf|Tue, 20 Dec 2016 21:10:41 +0100|4e61bac|subject",
expectedParents: []string{"abc123"},
},
{
name: "multiple parents (merge)",
data: "abc123 def456||4e61bacab44e9b4730e44a6615d04098dd3a8eaf|Tue, 20 Dec 2016 21:10:41 +0100|4e61bac|subject",
expectedParents: []string{"abc123", "def456"},
},
}
for _, test := range tests {
t.Run(test.name, func(t *testing.T) {
commit, err := NewCommit(0, 0, []byte(test.data))
require.NoError(t, err)
assert.Equal(t, test.expectedParents, commit.ParentHashes)
})
}
}
func TestComputeGlyphConnectivity(t *testing.T) {
addCommit := func(graph *Graph, row, col int, hash string, parents []string) {
flowID := int64(col + 1)
commit := &Commit{Row: row, Column: col, Rev: hash, ParentHashes: parents, Flow: flowID}
graph.AddGlyph(row, col, flowID, 1, '*')
graph.Commits = append(graph.Commits, commit)
graph.Flows[flowID].Commits = append(graph.Flows[flowID].Commits, commit)
}
getCommitConnectivity := func(graph *Graph, row, col int) (up, down bool) {
for _, flow := range graph.Flows {
for _, g := range flow.Glyphs {
if g.Row == row && g.Column == col && g.Glyph == '*' {
return g.ConnectsUp, g.ConnectsDown
}
}
}
return false, false
}
t.Run("ConnectsDown/no parents", func(t *testing.T) {
graph := NewGraph()
addCommit(graph, 0, 0, "orphan", nil)
graph.ComputeGlyphConnectivity()
_, down := getCommitConnectivity(graph, 0, 0)
assert.False(t, down)
})
t.Run("ConnectsDown/has parent in graph", func(t *testing.T) {
graph := NewGraph()
addCommit(graph, 0, 0, "child", []string{"parent"})
addCommit(graph, 1, 0, "parent", nil)
graph.ComputeGlyphConnectivity()
_, down := getCommitConnectivity(graph, 0, 0)
assert.True(t, down)
})
t.Run("ConnectsDown/has parent outside graph", func(t *testing.T) {
graph := NewGraph()
addCommit(graph, 0, 0, "child", []string{"parent-not-in-graph"})
graph.ComputeGlyphConnectivity()
_, down := getCommitConnectivity(graph, 0, 0)
assert.True(t, down)
})
t.Run("ConnectsUp/no child, no glyph above, no continuation", func(t *testing.T) {
graph := NewGraph()
addCommit(graph, 0, 0, "orphan", nil)
graph.ComputeGlyphConnectivity()
up, _ := getCommitConnectivity(graph, 0, 0)
assert.False(t, up)
})
t.Run("ConnectsUp/has visible child", func(t *testing.T) {
graph := NewGraph()
addCommit(graph, 0, 0, "child", []string{"parent"})
addCommit(graph, 1, 0, "parent", nil)
graph.ComputeGlyphConnectivity()
up, _ := getCommitConnectivity(graph, 1, 0)
assert.True(t, up)
})
t.Run("ConnectsUp/has non-commit glyph above", func(t *testing.T) {
graph := NewGraph()
graph.AddGlyph(0, 0, 1, 1, '|')
addCommit(graph, 1, 0, "commit", nil)
graph.ComputeGlyphConnectivity()
up, _ := getCommitConnectivity(graph, 1, 0)
assert.True(t, up)
})
t.Run("ConnectsUp/has continuationAbove", func(t *testing.T) {
graph := NewGraph()
addCommit(graph, 0, 0, "commit", nil)
graph.continuationAbove[[2]int{0, 0}] = true
graph.ComputeGlyphConnectivity()
up, _ := getCommitConnectivity(graph, 0, 0)
assert.True(t, up)
})
t.Run("non-commit glyphs always connect both ways", func(t *testing.T) {
for _, glyph := range []byte{'|', '/', '\\'} {
graph := NewGraph()
graph.AddGlyph(0, 0, 1, 1, glyph)
graph.ComputeGlyphConnectivity()
g := graph.Flows[1].Glyphs[0]
assert.True(t, g.ConnectsUp, "glyph %q should connect up", glyph)
assert.True(t, g.ConnectsDown, "glyph %q should connect down", glyph)
}
})
}
var testglyphs = `*
*
*

3
services/repository/gitgraph/parser.go
View file

@ -80,6 +80,9 @@ func (parser *Parser) AddLineToGraph(graph *Graph, row int, line []byte) error {
}
continue
}
if column < len(parser.oldGlyphs) && parser.oldGlyphs[column] == '|' {
graph.continuationAbove[[2]int{row, column}] = true
}
err2 := graph.AddCommit(row, column, flowID, line[idx+5:])
if err != nil && err2 != nil {
err = fmt.Errorf("%v %w", err2, err)

10
templates/repo/graph/svgcontainer.tmpl
View file

@ -3,8 +3,16 @@
{{range $flowid, $flow := .Graph.Flows}}
<g id="flow-{{$flow.ID}}" class="flow-group flow-color-{{$flow.ColorNumber}} flow-color-16-{{$flow.Color16}}" data-flow="{{$flow.ID}}" data-color="{{$flow.ColorNumber}}">
<path d="{{range $i, $glyph := $flow.Glyphs -}}
{{- if or (eq $glyph.Glyph '*') (eq $glyph.Glyph '|') -}}
{{- if eq $glyph.Glyph '|' -}}
M {{Eval $glyph.Column "*" 5 "+" 5}} {{Eval $glyph.Row "*" 12 "+" 0}} v 12 {{/* */ -}}
{{- else if eq $glyph.Glyph '*' -}}
{{- if and $glyph.ConnectsUp $glyph.ConnectsDown -}}
M {{Eval $glyph.Column "*" 5 "+" 5}} {{Eval $glyph.Row "*" 12 "+" 0}} v 12 {{/* */ -}}
{{- else if $glyph.ConnectsUp -}}
M {{Eval $glyph.Column "*" 5 "+" 5}} {{Eval $glyph.Row "*" 12 "+" 0}} v 6 {{/* */ -}}
{{- else if $glyph.ConnectsDown -}}
M {{Eval $glyph.Column "*" 5 "+" 5}} {{Eval $glyph.Row "*" 12 "+" 6}} v 6 {{/* */ -}}
{{- end -}}
{{- else if eq $glyph.Glyph '/' -}}
M {{Eval $glyph.Column "*" 5 "+" 10}} {{Eval $glyph.Row "*" 12 "+" 0}} l -10 12 {{/* */ -}}
{{- else if eq $glyph.Glyph '\\' -}}