From bf4f06ab4e6ceefe388e0efefcc553526f3d96c2 Mon Sep 17 00:00:00 2001 From: Jesse Duffield Date: Sat, 19 Mar 2022 16:34:46 +1100 Subject: more generics --- pkg/commands/git_commands/working_tree.go | 8 ++-- pkg/commands/loaders/branches.go | 32 +++++++------- pkg/commands/loaders/commit_files.go | 31 +++++++------- pkg/commands/loaders/commit_files_test.go | 71 +++++++++++++++++++++++++++++++ pkg/commands/loaders/commits.go | 8 ++-- pkg/commands/loaders/commits_test.go | 18 -------- pkg/commands/loaders/tags.go | 20 +++------ pkg/commands/loaders/tags_test.go | 68 +++++++++++++++++++++++++++++ pkg/commands/oscommands/os.go | 15 +++---- pkg/commands/patch/patch_manager.go | 25 +++++------ pkg/commands/patch/patch_parser.go | 28 ++++++------ 11 files changed, 218 insertions(+), 106 deletions(-) create mode 100644 pkg/commands/loaders/commit_files_test.go create mode 100644 pkg/commands/loaders/tags_test.go (limited to 'pkg/commands') diff --git a/pkg/commands/git_commands/working_tree.go b/pkg/commands/git_commands/working_tree.go index f594a639b..08e247459 100644 --- a/pkg/commands/git_commands/working_tree.go +++ b/pkg/commands/git_commands/working_tree.go @@ -8,6 +8,7 @@ import ( "time" "github.com/go-errors/errors" + "github.com/jesseduffield/generics/slices" "github.com/jesseduffield/lazygit/pkg/commands/loaders" "github.com/jesseduffield/lazygit/pkg/commands/models" "github.com/jesseduffield/lazygit/pkg/commands/oscommands" @@ -46,10 +47,9 @@ func (self *WorkingTreeCommands) StageFile(path string) error { } func (self *WorkingTreeCommands) StageFiles(paths []string) error { - quotedPaths := make([]string, len(paths)) - for i, path := range paths { - quotedPaths[i] = self.cmd.Quote(path) - } + quotedPaths := slices.Map(paths, func(path string) string { + return self.cmd.Quote(path) + }) return self.cmd.New(fmt.Sprintf("git add -- %s", strings.Join(quotedPaths, " "))).Run() } diff --git a/pkg/commands/loaders/branches.go b/pkg/commands/loaders/branches.go index 90480ca9a..682c23ad4 100644 --- a/pkg/commands/loaders/branches.go +++ b/pkg/commands/loaders/branches.go @@ -66,13 +66,13 @@ outer: if strings.EqualFold(reflogBranch.Name, branch.Name) { branch.Recency = reflogBranch.Recency branchesWithRecency = append(branchesWithRecency, branch) - branches = append(branches[0:j], branches[j+1:]...) + branches = slices.Remove(branches, j) continue outer } } } - branches = append(branchesWithRecency, branches...) + branches = slices.Prepend(branches, branchesWithRecency...) foundHead := false for i, branch := range branches { @@ -159,7 +159,7 @@ func (self *BranchLoader) obtainBranches() []*models.Branch { trimmedOutput := strings.TrimSpace(output) outputLines := strings.Split(trimmedOutput, "\n") - branches := slices.FilterMap(outputLines, func(line string) (*models.Branch, bool) { + return slices.FilterMap(outputLines, func(line string) (*models.Branch, bool) { if line == "" { return nil, false } @@ -174,8 +174,6 @@ func (self *BranchLoader) obtainBranches() []*models.Branch { return obtainBranch(split), true }) - - return branches } // TODO: only look at the new reflog commits, and otherwise store the recencies in @@ -184,17 +182,21 @@ func (self *BranchLoader) obtainReflogBranches(reflogCommits []*models.Commit) [ foundBranches := set.New[string]() re := regexp.MustCompile(`checkout: moving from ([\S]+) to ([\S]+)`) reflogBranches := make([]*models.Branch, 0, len(reflogCommits)) + for _, commit := range reflogCommits { - if match := re.FindStringSubmatch(commit.Name); len(match) == 3 { - recency := utils.UnixToTimeAgo(commit.UnixTimestamp) - for _, branchName := range match[1:] { - if !foundBranches.Includes(branchName) { - foundBranches.Add(branchName) - reflogBranches = append(reflogBranches, &models.Branch{ - Recency: recency, - Name: branchName, - }) - } + match := re.FindStringSubmatch(commit.Name) + if len(match) != 3 { + continue + } + + recency := utils.UnixToTimeAgo(commit.UnixTimestamp) + for _, branchName := range match[1:] { + if !foundBranches.Includes(branchName) { + foundBranches.Add(branchName) + reflogBranches = append(reflogBranches, &models.Branch{ + Recency: recency, + Name: branchName, + }) } } } diff --git a/pkg/commands/loaders/commit_files.go b/pkg/commands/loaders/commit_files.go index 755db768d..d68571edb 100644 --- a/pkg/commands/loaders/commit_files.go +++ b/pkg/commands/loaders/commit_files.go @@ -4,9 +4,11 @@ import ( "fmt" "strings" + "github.com/jesseduffield/generics/slices" "github.com/jesseduffield/lazygit/pkg/commands/models" "github.com/jesseduffield/lazygit/pkg/commands/oscommands" "github.com/jesseduffield/lazygit/pkg/common" + "github.com/samber/lo" ) type CommitFileLoader struct { @@ -33,25 +35,22 @@ func (self *CommitFileLoader) GetFilesInDiff(from string, to string, reverse boo return nil, err } - return self.getCommitFilesFromFilenames(filenames), nil + return getCommitFilesFromFilenames(filenames), nil } -// filenames string is something like "file1\nfile2\nfile3" -func (self *CommitFileLoader) getCommitFilesFromFilenames(filenames string) []*models.CommitFile { - commitFiles := make([]*models.CommitFile, 0) - +// filenames string is something like "MM\x00file1\x00MU\x00file2\x00AA\x00file3\x00" +// so we need to split it by the null character and then map each status-name pair to a commit file +func getCommitFilesFromFilenames(filenames string) []*models.CommitFile { lines := strings.Split(strings.TrimRight(filenames, "\x00"), "\x00") - n := len(lines) - for i := 0; i < n-1; i += 2 { - // typical result looks like 'A my_file' meaning my_file was added - changeStatus := lines[i] - name := lines[i+1] - - commitFiles = append(commitFiles, &models.CommitFile{ - Name: name, - ChangeStatus: changeStatus, - }) + if len(lines) == 1 { + return []*models.CommitFile{} } - return commitFiles + // typical result looks like 'A my_file' meaning my_file was added + return slices.Map(lo.Chunk(lines, 2), func(chunk []string) *models.CommitFile { + return &models.CommitFile{ + ChangeStatus: chunk[0], + Name: chunk[1], + } + }) } diff --git a/pkg/commands/loaders/commit_files_test.go b/pkg/commands/loaders/commit_files_test.go new file mode 100644 index 000000000..a07390052 --- /dev/null +++ b/pkg/commands/loaders/commit_files_test.go @@ -0,0 +1,71 @@ +package loaders + +import ( + "testing" + + "github.com/jesseduffield/lazygit/pkg/commands/models" + "github.com/stretchr/testify/assert" +) + +func TestGetCommitFilesFromFilenames(t *testing.T) { + tests := []struct { + testName string + input string + output []*models.CommitFile + }{ + { + testName: "no files", + input: "", + output: []*models.CommitFile{}, + }, + { + testName: "one file", + input: "MM\x00Myfile\x00", + output: []*models.CommitFile{ + { + Name: "Myfile", + ChangeStatus: "MM", + }, + }, + }, + { + testName: "two files", + input: "MM\x00Myfile\x00M \x00MyOtherFile\x00", + output: []*models.CommitFile{ + { + Name: "Myfile", + ChangeStatus: "MM", + }, + { + Name: "MyOtherFile", + ChangeStatus: "M ", + }, + }, + }, + { + testName: "three files", + input: "MM\x00Myfile\x00M \x00MyOtherFile\x00 M\x00YetAnother\x00", + output: []*models.CommitFile{ + { + Name: "Myfile", + ChangeStatus: "MM", + }, + { + Name: "MyOtherFile", + ChangeStatus: "M ", + }, + { + Name: "YetAnother", + ChangeStatus: " M", + }, + }, + }, + } + + for _, test := range tests { + t.Run(test.testName, func(t *testing.T) { + result := getCommitFilesFromFilenames(test.input) + assert.Equal(t, test.output, result) + }) + } +} diff --git a/pkg/commands/loaders/commits.go b/pkg/commands/loaders/commits.go index 187a13bb0..20721be42 100644 --- a/pkg/commands/loaders/commits.go +++ b/pkg/commands/loaders/commits.go @@ -9,6 +9,7 @@ import ( "strconv" "strings" + "github.com/jesseduffield/generics/slices" "github.com/jesseduffield/lazygit/pkg/commands/models" "github.com/jesseduffield/lazygit/pkg/commands/oscommands" "github.com/jesseduffield/lazygit/pkg/commands/types/enums" @@ -200,10 +201,9 @@ func (self *CommitLoader) getHydratedRebasingCommits(rebaseMode enums.RebaseMode return nil, nil } - commitShas := make([]string, len(commits)) - for i, commit := range commits { - commitShas[i] = commit.Sha - } + commitShas := slices.Map(commits, func(commit *models.Commit) string { + return commit.Sha + }) // note that we're not filtering these as we do non-rebasing commits just because // I suspect that will cause some damage diff --git a/pkg/commands/loaders/commits_test.go b/pkg/commands/loaders/commits_test.go index ff406abaf..6bb81c57d 100644 --- a/pkg/commands/loaders/commits_test.go +++ b/pkg/commands/loaders/commits_test.go @@ -11,24 +11,6 @@ import ( "github.com/stretchr/testify/assert" ) -func NewDummyCommitLoader() *CommitLoader { - cmn := utils.NewDummyCommon() - - return &CommitLoader{ - Common: cmn, - cmd: nil, - getCurrentBranchName: func() (string, string, error) { return "master", "master", nil }, - getRebaseMode: func() (enums.RebaseMode, error) { return enums.REBASE_MODE_NONE, nil }, - dotGitDir: ".git", - readFile: func(filename string) ([]byte, error) { - return []byte(""), nil - }, - walkFiles: func(root string, fn filepath.WalkFunc) error { - return nil - }, - } -} - const commitsOutput = `0eea75e8c631fba6b58135697835d58ba4c18dbc|1640826609|Jesse Duffield| (HEAD -> better-tests)|b21997d6b4cbdf84b149|better typing for rebase mode b21997d6b4cbdf84b149d8e6a2c4d06a8e9ec164|1640824515|Jesse Duffield| (origin/better-tests)|e94e8fc5b6fab4cb755f|fix logging e94e8fc5b6fab4cb755f29f1bdb3ee5e001df35c|1640823749|Jesse Duffield||d8084cd558925eb7c9c3|refactor diff --git a/pkg/commands/loaders/tags.go b/pkg/commands/loaders/tags.go index 45b08a002..8e5063c34 100644 --- a/pkg/commands/loaders/tags.go +++ b/pkg/commands/loaders/tags.go @@ -1,8 +1,7 @@ package loaders import ( - "strings" - + "github.com/jesseduffield/generics/slices" "github.com/jesseduffield/lazygit/pkg/commands/models" "github.com/jesseduffield/lazygit/pkg/commands/oscommands" "github.com/jesseduffield/lazygit/pkg/common" @@ -27,25 +26,18 @@ func NewTagLoader( func (self *TagLoader) GetTags() ([]*models.Tag, error) { // get remote branches, sorted by creation date (descending) // see: https://git-scm.com/docs/git-tag#Documentation/git-tag.txt---sortltkeygt - remoteBranchesStr, err := self.cmd.New(`git tag --list --sort=-creatordate`).DontLog().RunWithOutput() + tagsOutput, err := self.cmd.New(`git tag --list --sort=-creatordate`).DontLog().RunWithOutput() if err != nil { return nil, err } - content := utils.TrimTrailingNewline(remoteBranchesStr) - if content == "" { - return nil, nil - } - - split := strings.Split(content, "\n") + split := utils.SplitLines(tagsOutput) - // first step is to get our remotes from go-git - tags := make([]*models.Tag, len(split)) - for i, tagName := range split { - tags[i] = &models.Tag{ + tags := slices.Map(split, func(tagName string) *models.Tag { + return &models.Tag{ Name: tagName, } - } + }) return tags, nil } diff --git a/pkg/commands/loaders/tags_test.go b/pkg/commands/loaders/tags_test.go new file mode 100644 index 000000000..5394fa3a8 --- /dev/null +++ b/pkg/commands/loaders/tags_test.go @@ -0,0 +1,68 @@ +package loaders + +import ( + "testing" + + "github.com/jesseduffield/lazygit/pkg/commands/models" + "github.com/jesseduffield/lazygit/pkg/commands/oscommands" + "github.com/jesseduffield/lazygit/pkg/utils" + "github.com/stretchr/testify/assert" +) + +const tagsOutput = `v0.34 +v0.33 +v0.32.2 +v0.32.1 +v0.32 +testtag +` + +func TestGetTags(t *testing.T) { + type scenario struct { + testName string + runner *oscommands.FakeCmdObjRunner + expectedTags []*models.Tag + expectedError error + } + + scenarios := []scenario{ + { + testName: "should return no tags if there are none", + runner: oscommands.NewFakeRunner(t). + Expect(`git tag --list --sort=-creatordate`, "", nil), + expectedTags: []*models.Tag{}, + expectedError: nil, + }, + { + testName: "should return tags if present", + runner: oscommands.NewFakeRunner(t). + Expect(`git tag --list --sort=-creatordate`, tagsOutput, nil), + expectedTags: []*models.Tag{ + {Name: "v0.34"}, + {Name: "v0.33"}, + {Name: "v0.32.2"}, + {Name: "v0.32.1"}, + {Name: "v0.32"}, + {Name: "testtag"}, + }, + expectedError: nil, + }, + } + + for _, scenario := range scenarios { + scenario := scenario + t.Run(scenario.testName, func(t *testing.T) { + loader := &TagLoader{ + Common: utils.NewDummyCommon(), + cmd: oscommands.NewDummyCmdObjBuilder(scenario.runner), + } + + tags, err := loader.GetTags() + + assert.Equal(t, scenario.expectedTags, tags) + assert.Equal(t, scenario.expectedError, err) + + scenario.runner.CheckForMissingCalls() + }) + } +} diff --git a/pkg/commands/oscommands/os.go b/pkg/commands/oscommands/os.go index f3df3956f..b6f018af5 100644 --- a/pkg/commands/oscommands/os.go +++ b/pkg/commands/oscommands/os.go @@ -12,6 +12,7 @@ import ( "github.com/go-errors/errors" "github.com/atotto/clipboard" + "github.com/jesseduffield/generics/slices" "github.com/jesseduffield/lazygit/pkg/common" "github.com/jesseduffield/lazygit/pkg/utils" ) @@ -173,15 +174,11 @@ func (c *OSCommand) FileExists(path string) (bool, error) { // PipeCommands runs a heap of commands and pipes their inputs/outputs together like A | B | C func (c *OSCommand) PipeCommands(commandStrings ...string) error { - cmds := make([]*exec.Cmd, len(commandStrings)) - logCmdStr := "" - for i, str := range commandStrings { - if i > 0 { - logCmdStr += " | " - } - logCmdStr += str - cmds[i] = c.Cmd.New(str).GetCmd() - } + cmds := slices.Map(commandStrings, func(cmdString string) *exec.Cmd { + return c.Cmd.New(cmdString).GetCmd() + }) + + logCmdStr := strings.Join(commandStrings, " | ") c.LogCommand(logCmdStr, true) for i := 0; i < len(cmds)-1; i++ { diff --git a/pkg/commands/patch/patch_manager.go b/pkg/commands/patch/patch_manager.go index 1282356f8..4fb6507e6 100644 --- a/pkg/commands/patch/patch_manager.go +++ b/pkg/commands/patch/patch_manager.go @@ -4,6 +4,8 @@ import ( "sort" "strings" + "github.com/jesseduffield/generics/maps" + "github.com/jesseduffield/generics/slices" "github.com/samber/lo" "github.com/sirupsen/logrus" ) @@ -72,8 +74,9 @@ func (p *PatchManager) Start(from, to string, reverse bool, canRebase bool) { func (p *PatchManager) addFileWhole(info *fileInfo) { info.mode = WHOLE lineCount := len(strings.Split(info.diff, "\n")) - info.includedLineIndices = make([]int, lineCount) // add every line index + // TODO: add tests and then use lo.Range to simplify + info.includedLineIndices = make([]int, lineCount) for i := 0; i < lineCount; i++ { info.includedLineIndices[i] = i } @@ -192,21 +195,15 @@ func (p *PatchManager) RenderPatchForFile(filename string, plain bool, reverse b func (p *PatchManager) renderEachFilePatch(plain bool) []string { // sort files by name then iterate through and render each patch - filenames := make([]string, len(p.fileInfoMap)) - index := 0 - for filename := range p.fileInfoMap { - filenames[index] = filename - index++ - } + filenames := maps.Keys(p.fileInfoMap) sort.Strings(filenames) - output := []string{} - for _, filename := range filenames { - patch := p.RenderPatchForFile(filename, plain, false, true) - if patch != "" { - output = append(output, patch) - } - } + patches := slices.Map(filenames, func(filename string) string { + return p.RenderPatchForFile(filename, plain, false, true) + }) + output := slices.Filter(patches, func(patch string) bool { + return patch != "" + }) return output } diff --git a/pkg/commands/patch/patch_parser.go b/pkg/commands/patch/patch_parser.go index 3810d8a29..097f01329 100644 --- a/pkg/commands/patch/patch_parser.go +++ b/pkg/commands/patch/patch_parser.go @@ -4,9 +4,9 @@ import ( "regexp" "strings" + "github.com/jesseduffield/generics/slices" "github.com/jesseduffield/lazygit/pkg/gui/style" "github.com/jesseduffield/lazygit/pkg/theme" - "github.com/jesseduffield/lazygit/pkg/utils" "github.com/samber/lo" "github.com/sirupsen/logrus" ) @@ -184,16 +184,21 @@ func parsePatch(patch string) ([]int, []int, []*PatchLine) { // Render returns the coloured string of the diff with any selected lines highlighted func (p *PatchParser) Render(firstLineIndex int, lastLineIndex int, incLineIndices []int) string { - renderedLines := make([]string, len(p.PatchLines)) - for index, patchLine := range p.PatchLines { + contentToDisplay := slices.Some(p.PatchLines, func(line *PatchLine) bool { + return line.Content != "" + }) + if !contentToDisplay { + return "" + } + + renderedLines := lo.Map(p.PatchLines, func(patchLine *PatchLine, index int) string { selected := index >= firstLineIndex && index <= lastLineIndex included := lo.Contains(incLineIndices, index) - renderedLines[index] = patchLine.render(selected, included) - } + return patchLine.render(selected, included) + }) + result := strings.Join(renderedLines, "\n") - if strings.TrimSpace(utils.Decolorise(result)) == "" { - return "" - } + return result } @@ -202,10 +207,9 @@ func (p *PatchParser) Render(firstLineIndex int, lastLineIndex int, incLineIndic func (p *PatchParser) PlainRenderLines(firstLineIndex, lastLineIndex int) string { linesToCopy := p.PatchLines[firstLineIndex : lastLineIndex+1] - renderedLines := make([]string, len(linesToCopy)) - for index, line := range linesToCopy { - renderedLines[index] = line.Content - } + renderedLines := slices.Map(linesToCopy, func(line *PatchLine) string { + return line.Content + }) return strings.Join(renderedLines, "\n") } -- cgit v1.2.3