From ae610dcbb713bb08d60346b3b5e03a4e7541b74d Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Wed, 9 Oct 2024 15:08:01 +0200 Subject: [PATCH 1/4] Extract helper function for easier testing --- pkg/utils/io.go | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/pkg/utils/io.go b/pkg/utils/io.go index d31c5fc073e..98d026429a9 100644 --- a/pkg/utils/io.go +++ b/pkg/utils/io.go @@ -2,6 +2,7 @@ package utils import ( "bufio" + "io" "os" ) @@ -12,14 +13,18 @@ func ForEachLineInFile(path string, f func(string, int)) error { } defer file.Close() - reader := bufio.NewReader(file) + forEachLineInStream(file, f) + + return nil +} + +func forEachLineInStream(reader io.Reader, f func(string, int)) { + bufferedReader := bufio.NewReader(reader) for i := 0; true; i++ { - line, err := reader.ReadString('\n') + line, err := bufferedReader.ReadString('\n') if err != nil { break } f(line, i) } - - return nil } From 72cf3efb1d39d3b268659edd759532bdf96ff49e Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Wed, 9 Oct 2024 15:08:42 +0200 Subject: [PATCH 2/4] Add test demonstrating problem with ForEachLineInFile The function drops the last line if it doesn't end with a line feed. --- pkg/utils/io_test.go | 63 ++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 63 insertions(+) create mode 100644 pkg/utils/io_test.go diff --git a/pkg/utils/io_test.go b/pkg/utils/io_test.go new file mode 100644 index 00000000000..bf1d46ebe3b --- /dev/null +++ b/pkg/utils/io_test.go @@ -0,0 +1,63 @@ +package utils + +import ( + "strings" + "testing" + + "github.com/stretchr/testify/assert" +) + +func Test_forEachLineInStream(t *testing.T) { + scenarios := []struct { + name string + input string + expectedLines []string + }{ + { + name: "empty input", + input: "", + expectedLines: []string{}, + }, + { + name: "single line", + input: "abc\n", + expectedLines: []string{"abc\n"}, + }, + { + name: "single line without line feed", + input: "abc", + /* EXPECTED: + expectedLines: []string{"abc"}, + ACTUAL: */ + expectedLines: []string{}, + }, + { + name: "multiple lines", + input: "abc\ndef\n", + expectedLines: []string{"abc\n", "def\n"}, + }, + { + name: "multiple lines including empty lines", + input: "abc\n\ndef\n", + expectedLines: []string{"abc\n", "\n", "def\n"}, + }, + { + name: "multiple lines without linefeed at end of file", + input: "abc\ndef\nghi", + /* EXPECTED: + expectedLines: []string{"abc\n", "def\n", "ghi"}, + ACTUAL: */ + expectedLines: []string{"abc\n", "def\n"}, + }, + } + + for _, s := range scenarios { + t.Run(s.name, func(t *testing.T) { + lines := []string{} + forEachLineInStream(strings.NewReader(s.input), func(line string, i int) { + lines = append(lines, line) + }) + assert.EqualValues(t, s.expectedLines, lines) + }) + } +} From b71aa5e23bf99f5973253adf5c9d26c326bcc1f0 Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Wed, 9 Oct 2024 15:30:22 +0200 Subject: [PATCH 3/4] Add regression test for resolving conflicts in a file without a trailing LF The test shows that the last line of the file is lost. --- .../conflicts/resolve_without_trailing_lf.go | 60 +++++++++++++++++++ pkg/integration/tests/test_list.go | 1 + 2 files changed, 61 insertions(+) create mode 100644 pkg/integration/tests/conflicts/resolve_without_trailing_lf.go diff --git a/pkg/integration/tests/conflicts/resolve_without_trailing_lf.go b/pkg/integration/tests/conflicts/resolve_without_trailing_lf.go new file mode 100644 index 00000000000..c85c3ea48b6 --- /dev/null +++ b/pkg/integration/tests/conflicts/resolve_without_trailing_lf.go @@ -0,0 +1,60 @@ +package conflicts + +import ( + "github.com/jesseduffield/lazygit/pkg/config" + . "github.com/jesseduffield/lazygit/pkg/integration/components" +) + +var ResolveWithoutTrailingLf = NewIntegrationTest(NewIntegrationTestArgs{ + Description: "Regression test for resolving a merge conflict when the file doesn't have a trailing newline", + ExtraCmdArgs: []string{}, + Skip: false, + SetupConfig: func(config *config.AppConfig) {}, + SetupRepo: func(shell *Shell) { + shell. + NewBranch("branch1"). + CreateFileAndAdd("file", "a\n\nno eol"). + Commit("initial commit"). + UpdateFileAndAdd("file", "a1\n\nno eol"). + Commit("commit on branch1"). + NewBranchFrom("branch2", "HEAD^"). + UpdateFileAndAdd("file", "a2\n\nno eol"). + Commit("commit on branch2"). + Checkout("branch1"). + RunCommandExpectError([]string{"git", "merge", "--no-edit", "branch2"}) + }, + Run: func(t *TestDriver, keys config.KeybindingConfig) { + t.Views().Files(). + IsFocused(). + Lines( + Contains("UU file").IsSelected(), + ). + PressEnter() + + t.Views().MergeConflicts(). + IsFocused(). + SelectedLines( + Contains("<<<<<<< HEAD"), + Contains("a1"), + Contains("======="), + ). + SelectNextItem(). + PressPrimaryAction() + + t.ExpectPopup().Alert(). + Title(Equals("Continue")). + Content(Contains("All merge conflicts resolved. Continue?")). + Cancel() + + t.Views().Files(). + Focus(). + Lines( + Contains("M file").IsSelected(), + ) + + /* EXPECTED: + t.Views().Main().Content(Contains("-a1\n+a2\n").DoesNotContain("-no eol")) + ACTUAL: */ + t.Views().Main().Content(Contains("-a1\n+a2\n").Contains("-no eol")) + }, +}) diff --git a/pkg/integration/tests/test_list.go b/pkg/integration/tests/test_list.go index 6c6d484cf75..0c0142e40a9 100644 --- a/pkg/integration/tests/test_list.go +++ b/pkg/integration/tests/test_list.go @@ -123,6 +123,7 @@ var tests = []*components.IntegrationTest{ conflicts.ResolveExternally, conflicts.ResolveMultipleFiles, conflicts.ResolveNoAutoStage, + conflicts.ResolveWithoutTrailingLf, conflicts.UndoChooseHunk, custom_commands.AccessCommitProperties, custom_commands.BasicCommand, From 696e78fcc8941aa684421e20fea11b69b07206b8 Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Wed, 9 Oct 2024 15:37:08 +0200 Subject: [PATCH 4/4] Fix ForEachLineInFile to not lose the last line if it doesn't end with a LF --- .../tests/conflicts/resolve_without_trailing_lf.go | 3 --- pkg/utils/io.go | 4 ++-- pkg/utils/io_test.go | 6 ------ 3 files changed, 2 insertions(+), 11 deletions(-) diff --git a/pkg/integration/tests/conflicts/resolve_without_trailing_lf.go b/pkg/integration/tests/conflicts/resolve_without_trailing_lf.go index c85c3ea48b6..2af1eac5445 100644 --- a/pkg/integration/tests/conflicts/resolve_without_trailing_lf.go +++ b/pkg/integration/tests/conflicts/resolve_without_trailing_lf.go @@ -52,9 +52,6 @@ var ResolveWithoutTrailingLf = NewIntegrationTest(NewIntegrationTestArgs{ Contains("M file").IsSelected(), ) - /* EXPECTED: t.Views().Main().Content(Contains("-a1\n+a2\n").DoesNotContain("-no eol")) - ACTUAL: */ - t.Views().Main().Content(Contains("-a1\n+a2\n").Contains("-no eol")) }, }) diff --git a/pkg/utils/io.go b/pkg/utils/io.go index 98d026429a9..1d222746b95 100644 --- a/pkg/utils/io.go +++ b/pkg/utils/io.go @@ -21,8 +21,8 @@ func ForEachLineInFile(path string, f func(string, int)) error { func forEachLineInStream(reader io.Reader, f func(string, int)) { bufferedReader := bufio.NewReader(reader) for i := 0; true; i++ { - line, err := bufferedReader.ReadString('\n') - if err != nil { + line, _ := bufferedReader.ReadString('\n') + if len(line) == 0 { break } f(line, i) diff --git a/pkg/utils/io_test.go b/pkg/utils/io_test.go index bf1d46ebe3b..b8dfa957c43 100644 --- a/pkg/utils/io_test.go +++ b/pkg/utils/io_test.go @@ -26,10 +26,7 @@ func Test_forEachLineInStream(t *testing.T) { { name: "single line without line feed", input: "abc", - /* EXPECTED: expectedLines: []string{"abc"}, - ACTUAL: */ - expectedLines: []string{}, }, { name: "multiple lines", @@ -44,10 +41,7 @@ func Test_forEachLineInStream(t *testing.T) { { name: "multiple lines without linefeed at end of file", input: "abc\ndef\nghi", - /* EXPECTED: expectedLines: []string{"abc\n", "def\n", "ghi"}, - ACTUAL: */ - expectedLines: []string{"abc\n", "def\n"}, }, }