Repository: jesseduffield/lazygit (language: go)

Issue:
Interactive rebase with a break prevents lazygit from startup
**Describe the bug**
Lazygit chokes on an interactive rebase with a break in the todo list.

**To Reproduce**
Steps to reproduce the behavior:
1. Start interactive rebase with `git rebase -i main`
2. Add two breaks anywhere
3. Start the rebase
4. Start lazygit - will crash immediately.

**Expected behavior**
No crash!

**Stacktrace**
```
panic: runtime error: slice bounds out of range [2:1]

goroutine 83 [running]:
github.com/jesseduffield/lazygit/pkg/commands/loaders.(*CommitLoader).getInteractiveRebasingCommits(0xc0000bf590)
	/tmp/makepkg/lazygit-git/src/src/github.com/jesseduffield/lazygit/pkg/commands/loaders/commits.go:320 +0x44b
github.com/jesseduffield/lazygit/pkg/commands/loaders.(*CommitLoader).getRebasingCommits(0xbdcc28, 0xc000202120)
	/tmp/makepkg/lazygit-git/src/src/github.com/jesseduffield/lazygit/pkg/commands/loaders/commits.go:244 +0x4f
github.com/jesseduffield/lazygit/pkg/commands/loaders.(*CommitLoader).getHydratedRebasingCommits(0xc0000bf590, 0xaeb365)
	/tmp/makepkg/lazygit-git/src/src/github.com/jesseduffield/lazygit/pkg/commands/loaders/commits.go:194 +0x33
github.com/jesseduffield/lazygit/pkg/commands/loaders.(*CommitLoader).MergeRebasingCommits(0xc0000bf590, {0x109ee90, 0x0, 0x0})
	/tmp/makepkg/lazygit-git/src/src/github.com/jesseduffield/lazygit/pkg/commands/loaders/commits.go:145 +0x14b
github.com/jesseduffield/lazygit/pkg/commands/loaders.(*CommitLoader).GetCommits(0xc0000bf590, {0x1, {0x0, 0x0}, 0x1, {0xaeb551, 0x4}, 0x0})
	/tmp/makepkg/lazygit-git/src/src/github.com/jesseduffield/lazygit/pkg/commands/loaders/commits.go:78 +0xdf
github.com/jesseduffield/lazygit/pkg/gui.(*Gui).refreshCommitsWithLimit(0xc00000c3c0)
	/tmp/makepkg/lazygit-git/src/src/github.com/jesseduffield/lazygit/pkg/gui/commits_panel.go:119 +0x1a5
github.com/jesseduffield/lazygit/pkg/gui.(*Gui).refreshCommits.func2()
	/tmp/makepkg/lazygit-git/src/src/github.com/jesseduffield/lazygit/pkg/gui/commits_panel.go:94 +0x2f
github.com/jesseduffield/lazygit/pkg/utils.Safe.func1()
	/tmp/makepkg/lazygit-git/src/src/github.com/jesseduffield/lazygit/pkg/utils/utils.go:95 +0x1b
github.com/jesseduffield/lazygit/pkg/utils.SafeWithError(0x0)
	/tmp/makepkg/lazygit-git/src/src/github.com/jesseduffield/lazygit/pkg/utils/utils.go:106 +0x67
github.com/jesseduffield/lazygit/pkg/utils.Safe(0x0)
	/tmp/makepkg/lazygit-git/src/src/github.com/jesseduffield/lazygit/pkg/utils/utils.go:95 +0x35
created by github.com/jesseduffield/lazygit/pkg/gui.(*Gui).refreshCommits
	/tmp/makepkg/lazygit-git/src/src/github.com/jesseduffield/lazygit/pkg/gui/commits_panel.go:93 +0x167
```

**Desktop:**
 - OS: Arch Linux
 - Lazygit Version: commit=d13a6481, build date=20220128.090800, build source=unknown, version=v0.32.1.d13a6481, os=linux, arch=amd64

**Additional context**
The same thing happens when running lazygit and then doing the same rebase on a different terminal. Lazygit will then crash on the next refresh.



Discussion / hints:
Interesting, the culprit of this bug seems to be that we assume that lines in the `git-rebase-todo` match the pattern
`<action> <sha> <commit-msg>`:
```go
splitLine := strings.Split(line, " ")
commits = append([]*models.Commit{{
	Sha:    splitLine[1],
	Name:   strings.Join(splitLine[2:], " "),
	Status: "rebasing",
	Action: splitLine[0],
}}, commits...)
```
https://github.com/jesseduffield/lazygit/blob/master/pkg/commands/loaders/commits.go#L317

