Repository: jesseduffield/lazygit
File: pkg/commands/git_commands/commit_loader.go (Go)

Code before:
type CommitLoader struct {
	*common.Common
	cmd oscommands.ICmdObjBuilder

	getRebaseMode   func() (enums.RebaseMode, error)
	readFile        func(filename string) ([]byte, error)
	walkFiles       func(root string, fn filepath.WalkFunc) error
	dotGitDir       string
	doBranchesExist map[string]bool
}

// making our dependencies explicit for the sake of easier testing
func NewCommitLoader(
	cmn *common.Common,
	cmd oscommands.ICmdObjBuilder,
	dotGitDir string,
	getRebaseMode func() (enums.RebaseMode, error),
) *CommitLoader {
	return &CommitLoader{
		Common:          cmn,
		cmd:             cmd,
		getRebaseMode:   getRebaseMode,
		readFile:        os.ReadFile,
		walkFiles:       filepath.Walk,
		dotGitDir:       dotGitDir,
		doBranchesExist: make(map[string]bool),
	}
}

type GetCommitsOptions struct {
	Limit                bool
	FilterPath           string
	IncludeRebaseCommits bool
	RefName              string // e.g. "HEAD" or "my_branch"
	// determines if we show the whole git graph i.e. pass the '--all' flag
	All bool
}

// GetCommits obtains the commits of the current branch
func (self *CommitLoader) GetCommits(opts GetCommitsOptions) ([]*models.Commit, error) {
	commits := []*models.Commit{}
	var rebasingCommits []*models.Commit

	if opts.IncludeRebaseCommits && opts.FilterPath == "" {
		var err error
		rebasingCommits, err = self.MergeRebasingCommits(commits)
		if err != nil {
			return nil, err
		}
		commits = append(commits, rebasingCommits...)
	}

Review comment (suggestion):
Rather than store a map of all the candidate branches and whether they exist, can we instead store the list of branches which do exist? So we can check if the slice is nil whenever we go to load commits and if it is, we can work out what the existing branches are. Having a per-branch mapping (with per-branch caching) seems too granular to me.

Code after:
type CommitLoader struct {
	*common.Common
	cmd oscommands.ICmdObjBuilder

	getRebaseMode func() (enums.RebaseMode, error)
	readFile      func(filename string) ([]byte, error)
	walkFiles     func(root string, fn filepath.WalkFunc) error
	dotGitDir     string
	// List of main branches that exist in the repo, quoted for direct use in a git command.
	// We use these to obtain the merge base of the branch.
	// When nil, we're yet to obtain the list of main branches.
	quotedMainBranches *string
}

// making our dependencies explicit for the sake of easier testing
func NewCommitLoader(
	cmn *common.Common,
	cmd oscommands.ICmdObjBuilder,
	dotGitDir string,
	getRebaseMode func() (enums.RebaseMode, error),
) *CommitLoader {
	return &CommitLoader{
		Common:             cmn,
		cmd:                cmd,
		getRebaseMode:      getRebaseMode,
		readFile:           os.ReadFile,
		walkFiles:          filepath.Walk,
		dotGitDir:          dotGitDir,
		quotedMainBranches: nil,
	}
}

type GetCommitsOptions struct {
	Limit                bool
	FilterPath           string
	IncludeRebaseCommits bool
	RefName              string // e.g. "HEAD" or "my_branch"
	// determines if we show the whole git graph i.e. pass the '--all' flag
	All bool
}

// GetCommits obtains the commits of the current branch
func (self *CommitLoader) GetCommits(opts GetCommitsOptions) ([]*models.Commit, error) {
	commits := []*models.Commit{}
	var rebasingCommits []*models.Commit

	if opts.IncludeRebaseCommits && opts.FilterPath == "" {
		var err error
		rebasingCommits, err = self.MergeRebasingCommits(commits)
		if err != nil {
			return nil, err