exclude interactive rebase TODO commits from commit graph

2 files changed, 263 insertions, 27 deletions

diff --git a/pkg/gui/presentation/commits.go b/pkg/gui/presentation/commits.go
index 0bc84d5e8..7f5b2e5cd 100644
--- a/pkg/gui/presentation/commits.go
+++ b/pkg/gui/presentation/commits.go
@@ -49,39 +49,40 @@ func GetCommitListDisplayStrings(
 		return nil
 	}
 
-	// given that our cache key is a commit sha and a commit count, it's very important that we don't actually try to render pipes
-	// when dealing with things like filtered commits.
-	cacheKey := pipeSetCacheKey{
-		commitSha:   commits[0].Sha,
-		commitCount: len(commits),
-	}
-
-	pipeSets, ok := pipeSetCache[cacheKey]
-	if !ok {
-		// pipe sets are unique to a commit head. and a commit count. Sometimes we haven't loaded everything for that.
-		// so let's just cache it based on that.
-		getStyle := func(commit *models.Commit) style.TextStyle {
-			return authors.AuthorStyle(commit.Author)
-		}
-		pipeSets = graph.GetPipeSets(commits, getStyle)
-		pipeSetCache[cacheKey] = pipeSets
-	}
-
 	if startIdx > len(commits) {
 		return nil
 	}
-	end := startIdx + length
-	if end > len(commits)-1 {
-		end = len(commits) - 1
-	}
 
-	filteredCommits := commits[startIdx : end+1]
+	// this is where my non-TODO commits begin
+	rebaseOffset := indexOfFirstNonTODOCommit(commits)
+
+	end := utils.Min(startIdx+length, len(commits))
+
+	filteredCommits := commits[startIdx:end]
 
+	// function expects to be passed the index of the commit in terms of the `commits` slice
 	var getGraphLine func(int) string
 	if showGraph {
-		filteredPipeSets := pipeSets[startIdx : end+1]
-		graphLines := graph.RenderAux(filteredPipeSets, filteredCommits, selectedCommitSha)
-		getGraphLine = func(idx int) string { return graphLines[idx] }
+		// this is where the graph begins (may be beyond the TODO commits depending on startIdx,
+		// but we'll never include TODO commits as part of the graph because it'll be messy)
+		graphOffset := utils.Max(startIdx, rebaseOffset)
+
+		pipeSets := loadPipesets(commits[rebaseOffset:])
+		pipeSetOffset := utils.Max(startIdx-rebaseOffset, 0)
+		graphPipeSets := pipeSets[pipeSetOffset:utils.Max(end-rebaseOffset, 0)]
+		graphCommits := commits[graphOffset:end]
+		graphLines := graph.RenderAux(
+			graphPipeSets,
+			graphCommits,
+			selectedCommitSha,
+		)
+		getGraphLine = func(idx int) string {
+			if idx >= graphOffset {
+				return graphLines[idx-graphOffset]
+			} else {
+				return ""
+			}
+		}
 	} else {
 		getGraphLine = func(idx int) string { return "" }
 	}