This may not be true for a bunch of cases, as shown in the example:
```
pick deadbee Implement feature XYZ
fixup f1a5c00 Fix to feature XYZ
exec make
pick c0ffeee The oneline of the next commit
break
edit deadbab The oneline of the commit after
squash deadbaf Squasssshhh
rename beefdea Renamed commit
drop beefdaa Oops
exec cd subdir; make test
```

So to really fix this we would have to do a proper parsing of the `todo` file grammar:
```
Commands:
p, pick <commit> = use commit
r, reword <commit> = use commit, but edit the commit message
e, edit <commit> = use commit, but stop for amending
s, squash <commit> = use commit, but meld into previous commit
f, fixup [-C | -c] <commit> = like "squash" but keep only the previous
                   commit's log message, unless -C is used, in which case
                   keep only this commit's message; -c is same as -C but
                   opens the editor
x, exec <command> = run command (the rest of the line) using shell
b, break = stop here (continue rebase later with 'git rebase --continue')
d, drop <commit> = remove commit
l, label <label> = label current HEAD with a name
t, reset <label> = reset HEAD to a label
m, merge [-C <commit> | -c <commit>] <label> [# <oneline>]
.       create a merge commit using the original merge commit's
.       message (or the oneline, if no original merge commit was
.       specified); use -c <commit> to reword the commit message
```

If we think implementing the parsing is worth it, I'd love to take a stab at it :D
Ah, good to know. That will likely make it break when using `exec`, too. That's something I didn't try with lazygit running, yet, but every now and then I have some complex rebase todo files including lots of `exec ...` commands. Would be cool if that could be parsed properly to not cause any problems.
Just as an update on this one, I've been working on a git todo file parser so maybe we'll be able to use it to solve this issue:
https://github.com/fsmiamoto/git-todo-parser
Very cool @fsmiamoto 

