diff --git a/pkg/commands/models/file.go b/pkg/commands/models/file.go index e48696a4f..9eedfb1fc 100644 --- a/pkg/commands/models/file.go +++ b/pkg/commands/models/file.go @@ -18,10 +18,14 @@ type File struct { Deleted bool HasMergeConflicts bool HasInlineMergeConflicts bool - DisplayString string - ShortStatus string // e.g. 'AD', ' A', 'M ', '??' - LinesDeleted int - LinesAdded int + // How long the conflict markers in this file are, taken from its + // conflict-marker-size gitattribute; 0 if it doesn't have that attribute. We + // only look this up for files that have inline merge conflicts. + ConflictMarkerSize int + DisplayString string + ShortStatus string // e.g. 'AD', ' A', 'M ', '??' + LinesDeleted int + LinesAdded int // If true, this must be a worktree folder IsWorktree bool diff --git a/pkg/gui/controllers/files_controller.go b/pkg/gui/controllers/files_controller.go index be8e94477..41f9197fe 100644 --- a/pkg/gui/controllers/files_controller.go +++ b/pkg/gui/controllers/files_controller.go @@ -328,7 +328,7 @@ func (self *FilesController) renderSubmoduleConflict(node *filetree.FileNode) { // (it was resolved in an editor), in which case the caller should fall back to // showing the file's diff. func (self *FilesController) renderInlineMergeConflict(node *filetree.FileNode) bool { - hasConflicts, err := self.c.Helpers().MergeConflicts.SetMergeState(node.GetPath()) + hasConflicts, err := self.c.Helpers().MergeConflicts.SetMergeState(node.File) if err != nil { return true } @@ -1264,7 +1264,7 @@ func (self *FilesController) switchToMerge() error { return nil } - return self.c.Helpers().MergeConflicts.SwitchToMerge(file.Path) + return self.c.Helpers().MergeConflicts.SwitchToMerge(file) } func (self *FilesController) createStashMenu() error { diff --git a/pkg/gui/controllers/helpers/merge_conflicts_helper.go b/pkg/gui/controllers/helpers/merge_conflicts_helper.go index 34ae285f0..3928ecebf 100644 --- a/pkg/gui/controllers/helpers/merge_conflicts_helper.go +++ b/pkg/gui/controllers/helpers/merge_conflicts_helper.go @@ -1,6 +1,7 @@ package helpers import ( + "github.com/jesseduffield/lazygit/pkg/commands/models" "github.com/jesseduffield/lazygit/pkg/gui/context" "github.com/jesseduffield/lazygit/pkg/gui/types" ) @@ -17,14 +18,14 @@ func NewMergeConflictsHelper( } } -func (self *MergeConflictsHelper) SetMergeState(path string) (bool, error) { +func (self *MergeConflictsHelper) SetMergeState(file *models.File) (bool, error) { self.context().GetMutex().Lock() defer self.context().GetMutex().Unlock() - return self.setMergeStateWithoutLock(path) + return self.setMergeStateWithoutLock(file.Path, file.ConflictMarkerSize) } -func (self *MergeConflictsHelper) setMergeStateWithoutLock(path string) (bool, error) { +func (self *MergeConflictsHelper) setMergeStateWithoutLock(path string, markerSize int) (bool, error) { content, err := self.c.Git().File.Cat(path) if err != nil { return false, err @@ -34,7 +35,7 @@ func (self *MergeConflictsHelper) setMergeStateWithoutLock(path string) (bool, e self.context().SetUserScrolling(false) } - self.context().GetState().SetContent(content, path) + self.context().GetState().SetContent(content, path, markerSize) return !self.context().GetState().NoConflicts(), nil } @@ -72,7 +73,8 @@ func (self *MergeConflictsHelper) SetConflictsAndRender() (bool, error) { self.context().GetMutex().Lock() defer self.context().GetMutex().Unlock() - hasConflicts, err := self.setMergeStateWithoutLock(self.context().GetState().GetPath()) + state := self.context().GetState() + hasConflicts, err := self.setMergeStateWithoutLock(state.GetPath(), state.GetMarkerSize()) if err != nil { return false, err } @@ -84,9 +86,9 @@ func (self *MergeConflictsHelper) SetConflictsAndRender() (bool, error) { return false, nil } -func (self *MergeConflictsHelper) SwitchToMerge(path string) error { - if self.context().GetState().GetPath() != path { - hasConflicts, err := self.SetMergeState(path) +func (self *MergeConflictsHelper) SwitchToMerge(file *models.File) error { + if self.context().GetState().GetPath() != file.Path { + hasConflicts, err := self.SetMergeState(file) if err != nil { return err } diff --git a/pkg/gui/controllers/helpers/refresh_helper.go b/pkg/gui/controllers/helpers/refresh_helper.go index d0327216c..53505ba6c 100644 --- a/pkg/gui/controllers/helpers/refresh_helper.go +++ b/pkg/gui/controllers/helpers/refresh_helper.go @@ -1305,7 +1305,8 @@ func (self *RefreshHelper) refreshStateFiles(captured capturedFilesState, env re // process working directory, which may already point at another // repo if the user switched while this refresh was in flight. hasConflicts, err := mergeconflicts.FileHasConflictMarkers( - filepath.Join(env.git.RepoPaths.WorktreePath(), file.Path)) + filepath.Join(env.git.RepoPaths.WorktreePath(), file.Path), + file.ConflictMarkerSize) if err != nil { self.c.Log.Error(err) } else if !hasConflicts { diff --git a/pkg/gui/filetree/file_node.go b/pkg/gui/filetree/file_node.go index 0836eaf02..04c98f1fa 100644 --- a/pkg/gui/filetree/file_node.go +++ b/pkg/gui/filetree/file_node.go @@ -51,7 +51,7 @@ func (self *FileNode) GetHasInlineMergeConflicts() bool { if !file.HasInlineMergeConflicts { return false } - hasConflicts, _ := mergeconflicts.FileHasConflictMarkers(file.Path) + hasConflicts, _ := mergeconflicts.FileHasConflictMarkers(file.Path, file.ConflictMarkerSize) return hasConflicts }) } diff --git a/pkg/gui/mergeconflicts/find_conflicts.go b/pkg/gui/mergeconflicts/find_conflicts.go index 5fe45624e..a296baf74 100644 --- a/pkg/gui/mergeconflicts/find_conflicts.go +++ b/pkg/gui/mergeconflicts/find_conflicts.go @@ -2,7 +2,6 @@ package mergeconflicts import ( "bufio" - "bytes" "io" "os" "strings" @@ -22,7 +21,23 @@ const ( NOT_A_MARKER ) -func findConflicts(content string) []*mergeConflict { +// The number of characters a conflict marker consists of, unless the file's +// conflict-marker-size gitattribute says otherwise. +const defaultConflictMarkerSize = 7 + +// The marker size that everything in here takes is the conflict-marker-size +// gitattribute of the file being examined, which is 0 for a file that doesn't +// have that attribute. Git falls back to its default size in that case, so we +// do the same. +func effectiveMarkerSize(markerSize int) int { + if markerSize < 1 { + return defaultConflictMarkerSize + } + + return markerSize +} + +func findConflicts(content string, markerSize int) []*mergeConflict { conflicts := make([]*mergeConflict, 0) if content == "" { @@ -31,7 +46,7 @@ func findConflicts(content string) []*mergeConflict { var newConflict *mergeConflict for i, line := range utils.SplitLines(content) { - switch determineLineType(line) { + switch determineLineType(line, markerSize) { case START: newConflict = &mergeConflict{start: i, ancestor: -1} case ANCESTOR: @@ -57,35 +72,57 @@ func findConflicts(content string) []*mergeConflict { return conflicts } -var ( - CONFLICT_START = "<<<<<<< " - CONFLICT_END = ">>>>>>> " - CONFLICT_START_BYTES = []byte(CONFLICT_START) - CONFLICT_END_BYTES = []byte(CONFLICT_END) -) +func determineLineType(line string, markerSize int) LineType { + markerSize = effectiveMarkerSize(markerSize) -func determineLineType(line string) LineType { // TODO: find out whether we ever actually get this prefix trimmedLine := strings.TrimPrefix(line, "++") switch { - case strings.HasPrefix(trimmedLine, CONFLICT_START): + case isConflictMarker(trimmedLine, '<', markerSize): return START - case strings.HasPrefix(trimmedLine, "||||||| "): + case isConflictMarker(trimmedLine, '|', markerSize): return ANCESTOR - case trimmedLine == "=======": + case isTargetMarker(trimmedLine, markerSize): return TARGET - case strings.HasPrefix(trimmedLine, CONFLICT_END): + case isConflictMarker(trimmedLine, '>', markerSize): return END default: return NOT_A_MARKER } } +// Tells us whether the line begins with markerSize repetitions of markerChar. +func hasMarkerPrefix[T string | []byte](line T, markerChar byte, markerSize int) bool { + if len(line) < markerSize { + return false + } + + for i := range markerSize { + if line[i] != markerChar { + return false + } + } + + return true +} + +// A start, ancestor or end marker is followed by a space and a label, e.g. +// "<<<<<<< HEAD". +func isConflictMarker[T string | []byte](line T, markerChar byte, markerSize int) bool { + return hasMarkerPrefix(line, markerChar, markerSize) && + len(line) > markerSize && line[markerSize] == ' ' +} + +// The marker separating the two sides of a conflict never has a label after it. +func isTargetMarker(line string, markerSize int) bool { + return hasMarkerPrefix(line, '=', markerSize) && len(line) == markerSize +} + // tells us whether a file actually has inline merge conflicts. We need to run this // because git will continue showing a status of 'UU' even after the conflicts have // been resolved in the user's editor -func FileHasConflictMarkers(path string) (bool, error) { +func FileHasConflictMarkers(path string, markerSize int) (bool, error) { file, err := os.Open(path) if err != nil { return false, err @@ -93,22 +130,20 @@ func FileHasConflictMarkers(path string) (bool, error) { defer file.Close() - return fileHasConflictMarkersAux(file) + return fileHasConflictMarkersAux(file, markerSize) } // Efficiently scans through a file looking for merge conflict markers. Returns true if it does -func fileHasConflictMarkersAux(file io.Reader) (bool, error) { +func fileHasConflictMarkersAux(file io.Reader, markerSize int) (bool, error) { + markerSize = effectiveMarkerSize(markerSize) + scanner := bufio.NewScanner(file) scanner.Split(utils.ScanLinesAndTruncateWhenLongerThanBuffer(bufio.MaxScanTokenSize)) for scanner.Scan() { line := scanner.Bytes() // only searching for start/end markers because the others are more ambiguous - if bytes.HasPrefix(line, CONFLICT_START_BYTES) { - return true, nil - } - - if bytes.HasPrefix(line, CONFLICT_END_BYTES) { + if isConflictMarker(line, '<', markerSize) || isConflictMarker(line, '>', markerSize) { return true, nil } } diff --git a/pkg/gui/mergeconflicts/find_conflicts_test.go b/pkg/gui/mergeconflicts/find_conflicts_test.go index c763aa51f..97a61a3c4 100644 --- a/pkg/gui/mergeconflicts/find_conflicts_test.go +++ b/pkg/gui/mergeconflicts/find_conflicts_test.go @@ -8,9 +8,12 @@ import ( ) func TestDetermineLineType(t *testing.T) { + // A markerSize of 0 means the file has no conflict-marker-size gitattribute, + // so git's default size applies. type scenario struct { - line string - expected LineType + line string + markerSize int + expected LineType } scenarios := []scenario{ @@ -54,17 +57,62 @@ func TestDetermineLineType(t *testing.T) { line: "||||||| adf33b9", expected: ANCESTOR, }, + { + line: "<<<<<<<<", + expected: NOT_A_MARKER, + }, + { + line: strings.Repeat("<", 32) + " HEAD", + markerSize: 32, + expected: START, + }, + { + line: strings.Repeat("|", 32) + " adf33b9", + markerSize: 32, + expected: ANCESTOR, + }, + { + line: strings.Repeat("=", 32), + markerSize: 32, + expected: TARGET, + }, + { + line: strings.Repeat(">", 32) + " blah", + markerSize: 32, + expected: END, + }, + // A file gets a bigger marker size precisely because its regular content + // tends to contain marker-looking lines, so lines with the default size + // must not be mistaken for markers + { + line: "<<<<<<< HEAD", + markerSize: 32, + expected: NOT_A_MARKER, + }, + { + line: "=======", + markerSize: 32, + expected: NOT_A_MARKER, + }, + { + line: strings.Repeat("=", 33), + markerSize: 32, + expected: NOT_A_MARKER, + }, } for _, s := range scenarios { - assert.EqualValues(t, s.expected, determineLineType(s.line)) + assert.EqualValues(t, s.expected, determineLineType(s.line, s.markerSize), s.line) } } func TestFindConflictsAux(t *testing.T) { + // A markerSize of 0 means the file has no conflict-marker-size gitattribute, + // so git's default size applies. type scenario struct { - content string - expected bool + content string + markerSize int + expected bool } scenarios := []scenario{ @@ -92,12 +140,28 @@ func TestFindConflictsAux(t *testing.T) { content: "a\nb\nc\n<<<<<<< ", expected: true, }, + { + content: "a\nb\nc\n" + strings.Repeat("<", 32) + " HEAD", + markerSize: 32, + expected: true, + }, + { + content: "a\nb\nc\n" + strings.Repeat(">", 32) + " blah", + markerSize: 32, + expected: true, + }, + // Marker-looking lines of the default size are the file's regular content + { + content: "a\nb\nc\n<<<<<<< HEAD\n=======\n>>>>>>> blah", + markerSize: 32, + expected: false, + }, } for _, s := range scenarios { reader := strings.NewReader(s.content) - result, err := fileHasConflictMarkersAux(reader) + result, err := fileHasConflictMarkersAux(reader, s.markerSize) assert.NoError(t, err) - assert.EqualValues(t, s.expected, result) + assert.EqualValues(t, s.expected, result, s.content) } } diff --git a/pkg/gui/mergeconflicts/state.go b/pkg/gui/mergeconflicts/state.go index 047241353..d38e0c754 100644 --- a/pkg/gui/mergeconflicts/state.go +++ b/pkg/gui/mergeconflicts/state.go @@ -12,6 +12,9 @@ type State struct { // path of the file with the conflicts path string + // the file's conflict-marker-size gitattribute, or 0 if it doesn't have one + markerSize int + // This is a stack of the file content. It is used to undo changes. // The last item is the current file content. contents []string @@ -74,12 +77,13 @@ func (s *State) currentConflict() *mergeConflict { } // this is for starting a new merge conflict session -func (s *State) SetContent(content string, path string) { - if content == s.GetContent() && path == s.path { +func (s *State) SetContent(content string, path string, markerSize int) { + if content == s.GetContent() && path == s.path && markerSize == s.markerSize { return } s.path = path + s.markerSize = markerSize s.contents = []string{} s.PushContent(content) } @@ -88,7 +92,7 @@ func (s *State) SetContent(content string, path string) { // state func (s *State) PushContent(content string) { s.contents = append(s.contents, content) - s.setConflicts(findConflicts(content)) + s.setConflicts(findConflicts(content, s.markerSize)) } func (s *State) GetContent() string { @@ -103,6 +107,10 @@ func (s *State) GetPath() string { return s.path } +func (s *State) GetMarkerSize() int { + return s.markerSize +} + func (s *State) Undo() bool { if len(s.contents) <= 1 { return false @@ -112,7 +120,7 @@ func (s *State) Undo() bool { newContent := s.GetContent() // We could be storing the old conflicts and selected index on a stack too. - s.setConflicts(findConflicts(newContent)) + s.setConflicts(findConflicts(newContent, s.markerSize)) return true } @@ -147,6 +155,7 @@ func (s *State) AllConflictsResolved() bool { func (s *State) Reset() { s.contents = []string{} s.path = "" + s.markerSize = 0 } // we're not resetting selectedIndex here because the user typically would want diff --git a/pkg/gui/mergeconflicts/state_test.go b/pkg/gui/mergeconflicts/state_test.go index 7a9ee8c26..06f8fa6bb 100644 --- a/pkg/gui/mergeconflicts/state_test.go +++ b/pkg/gui/mergeconflicts/state_test.go @@ -116,7 +116,7 @@ baz for _, s := range scenarios { t.Run(s.name, func(t *testing.T) { - assert.EqualValues(t, s.expected, findConflicts(s.content)) + assert.EqualValues(t, s.expected, findConflicts(s.content, defaultConflictMarkerSize)) }) } }