@@ -96,7 +97,7 @@ func GetCommitListDisplayStrings(
 			cherryPickedCommitShaMap,
 			diffName,
 			parseEmoji,
-			getGraphLine(i),
+			getGraphLine(i+startIdx),
 			fullDescription,
 			bisectStatus,
 			bisectInfo,
@@ -105,6 +106,40 @@ func GetCommitListDisplayStrings(
 	return lines
 }
 
+// precondition: slice is not empty
+func indexOfFirstNonTODOCommit(commits []*models.Commit) int {
+	for i, commit := range commits {
+		if !commit.IsTODO() {
+			return i
+		}
+	}
+
+	// shouldn't land here
+	return 0
+}
+
+func loadPipesets(commits []*models.Commit) [][]*graph.Pipe {
+	// given that our cache key is a commit sha and a commit count, it's very important that we don't actually try to render pipes
+	// when dealing with things like filtered commits.
+	cacheKey := pipeSetCacheKey{
+		commitSha:   commits[0].Sha,
+		commitCount: len(commits),
+	}
+
+	pipeSets, ok := pipeSetCache[cacheKey]
+	if !ok {
+		// pipe sets are unique to a commit head. and a commit count. Sometimes we haven't loaded everything for that.
+		// so let's just cache it based on that.
+		getStyle := func(commit *models.Commit) style.TextStyle {
+			return authors.AuthorStyle(commit.Author)
+		}
+		pipeSets = graph.GetPipeSets(commits, getStyle)
+		pipeSetCache[cacheKey] = pipeSets
+	}
+
+	return pipeSets
+}
+
 // similar to the git_commands.BisectStatus but more gui-focused
 type BisectStatus int
 
diff --git a/pkg/gui/presentation/commits_test.go b/pkg/gui/presentation/commits_test.go
new file mode 100644
index 000000000..604a5ef5f
--- /dev/null
+++ b/pkg/gui/presentation/commits_test.go
@@ -0,0 +1,201 @@
+package presentation
+
+import (
+	"strings"
+	"testing"
+
+	"github.com/gookit/color"
+	"github.com/jesseduffield/lazygit/pkg/commands/git_commands"
+	"github.com/jesseduffield/lazygit/pkg/commands/models"
+	"github.com/jesseduffield/lazygit/pkg/utils"
+	"github.com/stretchr/testify/assert"
+	"github.com/xo/terminfo"
+)
+
+func init() {
+	color.ForceSetColorLevel(terminfo.ColorLevelNone)
+}
+
+func formatExpected(expected string) string {
+	return strings.TrimSpace(strings.ReplaceAll(expected, "\t", ""))
+}
+
+func TestGetCommitListDisplayStrings(t *testing.T) {
+	scenarios := []struct {
+		testName                 string
+		commits                  []*models.Commit
+		fullDescription          bool
+		cherryPickedCommitShaMap map[string]bool
+		diffName                 string
+		parseEmoji               bool
+		selectedCommitSha        string
+		startIdx                 int
+		length                   int
+		showGraph                bool
+		bisectInfo               *git_commands.BisectInfo
+		expected                 string
+	}{
+		{
+			testName:   "no commits",
+			commits:    []*models.Commit{},
+			startIdx:   0,
+			length:     1,
+			showGraph:  false,
+			bisectInfo: git_commands.NewNullBisectInfo(),
+			expected:   "",
+		},
+		{
+			testName: "some commits",
+			commits: []*models.Commit{
+				{Name: "commit1", Sha: "sha1"},
+				{Name: "commit2", Sha: "sha2"},
+			},
+			startIdx:   0,
+			length:     2,
+			showGraph:  false,
+			bisectInfo: git_commands.NewNullBisectInfo(),
+			expected: formatExpected(`
+		sha1 commit1
+		sha2 commit2
+						`),
+		},
+		{
+			testName: "showing graph",
+			commits: []*models.Commit{
+				{Name: "commit1", Sha: "sha1", Parents: []string{"sha2", "sha3"}},
+				{Name: "commit2", Sha: "sha2", Parents: []string{"sha3"}},
+				{Name: "commit3", Sha: "sha3", Parents: []string{"sha4"}},
+				{Name: "commit4", Sha: "sha4", Parents: []string{"sha5"}},
+				{Name: "commit5", Sha: "sha5", Parents: []string{"sha7"}},
+			},
+			startIdx:   0,
+			length:     5,
+			showGraph:  true,
+			bisectInfo: git_commands.NewNullBisectInfo(),
+			expected: formatExpected(`
+		sha1 ⏣─╮ commit1
+		sha2 ◯ │ commit2
+		sha3 ◯─╯ commit3
+		sha4 ◯ commit4
+		sha5 ◯ commit5
+						`),
+		},
+		{
+			testName: "showing graph, including rebase commits",
+			commits: []*models.Commit{
+				{Name: "commit1", Sha: "sha1", Parents: []string{"sha2", "sha3"}, Action: "pick"},
+				{Name: "commit2", Sha: "sha2", Parents: []string{"sha3"}, Action: "pick"},
+				{Name: "commit3", Sha: "sha3", Parents: []string{"sha4"}},
+				{Name: "commit4", Sha: "sha4", Parents: []string{"sha5"}},
+				{Name: "commit5", Sha: "sha5", Parents: []string{"sha7"}},
+			},
+			startIdx:   0,
+			length:     5,
+			showGraph:  true,
+			bisectInfo: git_commands.NewNullBisectInfo(),
+			expected: formatExpected(`
+		sha1 pick  commit1
+		sha2 pick  commit2
+		sha3       ◯ commit3
+		sha4       ◯ commit4
+		sha5       ◯ commit5
+				`),
+		},
+		{
+			testName: "showing graph, including rebase commits, with offset",
+			commits: []*models.Commit{
+				{Name: "commit1", Sha: "sha1", Parents: []string{"sha2", "sha3"}, Action: "pick"},
+				{Name: "commit2", Sha: "sha2", Parents: []string{"sha3"}, Action: "pick"},
+				{Name: "commit3", Sha: "sha3", Parents: []string{"sha4"}},
+				{Name: "commit4", Sha: "sha4", Parents: []string{"sha5"}},
+				{Name: "commit5", Sha: "sha5", Parents: []string{"sha7"}},
+			},
+			startIdx:   1,
+			length:     10,
+			showGraph:  true,
+			bisectInfo: git_commands.NewNullBisectInfo(),
+			expected: formatExpected(`
+		sha2 pick  commit2
+		sha3       ◯ commit3
+		sha4       ◯ commit4
+		sha5       ◯ commit5
+				`),
+		},
+		{
+			testName: "startIdx is passed TODO commits",
+			commits: []*models.Commit{
+				{Name: "commit1", Sha: "sha1", Parents: []string{"sha2", "sha3"}, Action: "pick"},
+				{Name: "commit2", Sha: "sha2", Parents: []string{"sha3"}, Action: "pick"},
+				{Name: "commit3", Sha: "sha3", Parents: []string{"sha4"}},
+				{Name: "commit4", Sha: "sha4", Parents: []string{"sha5"}},
+				{Name: "commit5", Sha: "sha5", Parents: []string{"sha7"}},
+			},
+			startIdx:   3,
+			length:     2,
+			showGraph:  true,
+			bisectInfo: git_commands.NewNullBisectInfo(),
+			expected: formatExpected(`
+		sha4 ◯ commit4
+		sha5 ◯ commit5
+				`),
+		},
+		{
+			testName: "only showing TODO commits",
+			commits: []*models.Commit{
+				{Name: "commit1", Sha: "sha1", Parents: []string{"sha2", "sha3"}, Action: "pick"},
+				{Name: "commit2", Sha: "sha2", Parents: []string{"sha3"}, Action: "pick"},
+				{Name: "commit3", Sha: "sha3", Parents: []string{"sha4"}},
+				{Name: "commit4", Sha: "sha4", Parents: []string{"sha5"}},
+				{Name: "commit5", Sha: "sha5", Parents: []string{"sha7"}},
+			},
+			startIdx:   0,
+			length:     2,
+			showGraph:  true,
+			bisectInfo: git_commands.NewNullBisectInfo(),
+			expected: formatExpected(`
+		sha1 pick  commit1
+		sha2 pick  commit2
+				`),
+		},
+		{
+			testName: "no TODO commits, towards bottom",
+			commits: []*models.Commit{
+				{Name: "commit1", Sha: "sha1", Parents: []string{"sha2", "sha3"}},
+				{Name: "commit2", Sha: "sha2", Parents: []string{"sha3"}},
+				{Name: "commit3", Sha: "sha3", Parents: []string{"sha4"}},
+				{Name: "commit4", Sha: "sha4", Parents: []string{"sha5"}},
+				{Name: "commit5", Sha: "sha5", Parents: []string{"sha7"}},
+			},
+			startIdx:   4,
+			length:     2,
+			showGraph:  true,
+			bisectInfo: git_commands.NewNullBisectInfo(),
+			expected: formatExpected(`
+			sha5 ◯ commit5
+				`),
+		},
+	}
+
+	for _, s := range scenarios {
+		s := s
+		t.Run(s.testName, func(t *testing.T) {
+			result := GetCommitListDisplayStrings(
+				s.commits,
+				s.fullDescription,
+				s.cherryPickedCommitShaMap,
+				s.diffName,
+				s.parseEmoji,
+				s.selectedCommitSha,
+				s.startIdx,
+				s.length,
+				s.showGraph,
+				s.bisectInfo,
+			)
+
+			renderedResult := utils.RenderDisplayStrings(result)
+			t.Logf("\n%s", renderedResult)
+
+			assert.EqualValues(t, s.expected, renderedResult)
+		})
+	}
+}