Fix patch:
diff --git a/go.mod b/go.mod
index 29671a030c2..50d6d2bacf6 100644
--- a/go.mod
+++ b/go.mod
@@ -9,6 +9,7 @@ require (
 	github.com/cli/safeexec v1.0.0
 	github.com/cloudfoundry/jibber_jabber v0.0.0-20151120183258-bcc4c8345a21
 	github.com/creack/pty v1.1.11
+	github.com/fsmiamoto/git-todo-parser v0.0.2
 	github.com/fsnotify/fsnotify v1.4.7
 	github.com/go-errors/errors v1.4.2
 	github.com/gookit/color v1.4.2
diff --git a/go.sum b/go.sum
index fc123630fd5..5a556e0138d 100644
--- a/go.sum
+++ b/go.sum
@@ -28,6 +28,8 @@ github.com/fatih/color v1.7.1-0.20180516100307-2d684516a886/go.mod h1:Zm6kSWBoL9
 github.com/fatih/color v1.9.0 h1:8xPHl4/q1VyqGIPif1F+1V3Y3lSmrq01EabUW3CoW5s=
 github.com/fatih/color v1.9.0/go.mod h1:eQcE1qtQxscV5RaZvpXrrb8Drkc3/DdQ+uUYCNjL+zU=
 github.com/flynn/go-shlex v0.0.0-20150515145356-3f9db97f8568/go.mod h1:xEzjJPgXI435gkrCt3MPfRiAkVrwSbHsst4LCFVfpJc=
+github.com/fsmiamoto/git-todo-parser v0.0.2 h1:l6Y+9q7jbM+yK/w6kASpHO7ejL9ARCErm3tCEqOT278=
+github.com/fsmiamoto/git-todo-parser v0.0.2/go.mod h1:B+AgTbNE2BARvJqzXygThzqxLIaEWvwr2sxKYYb0Fas=
 github.com/fsnotify/fsnotify v1.4.7 h1:IXs+QLmnXW2CcXuY+8Mzv/fWEsPGWxqefPtCP5CnV9I=
 github.com/fsnotify/fsnotify v1.4.7/go.mod h1:jwhsz4b93w/PPRr/qN1Yymfu8t87LnFCMoQvtojpjFo=
 github.com/gdamore/encoding v1.0.0 h1:+7OoQ1Bc6eTm5niUzBa0Ctsh6JbMW6Ra+YNuAtDBdko=
diff --git a/pkg/commands/loaders/commits.go b/pkg/commands/loaders/commits.go
index e38fbe21d42..69c88ccf57d 100644
--- a/pkg/commands/loaders/commits.go
+++ b/pkg/commands/loaders/commits.go
@@ -1,6 +1,7 @@
 package loaders
 
 import (
+	"bytes"
 	"fmt"
 	"io/ioutil"
 	"os"
@@ -9,6 +10,7 @@ import (
 	"strconv"
 	"strings"
 
+	"github.com/fsmiamoto/git-todo-parser/todo"
 	"github.com/jesseduffield/generics/slices"
 	"github.com/jesseduffield/lazygit/pkg/commands/models"
 	"github.com/jesseduffield/lazygit/pkg/commands/oscommands"
@@ -307,21 +309,23 @@ func (self *CommitLoader) getInteractiveRebasingCommits() ([]*models.Commit, err
 	}
 
 	commits := []*models.Commit{}
-	lines := strings.Split(string(bytesContent), "\n")
 
-	for _, line := range lines {
-		if line == "" || line == "noop" {
-			return commits, nil
-		}
-		if strings.HasPrefix(line, "#") {
+	todos, err := todo.Parse(bytes.NewBuffer(bytesContent))
+	if err != nil {
+		self.Log.Error(fmt.Sprintf("error occurred while parsing git-rebase-todo file: %s", err.Error()))
+		return nil, nil
+	}
+
+	for _, t := range todos {
+		if t.Commit == "" {
+			// Command does not have a commit associated, skip
 			continue
 		}
-		splitLine := strings.Split(line, " ")
 		commits = slices.Prepend(commits, &models.Commit{
-			Sha:    splitLine[1],
-			Name:   strings.Join(splitLine[2:], " "),
+			Sha:    t.Commit,
+			Name:   t.Msg,
 			Status: "rebasing",
-			Action: splitLine[0],
+			Action: t.Command.String(),
 		})
 	}
 
diff --git a/vendor/github.com/fsmiamoto/git-todo-parser/todo/parse.go b/vendor/github.com/fsmiamoto/git-todo-parser/todo/parse.go
new file mode 100644
index 00000000000..8203d315110
--- /dev/null
+++ b/vendor/github.com/fsmiamoto/git-todo-parser/todo/parse.go
@@ -0,0 +1,141 @@
+package todo
+
+import (
+	"bufio"
+	"errors"
+	"fmt"
+	"io"
+	"strings"
+)
+
+var (
+	ErrUnexpectedCommand = errors.New("unexpected command")
+	ErrMissingLabel      = errors.New("missing label")
+	ErrMissingCommit     = errors.New("missing commit")
+	ErrMissingExecCmd    = errors.New("missing command for exec")
+)
+
+func Parse(f io.Reader) ([]Todo, error) {
+	var result []Todo
+
+	scanner := bufio.NewScanner(f)
+	scanner.Split(bufio.ScanLines)
+
+	for scanner.Scan() {
+		line := scanner.Text()
+
+		trimmed := strings.TrimSpace(line)
+		if trimmed == "" {
+			continue
+		}
+
+		cmd, err := parseLine(line)
+		if err != nil {
+			return nil, fmt.Errorf("failed to parse line %q: %w", line, err)
+		}
+
+		result = append(result, cmd)
+	}
+
+	if err := scanner.Err(); err != nil {
+		return nil, fmt.Errorf("failed to parse input: %w", err)
+	}
+
+	return result, nil
+}
+
+func parseLine(line string) (Todo, error) {
+	var todo Todo
+
+	if strings.HasPrefix(line, CommentChar) {
+		todo.Command = Comment
+		todo.Comment = strings.TrimLeft(line, CommentChar)
+		return todo, nil
+	}
+
+	fields := strings.Fields(line)
+
+	for i := TodoCommand(Pick); i < Comment; i++ {
+		if isCommand(i, fields[0]) {
+			todo.Command = TodoCommand(i)
+			fields = fields[1:]
+			break
+		}
+	}
+
+	if todo.Command == 0 {
+		// unexpected command
+		return todo, ErrUnexpectedCommand
+	}
+
+	if todo.Command == Break {
+		return todo, nil
+	}
+
+	if todo.Command == Label || todo.Command == Reset {
+		if len(fields) == 0 {
+			return todo, ErrMissingLabel
+		}
+		todo.Label = fields[0]
+		return todo, nil
+	}
+
+	if todo.Command == Exec {
+		if len(fields) == 0 {
+			return todo, ErrMissingExecCmd
+		}
+		todo.ExecCommand = strings.Join(fields, " ")
+		return todo, nil
+	}
+
+	if todo.Command == Merge {
+		if fields[0] == "-C" || fields[0] == "-c" {
+			fields = fields[1:]
+			if len(fields) == 0 {
+				return todo, ErrMissingCommit
+			}
+			todo.Commit = fields[0]
+			fields = fields[1:]
+		}
+		if len(fields) == 0 {
+			return todo, ErrMissingLabel
+		}
+		todo.Label = fields[0]
+		fields = fields[1:]
+		if fields[0] == "#" {
+			fields = fields[1:]
+			todo.Msg = strings.Join(fields, " ")
+		}
+		return todo, nil
+	}
+
+	if todo.Command == Fixup {
+		if len(fields) == 0 {
+			return todo, ErrMissingCommit
+		}
+		// Skip flags
+		if fields[0] == "-C" || fields[0] == "-c" {
+			fields = fields[1:]
+		}
+	}
+
+	if len(fields) == 0 {
+		return todo, ErrMissingCommit
+	}
+
+	todo.Commit = fields[0]
+	fields = fields[1:]
+
+	// Trim # and whitespace
+	todo.Msg = strings.TrimPrefix(strings.Join(fields, " "), CommentChar+" ")
+
+	return todo, nil
+}
+
+func isCommand(i TodoCommand, s string) bool {
+	if i < 0 || i > Comment {
+		return false
+	}
+	return len(s) > 0 &&
+		(todoCommandInfo[i].cmd == s || todoCommandInfo[i].nickname == s)
+}
diff --git a/vendor/github.com/fsmiamoto/git-todo-parser/todo/todo.go b/vendor/github.com/fsmiamoto/git-todo-parser/todo/todo.go
new file mode 100644
index 00000000000..ce16652db95
--- /dev/null
+++ b/vendor/github.com/fsmiamoto/git-todo-parser/todo/todo.go
@@ -0,0 +1,75 @@
+package todo
+
+type TodoCommand int
+
+const (
+	Pick TodoCommand = iota + 1
+	Revert
+	Edit
+	Reword
+	Fixup
+	Squash
+
+	Exec
+	Break
+	Label
+	Reset
+	Merge
+
+	NoOp
+	Drop
+
+	Comment
+)
+
+const CommentChar = "#"
+
+type Todo struct {
+	Command     TodoCommand
+	Commit      string
+	Comment     string
+	ExecCommand string
+	Label       string
+	Msg         string
+}
+
+func (t TodoCommand) String() string {
+	return commandToString[t]
+}
+
+var commandToString = map[TodoCommand]string{
+	Pick:    "pick",
+	Revert:  "revert",
+	Edit:    "edit",
+	Reword:  "reword",
+	Fixup:   "fixup",
+	Squash:  "squash",
+	Exec:    "exec",
+	Break:   "break",
+	Label:   "label",
+	Reset:   "reset",
+	Merge:   "merge",
+	NoOp:    "noop",
+	Drop:    "drop",
+	Comment: "comment",
+}
+
+var todoCommandInfo = [14]struct {
+	nickname string
+	cmd      string
+}{
+	{"", ""}, // dummy value since we're using 1-based indexing
+	{"p", "pick"},
+	{"", "revert"},
+	{"e", "edit"},
+	{"r", "reword"},
+	{"f", "fixup"},
+	{"s", "squash"},
+	{"x", "exec"},
+	{"b", "break"},
+	{"l", "label"},
+	{"t", "reset"},
+	{"m", "merge"},
+	{"", "noop"},
+	{"d", "drop"},
+}
diff --git a/vendor/modules.txt b/vendor/modules.txt
index 4079924d60b..14915d4dae3 100644
--- a/vendor/modules.txt
+++ b/vendor/modules.txt
@@ -30,6 +30,9 @@ github.com/emirpasic/gods/utils
 # github.com/fatih/color v1.9.0
 ## explicit; go 1.13
 github.com/fatih/color
+# github.com/fsmiamoto/git-todo-parser v0.0.2
+## explicit; go 1.13
+github.com/fsmiamoto/git-todo-parser/todo
 # github.com/fsnotify/fsnotify v1.4.7
 ## explicit
 github.com/fsnotify/fsnotify


Test patch: