diff --git a/pkg/commands/git_commands/commit_loader.go b/pkg/commands/git_commands/commit_loader.go index 48545edb9..3c01344d9 100644 --- a/pkg/commands/git_commands/commit_loader.go +++ b/pkg/commands/git_commands/commit_loader.go @@ -211,11 +211,11 @@ func (self *CommitLoader) extractCommitFromLine(line string, showDivergence bool authorEmail := split[3] extraInfo := strings.TrimSpace(split[4]) parentHashes := split[5] - message := split[6] divergence := models.DivergenceNone if showDivergence { - divergence = lo.Ternary(split[7] == "<", models.DivergenceLeft, models.DivergenceRight) + divergence = lo.Ternary(split[6] == "<", models.DivergenceLeft, models.DivergenceRight) } + message := split[7] tags := []string{} @@ -605,4 +605,4 @@ func (self *CommitLoader) getLogCmd(opts GetCommitsOptions) oscommands.ICmdObj { return self.cmd.New(cmdArgs).DontLog() } -const prettyFormat = `--pretty=format:%H%x00%at%x00%aN%x00%ae%x00%D%x00%p%x00%s%x00%m` +const prettyFormat = `--pretty=format:%H%x00%at%x00%aN%x00%ae%x00%D%x00%p%x00%m%x00%s` diff --git a/pkg/commands/git_commands/commit_loader_test.go b/pkg/commands/git_commands/commit_loader_test.go index 7f60209a4..d9aa8456c 100644 --- a/pkg/commands/git_commands/commit_loader_test.go +++ b/pkg/commands/git_commands/commit_loader_test.go @@ -15,16 +15,16 @@ import ( "github.com/stretchr/testify/assert" ) -var commitsOutput = strings.Replace(`0eea75e8c631fba6b58135697835d58ba4c18dbc|1640826609|Jesse Duffield|jessedduffield@gmail.com|HEAD -> better-tests|b21997d6b4cbdf84b149|better typing for rebase mode -b21997d6b4cbdf84b149d8e6a2c4d06a8e9ec164|1640824515|Jesse Duffield|jessedduffield@gmail.com|origin/better-tests|e94e8fc5b6fab4cb755f|fix logging -e94e8fc5b6fab4cb755f29f1bdb3ee5e001df35c|1640823749|Jesse Duffield|jessedduffield@gmail.com|tag: 123, tag: 456|d8084cd558925eb7c9c3|refactor -d8084cd558925eb7c9c38afeed5725c21653ab90|1640821426|Jesse Duffield|jessedduffield@gmail.com||65f910ebd85283b5cce9|WIP -65f910ebd85283b5cce9bf67d03d3f1a9ea3813a|1640821275|Jesse Duffield|jessedduffield@gmail.com||26c07b1ab33860a1a759|WIP -26c07b1ab33860a1a7591a0638f9925ccf497ffa|1640750752|Jesse Duffield|jessedduffield@gmail.com||3d4470a6c072208722e5|WIP -3d4470a6c072208722e5ae9a54bcb9634959a1c5|1640748818|Jesse Duffield|jessedduffield@gmail.com||053a66a7be3da43aacdc|WIP -053a66a7be3da43aacdc7aa78e1fe757b82c4dd2|1640739815|Jesse Duffield|jessedduffield@gmail.com||985fe482e806b172aea4|refactoring the config struct`, "|", "\x00", -1) +var commitsOutput = strings.Replace(`0eea75e8c631fba6b58135697835d58ba4c18dbc|1640826609|Jesse Duffield|jessedduffield@gmail.com|HEAD -> better-tests|b21997d6b4cbdf84b149|>|better typing for rebase mode +b21997d6b4cbdf84b149d8e6a2c4d06a8e9ec164|1640824515|Jesse Duffield|jessedduffield@gmail.com|origin/better-tests|e94e8fc5b6fab4cb755f|>|fix logging +e94e8fc5b6fab4cb755f29f1bdb3ee5e001df35c|1640823749|Jesse Duffield|jessedduffield@gmail.com|tag: 123, tag: 456|d8084cd558925eb7c9c3|>|refactor +d8084cd558925eb7c9c38afeed5725c21653ab90|1640821426|Jesse Duffield|jessedduffield@gmail.com||65f910ebd85283b5cce9|>|WIP +65f910ebd85283b5cce9bf67d03d3f1a9ea3813a|1640821275|Jesse Duffield|jessedduffield@gmail.com||26c07b1ab33860a1a759|>|WIP +26c07b1ab33860a1a7591a0638f9925ccf497ffa|1640750752|Jesse Duffield|jessedduffield@gmail.com||3d4470a6c072208722e5|>|WIP +3d4470a6c072208722e5ae9a54bcb9634959a1c5|1640748818|Jesse Duffield|jessedduffield@gmail.com||053a66a7be3da43aacdc|>|WIP +053a66a7be3da43aacdc7aa78e1fe757b82c4dd2|1640739815|Jesse Duffield|jessedduffield@gmail.com||985fe482e806b172aea4|>|refactoring the config struct`, "|", "\x00", -1) -var singleCommitOutput = strings.Replace(`0eea75e8c631fba6b58135697835d58ba4c18dbc|1640826609|Jesse Duffield|jessedduffield@gmail.com|HEAD -> better-tests|b21997d6b4cbdf84b149|better typing for rebase mode`, "|", "\x00", -1) +var singleCommitOutput = strings.Replace(`0eea75e8c631fba6b58135697835d58ba4c18dbc|1640826609|Jesse Duffield|jessedduffield@gmail.com|HEAD -> better-tests|b21997d6b4cbdf84b149|>|better typing for rebase mode`, "|", "\x00", -1) func TestGetCommits(t *testing.T) { type scenario struct { @@ -46,7 +46,7 @@ func TestGetCommits(t *testing.T) { opts: GetCommitsOptions{RefName: "HEAD", RefForPushedStatus: "mybranch", IncludeRebaseCommits: false}, runner: oscommands.NewFakeRunner(t). ExpectGitArgs([]string{"merge-base", "mybranch", "mybranch@{u}"}, "b21997d6b4cbdf84b149d8e6a2c4d06a8e9ec164", nil). - ExpectGitArgs([]string{"log", "HEAD", "--topo-order", "--oneline", "--pretty=format:%H%x00%at%x00%aN%x00%ae%x00%D%x00%p%x00%s%x00%m", "--abbrev=40", "--no-show-signature", "--"}, "", nil), + 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), expectedCommits: []*models.Commit{}, expectedError: nil, @@ -58,7 +58,7 @@ func TestGetCommits(t *testing.T) { opts: GetCommitsOptions{RefName: "refs/heads/mybranch", RefForPushedStatus: "refs/heads/mybranch", IncludeRebaseCommits: false}, runner: oscommands.NewFakeRunner(t). ExpectGitArgs([]string{"merge-base", "refs/heads/mybranch", "mybranch@{u}"}, "b21997d6b4cbdf84b149d8e6a2c4d06a8e9ec164", nil). - ExpectGitArgs([]string{"log", "refs/heads/mybranch", "--topo-order", "--oneline", "--pretty=format:%H%x00%at%x00%aN%x00%ae%x00%D%x00%p%x00%s%x00%m", "--abbrev=40", "--no-show-signature", "--"}, "", nil), + 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), expectedCommits: []*models.Commit{}, expectedError: nil, @@ -73,7 +73,7 @@ func TestGetCommits(t *testing.T) { // here it's seeing which commits are yet to be pushed ExpectGitArgs([]string{"merge-base", "mybranch", "mybranch@{u}"}, "b21997d6b4cbdf84b149d8e6a2c4d06a8e9ec164", nil). // here it's actually getting all the commits in a formatted form, one per line - ExpectGitArgs([]string{"log", "HEAD", "--topo-order", "--oneline", "--pretty=format:%H%x00%at%x00%aN%x00%ae%x00%D%x00%p%x00%s%x00%m", "--abbrev=40", "--no-show-signature", "--"}, commitsOutput, nil). + 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", "--"}, commitsOutput, nil). // here it's testing which of the configured main branches have an upstream ExpectGitArgs([]string{"rev-parse", "--symbolic-full-name", "master@{u}"}, "refs/remotes/origin/master", nil). // this one does ExpectGitArgs([]string{"rev-parse", "--symbolic-full-name", "main@{u}"}, "", errors.New("error")). // this one doesn't, so it checks origin instead @@ -210,7 +210,7 @@ func TestGetCommits(t *testing.T) { // here it's seeing which commits are yet to be pushed ExpectGitArgs([]string{"merge-base", "mybranch", "mybranch@{u}"}, "b21997d6b4cbdf84b149d8e6a2c4d06a8e9ec164", nil). // here it's actually getting all the commits in a formatted form, one per line - ExpectGitArgs([]string{"log", "HEAD", "--topo-order", "--oneline", "--pretty=format:%H%x00%at%x00%aN%x00%ae%x00%D%x00%p%x00%s%x00%m", "--abbrev=40", "--no-show-signature", "--"}, singleCommitOutput, nil). + 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", "--"}, singleCommitOutput, nil). // here it's testing which of the configured main branches exist; neither does ExpectGitArgs([]string{"rev-parse", "--symbolic-full-name", "master@{u}"}, "", errors.New("error")). ExpectGitArgs([]string{"rev-parse", "--verify", "--quiet", "refs/remotes/origin/master"}, "", errors.New("error")). @@ -247,7 +247,7 @@ func TestGetCommits(t *testing.T) { // here it's seeing which commits are yet to be pushed ExpectGitArgs([]string{"merge-base", "mybranch", "mybranch@{u}"}, "b21997d6b4cbdf84b149d8e6a2c4d06a8e9ec164", nil). // here it's actually getting all the commits in a formatted form, one per line - ExpectGitArgs([]string{"log", "HEAD", "--topo-order", "--oneline", "--pretty=format:%H%x00%at%x00%aN%x00%ae%x00%D%x00%p%x00%s%x00%m", "--abbrev=40", "--no-show-signature", "--"}, singleCommitOutput, nil). + 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", "--"}, singleCommitOutput, nil). // here it's testing which of the configured main branches exist ExpectGitArgs([]string{"rev-parse", "--symbolic-full-name", "master@{u}"}, "refs/remotes/origin/master", nil). ExpectGitArgs([]string{"rev-parse", "--symbolic-full-name", "main@{u}"}, "", errors.New("error")). @@ -283,7 +283,7 @@ func TestGetCommits(t *testing.T) { opts: GetCommitsOptions{RefName: "HEAD", RefForPushedStatus: "mybranch", IncludeRebaseCommits: false}, runner: oscommands.NewFakeRunner(t). ExpectGitArgs([]string{"merge-base", "mybranch", "mybranch@{u}"}, "b21997d6b4cbdf84b149d8e6a2c4d06a8e9ec164", nil). - ExpectGitArgs([]string{"log", "HEAD", "--oneline", "--pretty=format:%H%x00%at%x00%aN%x00%ae%x00%D%x00%p%x00%s%x00%m", "--abbrev=40", "--no-show-signature", "--"}, "", nil), + 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), expectedCommits: []*models.Commit{}, expectedError: nil, @@ -295,7 +295,7 @@ func TestGetCommits(t *testing.T) { opts: GetCommitsOptions{RefName: "HEAD", RefForPushedStatus: "mybranch", FilterPath: "src"}, runner: oscommands.NewFakeRunner(t). ExpectGitArgs([]string{"merge-base", "mybranch", "mybranch@{u}"}, "b21997d6b4cbdf84b149d8e6a2c4d06a8e9ec164", nil). - ExpectGitArgs([]string{"log", "HEAD", "--oneline", "--pretty=format:%H%x00%at%x00%aN%x00%ae%x00%D%x00%p%x00%s%x00%m", "--abbrev=40", "--follow", "--no-show-signature", "--", "src"}, "", nil), + 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), expectedCommits: []*models.Commit{}, expectedError: nil, diff --git a/pkg/commands/oscommands/cmd_obj_runner.go b/pkg/commands/oscommands/cmd_obj_runner.go index 296850e32..16257158e 100644 --- a/pkg/commands/oscommands/cmd_obj_runner.go +++ b/pkg/commands/oscommands/cmd_obj_runner.go @@ -161,7 +161,7 @@ func (self *cmdObjRunner) RunAndProcessLines(cmdObj ICmdObj, onLine func(line st } scanner := bufio.NewScanner(stdoutPipe) - scanner.Split(bufio.ScanLines) + scanner.Split(utils.ScanLinesAndTruncateWhenLongerThanBuffer(bufio.MaxScanTokenSize)) if err := cmd.Start(); err != nil { return err } @@ -178,6 +178,11 @@ func (self *cmdObjRunner) RunAndProcessLines(cmdObj ICmdObj, onLine func(line st } } + if scanner.Err() != nil { + _ = Kill(cmd) + return scanner.Err() + } + _ = cmd.Wait() self.log.Infof("%s (%s)", cmdObj.ToString(), time.Since(t)) diff --git a/pkg/gui/mergeconflicts/find_conflicts.go b/pkg/gui/mergeconflicts/find_conflicts.go index 3802a66b7..c4d3a51a8 100644 --- a/pkg/gui/mergeconflicts/find_conflicts.go +++ b/pkg/gui/mergeconflicts/find_conflicts.go @@ -99,7 +99,7 @@ func FileHasConflictMarkers(path string) (bool, error) { // Efficiently scans through a file looking for merge conflict markers. Returns true if it does func fileHasConflictMarkersAux(file io.Reader) bool { scanner := bufio.NewScanner(file) - scanner.Split(bufio.ScanLines) + scanner.Split(utils.ScanLinesAndTruncateWhenLongerThanBuffer(bufio.MaxScanTokenSize)) for scanner.Scan() { line := scanner.Bytes() diff --git a/pkg/tasks/tasks.go b/pkg/tasks/tasks.go index 202c3f23a..e80b63a2a 100644 --- a/pkg/tasks/tasks.go +++ b/pkg/tasks/tasks.go @@ -162,7 +162,7 @@ func (self *ViewBufferManager) NewCmdTask(start func() (*exec.Cmd, io.Reader), p done := make(chan struct{}) scanner := bufio.NewScanner(r) - scanner.Split(bufio.ScanLines) + scanner.Split(utils.ScanLinesAndTruncateWhenLongerThanBuffer(bufio.MaxScanTokenSize)) lineChan := make(chan []byte) lineWrittenChan := make(chan struct{}) diff --git a/pkg/utils/lines.go b/pkg/utils/lines.go index 662ba2f9b..c70d02ffc 100644 --- a/pkg/utils/lines.go +++ b/pkg/utils/lines.go @@ -1,6 +1,9 @@ package utils -import "strings" +import ( + "bytes" + "strings" +) // SplitLines takes a multiline string and splits it on newlines // currently we are also stripping \r's which may have adverse effects for @@ -43,3 +46,57 @@ func EscapeSpecialChars(str string) string { "\v", "\\v", ).Replace(str) } + +func dropCR(data []byte) []byte { + if len(data) > 0 && data[len(data)-1] == '\r' { + return data[0 : len(data)-1] + } + return data +} + +// ScanLinesAndTruncateWhenLongerThanBuffer returns a split function that can be +// used with bufio.Scanner.Split(). It is very similar to bufio.ScanLines, +// except that it will truncate lines that are longer than the scanner's read +// buffer (whereas bufio.ScanLines will return an error in that case, which is +// often difficult to handle). +// +// If you are using your own buffer for the scanner, you must set maxBufferSize +// to the same value as the max parameter that you passed to scanner.Buffer(). +// Otherwise, maxBufferSize must be set to bufio.MaxScanTokenSize. +func ScanLinesAndTruncateWhenLongerThanBuffer(maxBufferSize int) func(data []byte, atEOF bool) (int, []byte, error) { + skipOverRemainderOfLongLine := false + + return func(data []byte, atEOF bool) (int, []byte, error) { + if atEOF && len(data) == 0 { + // Done + return 0, nil, nil + } + if i := bytes.IndexByte(data, '\n'); i >= 0 { + if skipOverRemainderOfLongLine { + skipOverRemainderOfLongLine = false + return i + 1, nil, nil + } + return i + 1, dropCR(data[0:i]), nil + } + if atEOF { + if skipOverRemainderOfLongLine { + return len(data), nil, nil + } + + return len(data), dropCR(data), nil + } + + // Buffer is full, so we can't get more data + if len(data) >= maxBufferSize { + if skipOverRemainderOfLongLine { + return len(data), nil, nil + } + + skipOverRemainderOfLongLine = true + return len(data), data, nil + } + + // Request more data. + return 0, nil, nil + } +} diff --git a/pkg/utils/lines_test.go b/pkg/utils/lines_test.go index e7171022b..2192a3780 100644 --- a/pkg/utils/lines_test.go +++ b/pkg/utils/lines_test.go @@ -1,6 +1,8 @@ package utils import ( + "bufio" + "strings" "testing" "github.com/stretchr/testify/assert" @@ -100,3 +102,65 @@ func TestNormalizeLinefeeds(t *testing.T) { assert.EqualValues(t, string(s.expected), NormalizeLinefeeds(string(s.byteArray))) } } + +func TestScanLinesAndTruncateWhenLongerThanBuffer(t *testing.T) { + type scenario struct { + input string + expectedLines []string + } + + scenarios := []scenario{ + { + "", + []string{}, + }, + { + "\n", + []string{""}, + }, + { + "abc", + []string{"abc"}, + }, + { + "abc\ndef", + []string{"abc", "def"}, + }, + { + "abc\n\ndef", + []string{"abc", "", "def"}, + }, + { + "abc\r\ndef\r", + []string{"abc", "def"}, + }, + { + "abcdef", + []string{"abcde"}, + }, + { + "abcdef\n", + []string{"abcde"}, + }, + { + "abcdef\nghijkl\nx", + []string{"abcde", "ghijk", "x"}, + }, + { + "abc\ndefghijklmnopqrstuvw\nx", + []string{"abc", "defgh", "x"}, + }, + } + + for _, s := range scenarios { + scanner := bufio.NewScanner(strings.NewReader(s.input)) + scanner.Buffer(make([]byte, 5), 5) + scanner.Split(ScanLinesAndTruncateWhenLongerThanBuffer(5)) + result := []string{} + for scanner.Scan() { + result = append(result, scanner.Text()) + } + assert.NoError(t, scanner.Err()) + assert.EqualValues(t, s.expectedLines, result) + } +}