diff options
author | Jesse Duffield <jessedduffield@gmail.com> | 2024-01-28 12:13:42 +1100 |
---|---|---|
committer | GitHub <noreply@github.com> | 2024-01-28 12:13:42 +1100 |
commit | 6322944a5e30fed9bdb817dfb013ac3162d0af18 (patch) | |
tree | a7700e012428037f8f483f19bd9433821ca361ae | |
parent | 9b2a5f636ad51b5244debe5afdc512f94dffe343 (diff) | |
parent | 510f9a1ae1341c42251ece9c1c0bb277b8197cde (diff) |
Support selecting file range in patch builder (#3259)
- **PR Description**
Adds support for selecting a range of files and adding them to a custom
patch. Closes #3251
The behavior for node selection is the same as used in #3248 because I
copied the approach. Please let me know if there's a mismatch or if
something else is preferred.
I also copied `normalisedSelectedNodes` and
`isDescendentOfSelectedNodes` verbatim, just adapted their signature
types.
It seems like we could share those two functions between
`[]*filetree.CommitFileNode` and `[]*filetree.FileNode` by making those
functions like `func normalisedSelectedCommitNodes[T any](selectedNodes
[]*filetree.Node[T]) []*filetree.Node[T]`. That would require calling
them with a `lo.Map(...)` which returns `node.GetRaw()`, and I feel
weird about giving a different type back to the calling function.
I added a couple of test cases, and all of the existing patch tests pass
for me, but please do let me know if there are any other test cases I
should add.
- **Please check if the PR fulfills these requirements**
* [x] Cheatsheets are up-to-date (run `go generate ./...`)
* [x] Code has been formatted (see
[here](https://github.com/jesseduffield/lazygit/blob/master/CONTRIBUTING.md#code-formatting))
* [x] Tests have been added/updated (see
[here](https://github.com/jesseduffield/lazygit/blob/master/pkg/integration/README.md)
for the integration test guide)
* [ ] Text is internationalised (see
[here](https://github.com/jesseduffield/lazygit/blob/master/CONTRIBUTING.md#internationalisation))
* [ ] Docs (specifically `docs/Config.md`) have been updated if
necessary
* [x] You've read through your own file changes for silly mistakes etc
<!--
Be sure to name your PR with an imperative e.g. 'Add worktrees view'
see https://github.com/jesseduffield/lazygit/releases/tag/v0.40.0 for
examples
-->
-rw-r--r-- | pkg/commands/patch/patch_builder.go | 16 | ||||
-rw-r--r-- | pkg/gui/controllers/commits_files_controller.go | 63 | ||||
-rw-r--r-- | pkg/gui/filetree/commit_file_tree_view_model.go | 13 | ||||
-rw-r--r-- | pkg/integration/tests/patch_building/move_range_to_index.go | 70 | ||||
-rw-r--r-- | pkg/integration/tests/patch_building/toggle_range.go | 107 | ||||
-rw-r--r-- | pkg/integration/tests/test_list.go | 2 |
6 files changed, 247 insertions, 24 deletions
diff --git a/pkg/commands/patch/patch_builder.go b/pkg/commands/patch/patch_builder.go index 88f1becc5..0fedacd19 100644 --- a/pkg/commands/patch/patch_builder.go +++ b/pkg/commands/patch/patch_builder.go @@ -80,13 +80,15 @@ func (p *PatchBuilder) PatchToApply(reverse bool) string { } func (p *PatchBuilder) addFileWhole(info *fileInfo) { - info.mode = WHOLE - lineCount := len(strings.Split(info.diff, "\n")) - // 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 + if info.mode != WHOLE { + info.mode = WHOLE + lineCount := len(strings.Split(info.diff, "\n")) + // 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 + } } } diff --git a/pkg/gui/controllers/commits_files_controller.go b/pkg/gui/controllers/commits_files_controller.go index e2ed4b3f0..647d6594b 100644 --- a/pkg/gui/controllers/commits_files_controller.go +++ b/pkg/gui/controllers/commits_files_controller.go @@ -1,6 +1,8 @@ package controllers import ( + "strings" + "github.com/jesseduffield/gocui" "github.com/jesseduffield/lazygit/pkg/commands/git_commands" "github.com/jesseduffield/lazygit/pkg/commands/models" @@ -10,6 +12,7 @@ import ( "github.com/jesseduffield/lazygit/pkg/gui/filetree" "github.com/jesseduffield/lazygit/pkg/gui/types" "github.com/jesseduffield/lazygit/pkg/utils" + "github.com/samber/lo" ) type CommitFilesController struct { @@ -76,8 +79,8 @@ func (self *CommitFilesController) GetKeybindings(opts types.KeybindingsOpts) [] }, { Key: opts.GetKey(opts.Config.Universal.Select), - Handler: self.withItem(self.toggleForPatch), - GetDisabledReason: self.require(self.singleItemSelected()), + Handler: self.withItems(self.toggleForPatch), + GetDisabledReason: self.require(self.itemsSelected()), Description: self.c.Tr.ToggleAddToPatch, Tooltip: utils.ResolvePlaceholderString(self.c.Tr.ToggleAddToPatchTooltip, map[string]string{"doc": constants.Links.Docs.CustomPatchDemo}, @@ -240,7 +243,7 @@ func (self *CommitFilesController) openDiffTool(node *filetree.CommitFileNode) e return err } -func (self *CommitFilesController) toggleForPatch(node *filetree.CommitFileNode) error { +func (self *CommitFilesController) toggleForPatch(selectedNodes []*filetree.CommitFileNode) error { toggle := func() error { return self.c.WithWaitingStatus(self.c.Tr.UpdatingPatch, func(gocui.Task) error { if !self.c.Git().Patch.PatchBuilder.Active() { @@ -249,21 +252,29 @@ func (self *CommitFilesController) toggleForPatch(node *filetree.CommitFileNode) } } - // if there is any file that hasn't been fully added we'll fully add everything, - // otherwise we'll remove everything - adding := node.SomeFile(func(file *models.CommitFile) bool { - return self.c.Git().Patch.PatchBuilder.GetFileStatus(file.Name, self.context().GetRef().RefName()) != patch.WHOLE + selectedNodes = normalisedSelectedCommitFileNodes(selectedNodes) + + // Find if any file in the selection is unselected or partially added + adding := lo.SomeBy(selectedNodes, func(node *filetree.CommitFileNode) bool { + return node.SomeFile(func(file *models.CommitFile) bool { + fileStatus := self.c.Git().Patch.PatchBuilder.GetFileStatus(file.Name, self.context().GetRef().RefName()) + return fileStatus == patch.PART || fileStatus == patch.UNSELECTED + }) }) - err := node.ForEachFile(func(file *models.CommitFile) error { - if adding { - return self.c.Git().Patch.PatchBuilder.AddFileWhole(file.Name) - } else { - return self.c.Git().Patch.PatchBuilder.RemoveFile(file.Name) + patchOperationFunction := self.c.Git().Patch.PatchBuilder.RemoveFile + + if adding { + patchOperationFunction = self.c.Git().Patch.PatchBuilder.AddFileWhole + } + + for _, node := range selectedNodes { + err := node.ForEachFile(func(file *models.CommitFile) error { + return patchOperationFunction(file.Name) + }) + if err != nil { + return self.c.Error(err) } - }) - if err != nil { - return self.c.Error(err) } if self.c.Git().Patch.PatchBuilder.IsEmpty() { @@ -290,7 +301,7 @@ func (self *CommitFilesController) toggleForPatch(node *filetree.CommitFileNode) func (self *CommitFilesController) toggleAllForPatch(_ *filetree.CommitFileNode) error { root := self.context().CommitFileTreeViewModel.GetRoot() - return self.toggleForPatch(root) + return self.toggleForPatch([]*filetree.CommitFileNode{root}) } func (self *CommitFilesController) startPatchBuilder() error { @@ -354,3 +365,23 @@ func (self *CommitFilesController) toggleTreeView() error { return self.c.PostRefreshUpdate(self.context()) } + +// NOTE: these functions are identical to those in files_controller.go (except for types) and +// could also be cleaned up with some generics +func normalisedSelectedCommitFileNodes(selectedNodes []*filetree.CommitFileNode) []*filetree.CommitFileNode { + return lo.Filter(selectedNodes, func(node *filetree.CommitFileNode, _ int) bool { + return !isDescendentOfSelectedCommitFileNodes(node, selectedNodes) + }) +} + +func isDescendentOfSelectedCommitFileNodes(node *filetree.CommitFileNode, selectedNodes []*filetree.CommitFileNode) bool { + for _, selectedNode := range selectedNodes { + selectedNodePath := selectedNode.GetPath() + nodePath := node.GetPath() + + if strings.HasPrefix(nodePath, selectedNodePath) && nodePath != selectedNodePath { + return true + } + } + return false +} diff --git a/pkg/gui/filetree/commit_file_tree_view_model.go b/pkg/gui/filetree/commit_file_tree_view_model.go index 99ed8d477..95cb1a140 100644 --- a/pkg/gui/filetree/commit_file_tree_view_model.go +++ b/pkg/gui/filetree/commit_file_tree_view_model.go @@ -80,7 +80,18 @@ func (self *CommitFileTreeViewModel) GetSelectedItemId() string { } func (self *CommitFileTreeViewModel) GetSelectedItems() ([]*CommitFileNode, int, int) { - panic("Not implemented") + if self.Len() == 0 { + return nil, 0, 0 + } + + startIdx, endIdx := self.GetSelectionRange() + + nodes := []*CommitFileNode{} + for i := startIdx; i <= endIdx; i++ { + nodes = append(nodes, self.Get(i)) + } + + return nodes, startIdx, endIdx } func (self *CommitFileTreeViewModel) GetSelectedItemIds() ([]string, int, int) { diff --git a/pkg/integration/tests/patch_building/move_range_to_index.go b/pkg/integration/tests/patch_building/move_range_to_index.go new file mode 100644 index 000000000..2e7c95e9a --- /dev/null +++ b/pkg/integration/tests/patch_building/move_range_to_index.go @@ -0,0 +1,70 @@ +package patch_building + +import ( + "github.com/jesseduffield/lazygit/pkg/config" + . "github.com/jesseduffield/lazygit/pkg/integration/components" +) + +var MoveRangeToIndex = NewIntegrationTest(NewIntegrationTestArgs{ + Description: "Apply a custom patch", + ExtraCmdArgs: []string{}, + Skip: false, + SetupConfig: func(config *config.AppConfig) {}, + SetupRepo: func(shell *Shell) { + shell.CreateFileAndAdd("file1", "first line\n") + shell.Commit("first commit") + + shell.UpdateFileAndAdd("file1", "first line\nsecond line\n") + shell.CreateFileAndAdd("file2", "file two content\n") + shell.CreateFileAndAdd("file3", "file three content\n") + shell.Commit("second commit") + }, + Run: func(t *TestDriver, keys config.KeybindingConfig) { + t.Views().Commits(). + Focus(). + Lines( + Contains("second commit").IsSelected(), + Contains("first commit"), + ). + PressEnter() + + t.Views().CommitFiles(). + IsFocused(). + Lines( + Contains("M file1").IsSelected(), + Contains("A file2"), + Contains("A file3"), + ). + Press(keys.Universal.ToggleRangeSelect). + NavigateToLine(Contains("file2")). + PressPrimaryAction() + + t.Views().Information().Content(Contains("Building patch")) + + t.Views().PatchBuildingSecondary().Content(Contains("second line")) + t.Views().PatchBuildingSecondary().Content(Contains("file two content")) + + t.Common().SelectPatchOption(MatchesRegexp(`Move patch out into index$`)) + + t.Views().CommitFiles(). + IsFocused(). + Lines( + Contains("file3").IsSelected(), + ).PressEscape() + + t.Views().Files(). + Focus(). + Lines( + Contains("file1").IsSelected(), + Contains("file2"), + ) + + t.Views().Main(). + Content(Contains("second line")) + + t.Views().Files().Focus().NavigateToLine(Contains("file2")) + + t.Views().Main(). + Content(Contains("file two content")) + }, +}) diff --git a/pkg/integration/tests/patch_building/toggle_range.go b/pkg/integration/tests/patch_building/toggle_range.go new file mode 100644 index 000000000..6fd49adcf --- /dev/null +++ b/pkg/integration/tests/patch_building/toggle_range.go @@ -0,0 +1,107 @@ +package patch_building + +import ( + "github.com/jesseduffield/lazygit/pkg/config" + . "github.com/jesseduffield/lazygit/pkg/integration/components" +) + +var ToggleRange = NewIntegrationTest(NewIntegrationTestArgs{ + Description: "Check multi select toggle logic", + ExtraCmdArgs: []string{}, + Skip: false, + SetupConfig: func(config *config.AppConfig) {}, + SetupRepo: func(shell *Shell) { + shell.CreateDir("dir1") + shell.CreateFileAndAdd("dir1/file1-a", "d2f1 first line\nsecond line\nthird line\n") + shell.CreateFileAndAdd("dir1/file2-a", "d1f2 first line\n") + shell.CreateFileAndAdd("dir1/file3-a", "d1f3 first line\n") + + shell.CreateDir("dir2") + shell.CreateFileAndAdd("dir2/file1-b", "d2f1 first line\nsecond line\nthird line\n") + shell.CreateFileAndAdd("dir2/file2-b", "d2f2 first line\n") + shell.CreateFileAndAdd("dir2/file3-b", "d2f3 first line\nsecond line\n") + + shell.Commit("first commit") + }, + Run: func(t *TestDriver, keys config.KeybindingConfig) { + t.Views().Commits(). + Focus(). + Lines( + Contains("first commit").IsSelected(), + ). + PressEnter() + + t.Views().CommitFiles(). + IsFocused(). + Lines( + Contains("▼ dir1").IsSelected(), + Contains(" A").Contains("file1-a"), + Contains(" A").Contains("file2-a"), + Contains(" A").Contains("file3-a"), + Contains("▼ dir2"), + Contains(" A").Contains("file1-b"), + Contains(" A").Contains("file2-b"), + Contains(" A").Contains("file3-b"), + ). + NavigateToLine(Contains("file1-a")). + Press(keys.Universal.ToggleRangeSelect). + NavigateToLine(Contains("file3-a")). + PressPrimaryAction(). + Lines( + Contains("▼ dir1"), + Contains(" ●").Contains("file1-a").IsSelected(), + Contains(" ●").Contains("file2-a").IsSelected(), + Contains(" ●").Contains("file3-a").IsSelected(), + Contains("▼ dir2"), + Contains(" A").Contains("file1-b"), + Contains(" A").Contains("file2-b"), + Contains(" A").Contains("file3-b"), + ). + PressEscape(). + NavigateToLine(Contains("file3-b")). + PressEnter() + + t.Views().Main().IsFocused(). + NavigateToLine(Contains("second line")). + PressPrimaryAction(). + PressEscape() + + t.Views().CommitFiles(). + IsFocused(). + Lines( + Contains("▼ dir1"), + Contains(" ●").Contains("file1-a"), + Contains(" ●").Contains("file2-a"), + Contains(" ●").Contains("file3-a"), + Contains("▼ dir2"), + Contains(" A").Contains("file1-b"), + Contains(" A").Contains("file2-b"), + Contains(" ◐").Contains("file3-b").IsSelected(), + ). + NavigateToLine(Contains("dir1")). + Press(keys.Universal.ToggleRangeSelect). + NavigateToLine(Contains("dir2")). + PressPrimaryAction(). + Lines( + Contains("▼ dir1").IsSelected(), + Contains(" ●").Contains("file1-a").IsSelected(), + Contains(" ●").Contains("file2-a").IsSelected(), + Contains(" ●").Contains("file3-a").IsSelected(), + Contains("▼ dir2").IsSelected(), + Contains(" ●").Contains("file1-b"), + Contains(" ●").Contains("file2-b"), + Contains(" ●").Contains("file3-b"), + ). + PressPrimaryAction(). + Lines( + Contains("▼ dir1").IsSelected(), + Contains(" A").Contains("file1-a").IsSelected(), + Contains(" A").Contains("file2-a").IsSelected(), + Contains(" A").Contains("file3-a").IsSelected(), + Contains("▼ dir2").IsSelected(), + Contains(" A").Contains("file1-b"), + Contains(" A").Contains("file2-b"), + Contains(" A").Contains("file3-b"), + ) + }, +}) diff --git a/pkg/integration/tests/test_list.go b/pkg/integration/tests/test_list.go index f4693faef..9c1f8b5fe 100644 --- a/pkg/integration/tests/test_list.go +++ b/pkg/integration/tests/test_list.go @@ -194,6 +194,7 @@ var tests = []*components.IntegrationTest{ patch_building.Apply, patch_building.ApplyInReverse, patch_building.ApplyInReverseWithConflict, + patch_building.MoveRangeToIndex, patch_building.MoveToEarlierCommit, patch_building.MoveToEarlierCommitNoKeepEmpty, patch_building.MoveToIndex, @@ -209,6 +210,7 @@ var tests = []*components.IntegrationTest{ patch_building.SelectAllFiles, patch_building.SpecificSelection, patch_building.StartNewPatch, + patch_building.ToggleRange, reflog.Checkout, reflog.CherryPick, reflog.DoNotShowBranchMarkersInReflogSubcommits, |