mirror of
https://github.com/jesseduffield/lazygit.git
synced 2026-08-24 10:13:41 -05:00
Keep showing files whose conflicts have been resolved (#5940)
When several files have conflicts, resolving one of them makes it vanish from the files panel as soon as it is auto-staged, and it only comes back once the last conflict is resolved and the filter turns off again. By then it sits among all the other changed files of the merge, so it is hard to find the ones whose resulting diff you still wanted to check. So remember which files had conflicts while the conflicted-files filter is on, and keep showing them once they are resolved. This is the general solution that #5936 called for; that PR only helped for the case of a single conflicted file. The consequence is that the selection no longer moves on to the next conflicted file when one is resolved: it stays on the file you just resolved, which shows you its diff right away.
This commit is contained in:
@@ -87,6 +87,10 @@ while still being meaningful and self-contained.
|
||||
- **Wrap message body to 72 characters**. The subject is allowed to go up to 80
|
||||
characters, or even a little more if needed to convey a good single-line
|
||||
summary; the body should be wrapped at 72 exactly, no more, no less.
|
||||
- **End every commit message with the `Co-authored-by:` trailer** naming the
|
||||
model that wrote it, exactly as your harness instructions spell it. Nothing
|
||||
in `just check` catches a missing one, so it has to be part of writing the
|
||||
message rather than something to notice afterwards.
|
||||
|
||||
## Iterate with `fixup!` commits
|
||||
|
||||
@@ -105,6 +109,16 @@ separate, reviewable commit that the user decides when to fold in. A bare
|
||||
`--amend` rewrites the commit on the spot and skips that checkpoint. Don't
|
||||
treat "I'm only touching the tip commit" as an exception.
|
||||
|
||||
**When the tip is the wrong place for a fixup, insert it mid-branch.**
|
||||
Committing a fixup at the tip of the branch only works while the code it
|
||||
touches still looks the same there; once later commits have rewritten that
|
||||
code — or the target has since been split — the fixup won't apply, and
|
||||
rewriting the later commits to accommodate it defeats the point. Check out the
|
||||
target, make the change, `git commit --fixup=<target>`, then
|
||||
`git rebase --onto <the fixup> <target> <branch>` to replay the rest of the
|
||||
branch. The fixup stays a separate, reviewable commit; only its position
|
||||
changes.
|
||||
|
||||
If the changes don't map cleanly onto existing commits — say they cut
|
||||
across several of them, or restructure something at a different layer
|
||||
than any existing commit naturally owns — stop and ask the user how to
|
||||
|
||||
@@ -1376,12 +1376,9 @@ func (self *RefreshHelper) refreshStateFiles(captured capturedFilesState, env re
|
||||
Background: env.backgroundRoutine,
|
||||
})
|
||||
|
||||
conflictFileCount := 0
|
||||
for _, file := range files {
|
||||
if file.HasMergeConflicts {
|
||||
conflictFileCount++
|
||||
}
|
||||
}
|
||||
conflictedPaths := lo.FilterMap(files, func(file *models.File, _ int) (string, bool) {
|
||||
return file.Path, file.HasMergeConflicts
|
||||
})
|
||||
|
||||
repoState := self.c.State().GetRepoState()
|
||||
workingTreeState := env.git.Status.WorkingTreeState()
|
||||
@@ -1391,7 +1388,7 @@ func (self *RefreshHelper) refreshStateFiles(captured capturedFilesState, env re
|
||||
repoState.SetMergeOrRebaseStartedInLazygit(false)
|
||||
}
|
||||
|
||||
if workingTreeState.Any() && conflictFileCount == 0 {
|
||||
if workingTreeState.Any() && len(conflictedPaths) == 0 {
|
||||
if prevConflictFileCount > 0 && repoState.GetMergeOrRebaseStartedInLazygit() {
|
||||
// The conflicts of an operation we started have just been resolved
|
||||
// (e.g. in the user's editor). Offer to continue it. We only do this
|
||||
@@ -1429,16 +1426,20 @@ func (self *RefreshHelper) refreshStateFiles(captured capturedFilesState, env re
|
||||
|
||||
self.onUIThreadUnlessRepoChanged(env, func() {
|
||||
// only taking over the filter if it hasn't already been set by the user.
|
||||
if conflictFileCount > 0 && prevConflictFileCount == 0 {
|
||||
if len(conflictedPaths) > 0 && prevConflictFileCount == 0 {
|
||||
if fileTreeViewModel.GetStatusFilter() == filetree.DisplayAll {
|
||||
fileTreeViewModel.SetStatusFilter(filetree.DisplayConflicted)
|
||||
self.c.Contexts().Files.GetView().Subtitle = self.c.Tr.FilterLabelConflictingFiles
|
||||
}
|
||||
} else if conflictFileCount == 0 && fileTreeViewModel.GetStatusFilter() == filetree.DisplayConflicted {
|
||||
} else if len(conflictedPaths) == 0 && fileTreeViewModel.GetStatusFilter() == filetree.DisplayConflicted {
|
||||
fileTreeViewModel.SetStatusFilterPreservingSelection(filetree.DisplayAll)
|
||||
self.c.Contexts().Files.GetView().Subtitle = ""
|
||||
}
|
||||
|
||||
if fileTreeViewModel.GetStatusFilter() == filetree.DisplayConflicted {
|
||||
fileTreeViewModel.RememberConflictedPaths(conflictedPaths)
|
||||
}
|
||||
|
||||
self.c.Model().Submodules = submoduleConfigs
|
||||
self.c.Model().Files = files
|
||||
markWorktreeFiles(files, self.c.Model().Worktrees, env.git.RepoPaths.WorktreePath())
|
||||
|
||||
@@ -3,6 +3,7 @@ package filetree
|
||||
import (
|
||||
"fmt"
|
||||
|
||||
"github.com/jesseduffield/generics/set"
|
||||
"github.com/jesseduffield/lazygit/pkg/commands/models"
|
||||
"github.com/jesseduffield/lazygit/pkg/common"
|
||||
"github.com/jesseduffield/lazygit/pkg/gui/types"
|
||||
@@ -42,6 +43,7 @@ type IFileTree interface {
|
||||
|
||||
FilterFiles(test func(*models.File) bool) []*models.File
|
||||
SetStatusFilter(filter FileTreeDisplayFilter)
|
||||
RememberConflictedPaths(paths []string)
|
||||
ForceShowUntracked() bool
|
||||
Get(index int) *FileNode
|
||||
GetFile(path string) *models.File
|
||||
@@ -54,25 +56,31 @@ type IFileTree interface {
|
||||
}
|
||||
|
||||
type FileTree struct {
|
||||
getFiles func() []*models.File
|
||||
tree *Node[models.File]
|
||||
showTree bool
|
||||
common *common.Common
|
||||
filter FileTreeDisplayFilter
|
||||
collapsedPaths *CollapsedPaths
|
||||
textFilter string
|
||||
useFuzzySearch bool
|
||||
getFiles func() []*models.File
|
||||
tree *Node[models.File]
|
||||
showTree bool
|
||||
common *common.Common
|
||||
filter FileTreeDisplayFilter
|
||||
// Paths of the files that had conflicts while the current filter has been
|
||||
// active. The DisplayConflicted filter keeps showing them after their
|
||||
// conflicts have been resolved, so that their diffs can be reviewed while
|
||||
// the remaining files are still being worked on.
|
||||
conflictedPaths *set.Set[string]
|
||||
collapsedPaths *CollapsedPaths
|
||||
textFilter string
|
||||
useFuzzySearch bool
|
||||
}
|
||||
|
||||
var _ IFileTree = &FileTree{}
|
||||
|
||||
func NewFileTree(getFiles func() []*models.File, common *common.Common, showTree bool) *FileTree {
|
||||
return &FileTree{
|
||||
getFiles: getFiles,
|
||||
common: common,
|
||||
showTree: showTree,
|
||||
filter: DisplayAll,
|
||||
collapsedPaths: NewCollapsedPaths(),
|
||||
getFiles: getFiles,
|
||||
common: common,
|
||||
showTree: showTree,
|
||||
filter: DisplayAll,
|
||||
conflictedPaths: set.New[string](),
|
||||
collapsedPaths: NewCollapsedPaths(),
|
||||
}
|
||||
}
|
||||
|
||||
@@ -100,7 +108,9 @@ func (self *FileTree) getFilesForDisplay() []*models.File {
|
||||
case DisplayUntracked:
|
||||
files = self.FilterFiles(func(file *models.File) bool { return !(file.Tracked || file.HasStagedChanges) })
|
||||
case DisplayConflicted:
|
||||
files = self.FilterFiles(func(file *models.File) bool { return file.HasMergeConflicts })
|
||||
files = self.FilterFiles(func(file *models.File) bool {
|
||||
return file.HasMergeConflicts || self.conflictedPaths.Includes(file.Path)
|
||||
})
|
||||
default:
|
||||
panic(fmt.Sprintf("Unexpected files display filter: %d", self.filter))
|
||||
}
|
||||
@@ -122,9 +132,16 @@ func (self *FileTree) FilterFiles(test func(*models.File) bool) []*models.File {
|
||||
|
||||
func (self *FileTree) SetStatusFilter(filter FileTreeDisplayFilter) {
|
||||
self.filter = filter
|
||||
self.conflictedPaths = set.New[string]()
|
||||
self.SetTree()
|
||||
}
|
||||
|
||||
// RememberConflictedPaths records which files have conflicts right now, so that
|
||||
// the DisplayConflicted filter keeps showing them once they are resolved.
|
||||
func (self *FileTree) RememberConflictedPaths(paths []string) {
|
||||
self.conflictedPaths.Add(paths...)
|
||||
}
|
||||
|
||||
func (self *FileTree) ToggleShowTree() {
|
||||
self.showTree = !self.showTree
|
||||
self.SetTree()
|
||||
|
||||
@@ -4,6 +4,7 @@ import (
|
||||
"fmt"
|
||||
"testing"
|
||||
|
||||
"github.com/jesseduffield/generics/set"
|
||||
"github.com/jesseduffield/lazygit/pkg/commands/models"
|
||||
"github.com/jesseduffield/lazygit/pkg/common"
|
||||
"github.com/jesseduffield/lazygit/pkg/config"
|
||||
@@ -12,10 +13,11 @@ import (
|
||||
|
||||
func TestFilterAction(t *testing.T) {
|
||||
scenarios := []struct {
|
||||
name string
|
||||
filter FileTreeDisplayFilter
|
||||
files []*models.File
|
||||
expected []*models.File
|
||||
name string
|
||||
filter FileTreeDisplayFilter
|
||||
conflictedPaths []string
|
||||
files []*models.File
|
||||
expected []*models.File
|
||||
}{
|
||||
{
|
||||
name: "filter files with unstaged changes",
|
||||
@@ -84,11 +86,29 @@ func TestFilterAction(t *testing.T) {
|
||||
{Path: "file1", ShortStatus: "UU", HasMergeConflicts: true, HasInlineMergeConflicts: true},
|
||||
},
|
||||
},
|
||||
{
|
||||
name: "keep showing conflicted files whose conflicts have been resolved",
|
||||
filter: DisplayConflicted,
|
||||
conflictedPaths: []string{"dir2/dir2/file4", "file1"},
|
||||
files: []*models.File{
|
||||
{Path: "dir2/dir2/file4", ShortStatus: "M ", HasStagedChanges: true},
|
||||
{Path: "dir2/file5", ShortStatus: "M ", HasUnstagedChanges: true},
|
||||
{Path: "file1", ShortStatus: "UU", HasMergeConflicts: true, HasInlineMergeConflicts: true},
|
||||
},
|
||||
expected: []*models.File{
|
||||
{Path: "dir2/dir2/file4", ShortStatus: "M ", HasStagedChanges: true},
|
||||
{Path: "file1", ShortStatus: "UU", HasMergeConflicts: true, HasInlineMergeConflicts: true},
|
||||
},
|
||||
},
|
||||
}
|
||||
|
||||
for _, s := range scenarios {
|
||||
t.Run(s.name, func(t *testing.T) {
|
||||
mngr := &FileTree{getFiles: func() []*models.File { return s.files }, filter: s.filter}
|
||||
mngr := &FileTree{
|
||||
getFiles: func() []*models.File { return s.files },
|
||||
filter: s.filter,
|
||||
conflictedPaths: set.NewFromSlice(s.conflictedPaths),
|
||||
}
|
||||
result := mngr.getFilesForDisplay()
|
||||
assert.EqualValues(t, s.expected, result)
|
||||
})
|
||||
|
||||
@@ -7,7 +7,7 @@ import (
|
||||
)
|
||||
|
||||
var ResolveMultipleFiles = NewIntegrationTest(NewIntegrationTestArgs{
|
||||
Description: "Ensures that upon resolving conflicts for one file, the next file is selected",
|
||||
Description: "Ensures that a file whose conflicts have been resolved keeps being shown while other files still have conflicts",
|
||||
ExtraCmdArgs: []string{},
|
||||
Skip: false,
|
||||
SetupConfig: func(config *config.AppConfig) {},
|
||||
@@ -37,11 +37,16 @@ var ResolveMultipleFiles = NewIntegrationTest(NewIntegrationTestArgs{
|
||||
SelectNextItem().
|
||||
PressPrimaryAction()
|
||||
|
||||
// The resolved file is still shown, and stays selected so that its diff
|
||||
// can be reviewed
|
||||
t.Views().Files().
|
||||
IsFocused().
|
||||
Lines(
|
||||
Equals("UU file2").IsSelected(),
|
||||
Equals("▼ /"),
|
||||
Equals(" M file1").IsSelected(),
|
||||
Equals(" UU file2"),
|
||||
).
|
||||
SelectNextItem().
|
||||
PressEnter()
|
||||
|
||||
// coincidentally these files have the same conflict
|
||||
@@ -54,7 +59,14 @@ var ResolveMultipleFiles = NewIntegrationTest(NewIntegrationTestArgs{
|
||||
).
|
||||
PressPrimaryAction()
|
||||
|
||||
t.Views().Files().SelectedLines(Contains("file2"))
|
||||
// Now that all conflicts are resolved, the filter is turned off again
|
||||
t.Views().Files().
|
||||
Lines(
|
||||
Equals("▼ /"),
|
||||
Equals(" M file1"),
|
||||
Equals(" M file2").IsSelected(),
|
||||
Equals(" A file3"),
|
||||
)
|
||||
|
||||
t.Common().ContinueOnConflictsResolved("merge")
|
||||
},
|
||||
|
||||
Reference in New Issue
Block a user