Skip to content

Commit 1037371

Browse files
stefanhallerstefanhaller
committed
Make Commit.Hash a getter for an unexported hash field
This is in preparation for turning the hash into pointer to a string.
1 parent 97aa7a0 commit 1037371

28 files changed

+301
-245
lines changed

‎pkg/app/daemon/rebase.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ func (self *TodoLine) ToString() string {
2222
if self.Action == "break" {
2323
return self.Action + "\n"
2424
} else {
25-
return self.Action + " " + self.Commit.Hash + " " + self.Commit.Name + "\n"
25+
return self.Action + " " + self.Commit.Hash() + " " + self.Commit.Name + "\n"
2626
}
2727
}
2828

‎pkg/commands/git_commands/commit_loader.go‎

Lines changed: 16 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -121,7 +121,7 @@ func (self *CommitLoader) GetCommits(opts GetCommitsOptions) ([]*models.Commit,
121121
}
122122

123123
for _, commit := range commits {
124-
if commit.Hash == firstPushedCommit {
124+
if commit.Hash() == firstPushedCommit {
125125
passedFirstPushedCommit = true
126126
}
127127
if !commit.IsTODO() {
@@ -234,7 +234,7 @@ func (self *CommitLoader) extractCommitFromLine(line string, showDivergence bool
234234
parents = strings.Split(parentHashes, " ")
235235
}
236236

237-
return &models.Commit{
237+
return models.NewCommit(models.NewCommitOpts{
238238
Hash: hash,
239239
Name: message,
240240
Tags: tags,
@@ -244,7 +244,7 @@ func (self *CommitLoader) extractCommitFromLine(line string, showDivergence bool
244244
AuthorEmail: authorEmail,
245245
Parents: parents,
246246
Divergence: divergence,
247-
}
247+
})
248248
}
249249

250250
func (self *CommitLoader) getHydratedRebasingCommits(addConflictingCommit bool) ([]*models.Commit, error) {
@@ -277,7 +277,7 @@ func (self *CommitLoader) getHydratedTodoCommits(todoCommits []*models.Commit, t
277277
}
278278

279279
commitHashes := lo.FilterMap(todoCommits, func(commit *models.Commit, _ int) (string, bool) {
280-
return commit.Hash, commit.Hash != ""
280+
return commit.Hash(), commit.Hash() != ""
281281
})
282282

283283
// note that we're not filtering these as we do non-rebasing commits just because
@@ -293,7 +293,7 @@ func (self *CommitLoader) getHydratedTodoCommits(todoCommits []*models.Commit, t
293293
fullCommits := map[string]*models.Commit{}
294294
err := cmdObj.RunAndProcessLines(func(line string) (bool, error) {
295295
commit := self.extractCommitFromLine(line, false)
296-
fullCommits[commit.Hash] = commit
296+
fullCommits[commit.Hash()] = commit
297297
return false, nil
298298
})
299299
if err != nil {
@@ -315,9 +315,9 @@ func (self *CommitLoader) getHydratedTodoCommits(todoCommits []*models.Commit, t
315315

316316
hydratedCommits := make([]*models.Commit, 0, len(todoCommits))
317317
for _, rebasingCommit := range todoCommits {
318-
if rebasingCommit.Hash == "" {
318+
if rebasingCommit.Hash() == "" {
319319
hydratedCommits = append(hydratedCommits, rebasingCommit)
320-
} else if commit := findFullCommit(rebasingCommit.Hash); commit != nil {
320+
} else if commit := findFullCommit(rebasingCommit.Hash()); commit != nil {
321321
commit.Action = rebasingCommit.Action
322322
commit.Status = rebasingCommit.Status
323323
hydratedCommits = append(hydratedCommits, commit)
@@ -364,12 +364,12 @@ func (self *CommitLoader) getRebasingCommits(addConflictingCommit bool) []*model
364364
// Command does not have a commit associated, skip
365365
continue
366366
}
367-
commits = utils.Prepend(commits, &models.Commit{
367+
commits = utils.Prepend(commits, models.NewCommit(models.NewCommitOpts{
368368
Hash: t.Commit,
369369
Name: t.Msg,
370370
Status: models.StatusRebasing,
371371
Action: t.Command,
372-
})
372+
}))
373373
}
374374

375375
return commits
@@ -465,11 +465,11 @@ func (self *CommitLoader) getConflictedCommitImpl(todos []todo.Todo, doneTodos [
465465

466466
// Any other todo that has a commit associated with it must have failed with
467467
// a conflict, otherwise we wouldn't have stopped the rebase:
468-
return &models.Commit{
468+
return models.NewCommit(models.NewCommitOpts{
469469
Hash: lastTodo.Commit,
470470
Action: lastTodo.Command,
471471
Status: models.StatusConflicted,
472-
}
472+
})
473473
}
474474

475475
func (self *CommitLoader) getSequencerCommits() []*models.Commit {
@@ -493,12 +493,12 @@ func (self *CommitLoader) getSequencerCommits() []*models.Commit {
493493
// Command does not have a commit associated, skip
494494
continue
495495
}
496-
commits = utils.Prepend(commits, &models.Commit{
496+
commits = utils.Prepend(commits, models.NewCommit(models.NewCommitOpts{
497497
Hash: t.Commit,
498498
Name: t.Msg,
499499
Status: models.StatusCherryPickingOrReverting,
500500
Action: t.Command,
501-
})
501+
}))
502502
}
503503

504504
return commits
@@ -526,11 +526,11 @@ func (self *CommitLoader) getConflictedSequencerCommit(workingTreeState models.W
526526
if len(lines) == 0 {
527527
return nil
528528
}
529-
return &models.Commit{
529+
return models.NewCommit(models.NewCommitOpts{
530530
Hash: lines[0],
531531
Status: models.StatusConflicted,
532532
Action: action,
533-
}
533+
})
534534
}
535535

536536
func setCommitMergedStatuses(ancestor string, commits []*models.Commit) {
@@ -541,7 +541,7 @@ func setCommitMergedStatuses(ancestor string, commits []*models.Commit) {
541541
passedAncestor := false
542542
for i, commit := range commits {
543543
// some commits aren't really commits and don't have hashes, such as the update-ref todo
544-
if commit.Hash != "" && strings.HasPrefix(ancestor, commit.Hash) {
544+
if commit.Hash() != "" && strings.HasPrefix(ancestor, commit.Hash()) {
545545
passedAncestor = true
546546
}
547547
if commit.Status != models.StatusPushed && commit.Status != models.StatusUnpushed {

‎pkg/commands/git_commands/commit_loader_test.go‎

Lines changed: 41 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import (
1010
"github.com/jesseduffield/lazygit/pkg/commands/oscommands"
1111
"github.com/jesseduffield/lazygit/pkg/config"
1212
"github.com/jesseduffield/lazygit/pkg/utils"
13+
"github.com/samber/lo"
1314
"github.com/stefanhaller/git-todo-parser/todo"
1415
"github.com/stretchr/testify/assert"
1516
)
@@ -27,13 +28,13 @@ var singleCommitOutput = strings.Replace(`0eea75e8c631fba6b58135697835d58ba4c18d
2728

2829
func TestGetCommits(t *testing.T) {
2930
type scenario struct {
30-
testName string
31-
runner *oscommands.FakeCmdObjRunner
32-
expectedCommits []*models.Commit
33-
expectedError error
34-
logOrder string
35-
opts GetCommitsOptions
36-
mainBranches []string
31+
testName string
32+
runner *oscommands.FakeCmdObjRunner
33+
expectedCommitOpts []models.NewCommitOpts
34+
expectedError error
35+
logOrder string
36+
opts GetCommitsOptions
37+
mainBranches []string
3738
}
3839

3940
scenarios := []scenario{
@@ -45,8 +46,8 @@ func TestGetCommits(t *testing.T) {
4546
ExpectGitArgs([]string{"merge-base", "mybranch", "mybranch@{u}"}, "b21997d6b4cbdf84b149d8e6a2c4d06a8e9ec164", nil).
4647
ExpectGitArgs([]string{"log", "HEAD", "--topo-order", "--oneline", "--pretty=format:%H%x00%at%x00%aN%x00%ae%x00%D%x00%p%x00%m%x00%s", "--abbrev=40", "--no-show-signature", "--"}, "", nil),
4748

48-
expectedCommits: []*models.Commit{},
49-
expectedError: nil,
49+
expectedCommitOpts: []models.NewCommitOpts{},
50+
expectedError: nil,
5051
},
5152
{
5253
testName: "should use proper upstream name for branch",
@@ -56,8 +57,8 @@ func TestGetCommits(t *testing.T) {
5657
ExpectGitArgs([]string{"merge-base", "refs/heads/mybranch", "mybranch@{u}"}, "b21997d6b4cbdf84b149d8e6a2c4d06a8e9ec164", nil).
5758
ExpectGitArgs([]string{"log", "refs/heads/mybranch", "--topo-order", "--oneline", "--pretty=format:%H%x00%at%x00%aN%x00%ae%x00%D%x00%p%x00%m%x00%s", "--abbrev=40", "--no-show-signature", "--"}, "", nil),
5859

59-
expectedCommits: []*models.Commit{},
60-
expectedError: nil,
60+
expectedCommitOpts: []models.NewCommitOpts{},
61+
expectedError: nil,
6162
},
6263
{
6364
testName: "should return commits if they are present",
@@ -79,7 +80,7 @@ func TestGetCommits(t *testing.T) {
7980
// here it's seeing where our branch diverged from the master branch so that we can mark that commit and parent commits as 'merged'
8081
ExpectGitArgs([]string{"merge-base", "HEAD", "refs/remotes/origin/master", "refs/remotes/origin/main"}, "26c07b1ab33860a1a7591a0638f9925ccf497ffa", nil),
8182

82-
expectedCommits: []*models.Commit{
83+
expectedCommitOpts: []models.NewCommitOpts{
8384
{
8485
Hash: "0eea75e8c631fba6b58135697835d58ba4c18dbc",
8586
Name: "better typing for rebase mode",
@@ -213,7 +214,7 @@ func TestGetCommits(t *testing.T) {
213214
ExpectGitArgs([]string{"rev-parse", "--verify", "--quiet", "refs/remotes/origin/main"}, "", errors.New("error")).
214215
ExpectGitArgs([]string{"rev-parse", "--verify", "--quiet", "refs/heads/main"}, "", errors.New("error")),
215216

216-
expectedCommits: []*models.Commit{
217+
expectedCommitOpts: []models.NewCommitOpts{
217218
{
218219
Hash: "0eea75e8c631fba6b58135697835d58ba4c18dbc",
219220
Name: "better typing for rebase mode",
@@ -251,7 +252,7 @@ func TestGetCommits(t *testing.T) {
251252
// here it's seeing where our branch diverged from the master branch so that we can mark that commit and parent commits as 'merged'
252253
ExpectGitArgs([]string{"merge-base", "HEAD", "refs/remotes/origin/master", "refs/remotes/origin/develop", "refs/remotes/origin/1.0-hotfixes"}, "26c07b1ab33860a1a7591a0638f9925ccf497ffa", nil),
253254

254-
expectedCommits: []*models.Commit{
255+
expectedCommitOpts: []models.NewCommitOpts{
255256
{
256257
Hash: "0eea75e8c631fba6b58135697835d58ba4c18dbc",
257258
Name: "better typing for rebase mode",
@@ -277,8 +278,8 @@ func TestGetCommits(t *testing.T) {
277278
ExpectGitArgs([]string{"merge-base", "mybranch", "mybranch@{u}"}, "b21997d6b4cbdf84b149d8e6a2c4d06a8e9ec164", nil).
278279
ExpectGitArgs([]string{"log", "HEAD", "--oneline", "--pretty=format:%H%x00%at%x00%aN%x00%ae%x00%D%x00%p%x00%m%x00%s", "--abbrev=40", "--no-show-signature", "--"}, "", nil),
279280

280-
expectedCommits: []*models.Commit{},
281-
expectedError: nil,
281+
expectedCommitOpts: []models.NewCommitOpts{},
282+
expectedError: nil,
282283
},
283284
{
284285
testName: "should set filter path",
@@ -288,8 +289,8 @@ func TestGetCommits(t *testing.T) {
288289
ExpectGitArgs([]string{"merge-base", "mybranch", "mybranch@{u}"}, "b21997d6b4cbdf84b149d8e6a2c4d06a8e9ec164", nil).
289290
ExpectGitArgs([]string{"log", "HEAD", "--oneline", "--pretty=format:%H%x00%at%x00%aN%x00%ae%x00%D%x00%p%x00%m%x00%s", "--abbrev=40", "--follow", "--no-show-signature", "--", "src"}, "", nil),
290291

291-
expectedCommits: []*models.Commit{},
292-
expectedError: nil,
292+
expectedCommitOpts: []models.NewCommitOpts{},
293+
expectedError: nil,
293294
},
294295
}
295296

@@ -318,7 +319,9 @@ func TestGetCommits(t *testing.T) {
318319
opts.MainBranches = NewMainBranches(common, cmd)
319320
commits, err := builder.GetCommits(opts)
320321

321-
assert.Equal(t, scenario.expectedCommits, commits)
322+
expectedCommits := lo.Map(scenario.expectedCommitOpts,
323+
func(opts models.NewCommitOpts, _ int) *models.Commit { return models.NewCommit(opts) })
324+
assert.Equal(t, expectedCommits, commits)
322325
assert.Equal(t, scenario.expectedError, err)
323326

324327
scenario.runner.CheckForMissingCalls()
@@ -356,11 +359,11 @@ func TestCommitLoader_getConflictedCommitImpl(t *testing.T) {
356359
},
357360
},
358361
amendFileExists: false,
359-
expectedResult: &models.Commit{
362+
expectedResult: models.NewCommit(models.NewCommitOpts{
360363
Hash: "fa1afe1",
361364
Action: todo.Pick,
362365
Status: models.StatusConflicted,
363-
},
366+
}),
364367
},
365368
{
366369
testName: "last command was 'break'",
@@ -457,11 +460,11 @@ func TestCommitLoader_getConflictedCommitImpl(t *testing.T) {
457460
},
458461
},
459462
amendFileExists: false,
460-
expectedResult: &models.Commit{
463+
expectedResult: models.NewCommit(models.NewCommitOpts{
461464
Hash: "fa1afe1",
462465
Action: todo.Pick,
463466
Status: models.StatusConflicted,
464-
},
467+
}),
465468
},
466469
{
467470
testName: "'edit' with amend file",
@@ -486,11 +489,11 @@ func TestCommitLoader_getConflictedCommitImpl(t *testing.T) {
486489
},
487490
amendFileExists: false,
488491
messageFileExists: true,
489-
expectedResult: &models.Commit{
492+
expectedResult: models.NewCommit(models.NewCommitOpts{
490493
Hash: "fa1afe1",
491494
Action: todo.Edit,
492495
Status: models.StatusConflicted,
493-
},
496+
}),
494497
},
495498
{
496499
testName: "'edit' without amend and without message file",
@@ -531,36 +534,36 @@ func TestCommitLoader_getConflictedCommitImpl(t *testing.T) {
531534

532535
func TestCommitLoader_setCommitMergedStatuses(t *testing.T) {
533536
type scenario struct {
534-
testName string
535-
commits []*models.Commit
536-
ancestor string
537-
expectedCommits []*models.Commit
537+
testName string
538+
commitOpts []models.NewCommitOpts
539+
ancestor string
540+
expectedCommitOpts []models.NewCommitOpts
538541
}
539542

540543
scenarios := []scenario{
541544
{
542545
testName: "basic",
543-
commits: []*models.Commit{
546+
commitOpts: []models.NewCommitOpts{
544547
{Hash: "12345", Name: "1", Action: models.ActionNone, Status: models.StatusUnpushed},
545548
{Hash: "67890", Name: "2", Action: models.ActionNone, Status: models.StatusPushed},
546549
{Hash: "abcde", Name: "3", Action: models.ActionNone, Status: models.StatusPushed},
547550
},
548551
ancestor: "67890",
549-
expectedCommits: []*models.Commit{
552+
expectedCommitOpts: []models.NewCommitOpts{
550553
{Hash: "12345", Name: "1", Action: models.ActionNone, Status: models.StatusUnpushed},
551554
{Hash: "67890", Name: "2", Action: models.ActionNone, Status: models.StatusMerged},
552555
{Hash: "abcde", Name: "3", Action: models.ActionNone, Status: models.StatusMerged},
553556
},
554557
},
555558
{
556559
testName: "with update-ref",
557-
commits: []*models.Commit{
560+
commitOpts: []models.NewCommitOpts{
558561
{Hash: "12345", Name: "1", Action: models.ActionNone, Status: models.StatusUnpushed},
559562
{Hash: "", Name: "", Action: todo.UpdateRef, Status: models.StatusNone},
560563
{Hash: "abcde", Name: "3", Action: models.ActionNone, Status: models.StatusPushed},
561564
},
562565
ancestor: "deadbeef",
563-
expectedCommits: []*models.Commit{
566+
expectedCommitOpts: []models.NewCommitOpts{
564567
{Hash: "12345", Name: "1", Action: models.ActionNone, Status: models.StatusUnpushed},
565568
{Hash: "", Name: "", Action: todo.UpdateRef, Status: models.StatusNone},
566569
{Hash: "abcde", Name: "3", Action: models.ActionNone, Status: models.StatusPushed},
@@ -570,9 +573,12 @@ func TestCommitLoader_setCommitMergedStatuses(t *testing.T) {
570573

571574
for _, scenario := range scenarios {
572575
t.Run(scenario.testName, func(t *testing.T) {
573-
commits := scenario.commits
576+
commits := lo.Map(scenario.commitOpts,
577+
func(opts models.NewCommitOpts, _ int) *models.Commit { return models.NewCommit(opts) })
574578
setCommitMergedStatuses(scenario.ancestor, commits)
575-
assert.Equal(t, scenario.expectedCommits, commits)
579+
expectedCommits := lo.Map(scenario.expectedCommitOpts,
580+
func(opts models.NewCommitOpts, _ int) *models.Commit { return models.NewCommit(opts) })
581+
assert.Equal(t, expectedCommits, commits)
576582
})
577583
}
578584
}

‎pkg/commands/git_commands/patch.go‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -156,13 +156,13 @@ func (self *PatchCommands) MovePatchToSelectedCommit(commits []*models.Commit, s
156156
baseIndex := sourceCommitIdx + 1
157157

158158
changes := []daemon.ChangeTodoAction{
159-
{Hash: commits[sourceCommitIdx].Hash, NewAction: todo.Edit},
160-
{Hash: commits[destinationCommitIdx].Hash, NewAction: todo.Edit},
159+
{Hash: commits[sourceCommitIdx].Hash(), NewAction: todo.Edit},
160+
{Hash: commits[destinationCommitIdx].Hash(), NewAction: todo.Edit},
161161
}
162162
self.os.LogCommand(logTodoChanges(changes), false)
163163

164164
err := self.rebase.PrepareInteractiveRebaseCommand(PrepareInteractiveRebaseCommandOpts{
165-
baseHashOrRoot: commits[baseIndex].Hash,
165+
baseHashOrRoot: commits[baseIndex].Hash(),
166166
overrideEditor: true,
167167
instruction: daemon.NewChangeTodoActionsInstruction(changes),
168168
}).Run()
@@ -218,7 +218,7 @@ func (self *PatchCommands) MovePatchToSelectedCommit(commits []*models.Commit, s
218218

219219
func (self *PatchCommands) MovePatchIntoIndex(commits []*models.Commit, commitIdx int, stash bool) error {
220220
if stash {
221-
if err := self.stash.Push(self.Tr.StashPrefix + commits[commitIdx].Hash); err != nil {
221+
if err := self.stash.Push(self.Tr.StashPrefix + commits[commitIdx].Hash()); err != nil {
222222
return err
223223
}
224224
}
@@ -323,7 +323,7 @@ func (self *PatchCommands) diffHeadAgainstCommit(commit *models.Commit) (string,
323323
cmdArgs := NewGitCmd("diff").
324324
Config("diff.noprefix=false").
325325
Arg("--no-ext-diff").
326-
Arg("HEAD.." + commit.Hash).
326+
Arg("HEAD.." + commit.Hash()).
327327
ToArgv()
328328

329329
return self.cmd.New(cmdArgs).RunWithOutput()

0 commit comments

Comments
 (0)