diff --git a/pkg/commands/git.go b/pkg/commands/git.go index 7f95fbafb..d9add2790 100644 --- a/pkg/commands/git.go +++ b/pkg/commands/git.go @@ -117,8 +117,8 @@ func NewGitCommandAux( rebaseCommands := git_commands.NewRebaseCommands(gitCommon, commitCommands, workingTreeCommands) stashCommands := git_commands.NewStashCommands(gitCommon, fileLoader, workingTreeCommands) patchBuilder := patch.NewPatchBuilder(cmn.Log, - func(from string, to string, reverse bool, filename string, plain bool) (string, error) { - return workingTreeCommands.ShowFileDiff(from, to, reverse, filename, plain) + func(from string, to string, reverse bool, filename string, previousPath string, plain bool) (string, error) { + return workingTreeCommands.ShowFileDiff(from, to, reverse, filename, previousPath, plain) }) patchCommands := git_commands.NewPatchCommands(gitCommon, rebaseCommands, commitCommands, statusCommands, stashCommands, patchBuilder) bisectCommands := git_commands.NewBisectCommands(gitCommon) diff --git a/pkg/commands/git_commands/commit_file_loader.go b/pkg/commands/git_commands/commit_file_loader.go index 33bd40e13..9500dabfd 100644 --- a/pkg/commands/git_commands/commit_file_loader.go +++ b/pkg/commands/git_commands/commit_file_loader.go @@ -1,12 +1,12 @@ package git_commands import ( + "fmt" "strings" "github.com/jesseduffield/lazygit/pkg/commands/models" "github.com/jesseduffield/lazygit/pkg/commands/oscommands" "github.com/jesseduffield/lazygit/pkg/common" - "github.com/samber/lo" ) type CommitFileLoader struct { @@ -29,7 +29,7 @@ func (self *CommitFileLoader) GetFilesInDiff(from string, to string, reverse boo Arg("--no-ext-diff"). Arg("--name-status"). Arg("-z"). - Arg("--no-renames"). + Arg(fmt.Sprintf("--find-renames=%d%%", self.UserConfig().Git.RenameSimilarityThreshold)). ArgIf(reverse, "-R"). Arg(from). Arg(to). @@ -44,18 +44,37 @@ func (self *CommitFileLoader) GetFilesInDiff(from string, to string, reverse boo } // filenames string is something like "MM\x00file1\x00MU\x00file2\x00AA\x00file3\x00" -// so we need to split it by the null character and then map each status-name pair to a commit file +// so we need to split it by the null character and then map each status-name pair +// to a commit file. Renames (and copies) are special: their status is followed by +// two paths (the old one and the new one) rather than one, e.g. +// "R100\x00old\x00new\x00". func getCommitFilesFromFilenames(filenames string) []*models.CommitFile { - lines := strings.Split(strings.TrimRight(filenames, "\x00"), "\x00") - if len(lines) == 1 { + fields := strings.Split(strings.TrimRight(filenames, "\x00"), "\x00") + if len(fields) == 1 { return []*models.CommitFile{} } - // typical result looks like 'A my_file' meaning my_file was added - return lo.Map(lo.Chunk(lines, 2), func(chunk []string, _ int) *models.CommitFile { - return &models.CommitFile{ - ChangeStatus: chunk[0], - Path: chunk[1], + commitFiles := make([]*models.CommitFile, 0, len(fields)/2) + for i := 0; i < len(fields)-1; { + changeStatus := fields[i] + if changeStatus[0] == 'R' || changeStatus[0] == 'C' { + // The status has a similarity score appended (e.g. "R100"); drop it + // so the rest of the code only has to deal with a plain "R" or "C". + commitFiles = append(commitFiles, &models.CommitFile{ + ChangeStatus: changeStatus[:1], + PreviousPath: fields[i+1], + Path: fields[i+2], + }) + i += 3 + } else { + // typical result looks like 'A my_file' meaning my_file was added + commitFiles = append(commitFiles, &models.CommitFile{ + ChangeStatus: changeStatus, + Path: fields[i+1], + }) + i += 2 } - }) + } + + return commitFiles } diff --git a/pkg/commands/git_commands/commit_file_loader_test.go b/pkg/commands/git_commands/commit_file_loader_test.go index ec91ec22e..2fa745f8b 100644 --- a/pkg/commands/git_commands/commit_file_loader_test.go +++ b/pkg/commands/git_commands/commit_file_loader_test.go @@ -60,6 +60,25 @@ func TestGetCommitFilesFromFilenames(t *testing.T) { }, }, }, + { + testName: "a rename among regular files", + input: "M\x00Myfile\x00R100\x00before\x00after\x00A\x00Added\x00", + output: []*models.CommitFile{ + { + Path: "Myfile", + ChangeStatus: "M", + }, + { + Path: "after", + PreviousPath: "before", + ChangeStatus: "R", + }, + { + Path: "Added", + ChangeStatus: "A", + }, + }, + }, } for _, test := range tests { diff --git a/pkg/commands/git_commands/working_tree.go b/pkg/commands/git_commands/working_tree.go index 328da1404..8625158ab 100644 --- a/pkg/commands/git_commands/working_tree.go +++ b/pkg/commands/git_commands/working_tree.go @@ -432,8 +432,14 @@ func (self *WorkingTreeCommands) WorktreeFileDiffCmdObj(node models.IFile, plain // ShowFileDiff get the diff of specified from and to. Typically this will be used for a single commit so it'll be 123abc^..123abc // but when we're in diff mode it could be any 'from' to any 'to'. The reverse flag is also here thanks to diff mode. -func (self *WorkingTreeCommands) ShowFileDiff(from string, to string, reverse bool, fileName string, plain bool) (string, error) { - return self.ShowFileDiffCmdObj(from, to, reverse, []string{fileName}, plain).RunWithOutput() +// For a renamed file, previousPath is the path it was renamed from (empty otherwise); +// both paths must be passed to git for the rename to be detected. +func (self *WorkingTreeCommands) ShowFileDiff(from string, to string, reverse bool, fileName string, previousPath string, plain bool) (string, error) { + fileNames := []string{fileName} + if previousPath != "" { + fileNames = append(fileNames, previousPath) + } + return self.ShowFileDiffCmdObj(from, to, reverse, fileNames, plain).RunWithOutput() } func (self *WorkingTreeCommands) ShowFileDiffCmdObj(from string, to string, reverse bool, fileNames []string, plain bool) *oscommands.CmdObj { @@ -454,7 +460,7 @@ func (self *WorkingTreeCommands) ShowFileDiffCmdObj(from string, to string, reve ArgIfElse(useExtDiff || useExtDiffGitConfig, "--ext-diff", "--no-ext-diff"). Arg("--submodule"). Arg(fmt.Sprintf("--unified=%d", contextSize)). - Arg("--no-renames"). + Arg(fmt.Sprintf("--find-renames=%d%%", self.UserConfig().Git.RenameSimilarityThreshold)). Arg(fmt.Sprintf("--color=%s", colorArg)). Arg(from). Arg(to). diff --git a/pkg/commands/git_commands/working_tree_test.go b/pkg/commands/git_commands/working_tree_test.go index bb4c8e750..8af2b707d 100644 --- a/pkg/commands/git_commands/working_tree_test.go +++ b/pkg/commands/git_commands/working_tree_test.go @@ -339,6 +339,8 @@ func TestWorkingTreeShowFileDiff(t *testing.T) { from string to string reverse bool + fileName string + previousPath string plain bool ignoreWhitespace bool contextSize uint64 @@ -353,33 +355,49 @@ func TestWorkingTreeShowFileDiff(t *testing.T) { from: "1234567890", to: "0987654321", reverse: false, + fileName: "test.txt", plain: false, ignoreWhitespace: false, contextSize: 3, runner: oscommands.NewFakeRunner(t). - ExpectGitArgs([]string{"-C", "/path/to/worktree", "-c", "diff.noprefix=false", "diff", "--no-ext-diff", "--submodule", "--unified=3", "--no-renames", "--color=always", "1234567890", "0987654321", "--", "test.txt"}, expectedResult, nil), + ExpectGitArgs([]string{"-C", "/path/to/worktree", "-c", "diff.noprefix=false", "diff", "--no-ext-diff", "--submodule", "--unified=3", "--find-renames=50%", "--color=always", "1234567890", "0987654321", "--", "test.txt"}, expectedResult, nil), }, { testName: "Show diff with custom context size", from: "1234567890", to: "0987654321", reverse: false, + fileName: "test.txt", plain: false, ignoreWhitespace: false, contextSize: 123, runner: oscommands.NewFakeRunner(t). - ExpectGitArgs([]string{"-C", "/path/to/worktree", "-c", "diff.noprefix=false", "diff", "--no-ext-diff", "--submodule", "--unified=123", "--no-renames", "--color=always", "1234567890", "0987654321", "--", "test.txt"}, expectedResult, nil), + ExpectGitArgs([]string{"-C", "/path/to/worktree", "-c", "diff.noprefix=false", "diff", "--no-ext-diff", "--submodule", "--unified=123", "--find-renames=50%", "--color=always", "1234567890", "0987654321", "--", "test.txt"}, expectedResult, nil), }, { testName: "Default case (ignore whitespace)", from: "1234567890", to: "0987654321", reverse: false, + fileName: "test.txt", plain: false, ignoreWhitespace: true, contextSize: 3, runner: oscommands.NewFakeRunner(t). - ExpectGitArgs([]string{"-C", "/path/to/worktree", "-c", "diff.noprefix=false", "diff", "--no-ext-diff", "--submodule", "--unified=3", "--no-renames", "--color=always", "1234567890", "0987654321", "--ignore-all-space", "--", "test.txt"}, expectedResult, nil), + ExpectGitArgs([]string{"-C", "/path/to/worktree", "-c", "diff.noprefix=false", "diff", "--no-ext-diff", "--submodule", "--unified=3", "--find-renames=50%", "--color=always", "1234567890", "0987654321", "--ignore-all-space", "--", "test.txt"}, expectedResult, nil), + }, + { + testName: "Renamed file passes both paths so the rename is detected", + from: "1234567890", + to: "0987654321", + reverse: false, + fileName: "new.txt", + previousPath: "old.txt", + plain: false, + ignoreWhitespace: false, + contextSize: 3, + runner: oscommands.NewFakeRunner(t). + ExpectGitArgs([]string{"-C", "/path/to/worktree", "-c", "diff.noprefix=false", "diff", "--no-ext-diff", "--submodule", "--unified=3", "--find-renames=50%", "--color=always", "1234567890", "0987654321", "--", "new.txt", "old.txt"}, expectedResult, nil), }, } @@ -394,7 +412,7 @@ func TestWorkingTreeShowFileDiff(t *testing.T) { instance := buildWorkingTreeCommands(commonDeps{runner: s.runner, userConfig: userConfig, appState: &config.AppState{}, repoPaths: &repoPaths}) - result, err := instance.ShowFileDiff(s.from, s.to, s.reverse, "test.txt", s.plain) + result, err := instance.ShowFileDiff(s.from, s.to, s.reverse, s.fileName, s.previousPath, s.plain) assert.NoError(t, err) assert.Equal(t, expectedResult, result) s.runner.CheckForMissingCalls() diff --git a/pkg/commands/models/commit_file.go b/pkg/commands/models/commit_file.go index 90ffd6365..5183fd380 100644 --- a/pkg/commands/models/commit_file.go +++ b/pkg/commands/models/commit_file.go @@ -4,6 +4,9 @@ package models type CommitFile struct { Path string + // For a renamed file, the path it was renamed from; empty otherwise. + PreviousPath string + ChangeStatus string // e.g. 'A' for added or 'M' for modified. This is based on the result from git diff --name-status } @@ -23,6 +26,24 @@ func (f *CommitFile) Deleted() bool { return f.ChangeStatus == "D" } +func (f *CommitFile) IsRename() bool { + return f.PreviousPath != "" +} + +// Names returns an array containing just the path, or in the case of a rename, +// the after path and the before path. +func (f *CommitFile) Names() []string { + result := []string{f.Path} + if f.PreviousPath != "" { + result = append(result, f.PreviousPath) + } + return result +} + func (f *CommitFile) GetPath() string { return f.Path } + +func (f *CommitFile) GetPreviousPath() string { + return f.PreviousPath +} diff --git a/pkg/commands/patch/patch_builder.go b/pkg/commands/patch/patch_builder.go index db834ba16..b730d9f62 100644 --- a/pkg/commands/patch/patch_builder.go +++ b/pkg/commands/patch/patch_builder.go @@ -25,10 +25,14 @@ type fileInfo struct { mode PatchStatus includedLineIndices []int diff string + // For a renamed file, the path it was renamed from; empty otherwise. We + // need to keep hold of it so we can re-render the file's patch (which is + // keyed by the new path) without the caller having to supply it again. + previousPath string } type ( - loadFileDiffFunc func(from string, to string, reverse bool, filename string, plain bool) (string, error) + loadFileDiffFunc func(from string, to string, reverse bool, filename string, previousPath string, plain bool) (string, error) ) // PatchBuilder manages the building of a patch for a commit to be applied to another commit (or the working tree, or removed from the current commit). We also support building patches from things like stashes, for which there is less flexibility @@ -75,6 +79,7 @@ func (p *PatchBuilder) PatchToApply(reverse bool, turnAddedFilesIntoDiffAgainstE patch.WriteString(p.RenderPatchForFile(RenderPatchForFileOpts{ Filename: filename, + PreviousPath: info.previousPath, Plain: true, Reverse: reverse, TurnAddedFilesIntoDiffAgainstEmptyFile: turnAddedFilesIntoDiffAgainstEmptyFile, @@ -102,8 +107,8 @@ func (p *PatchBuilder) removeFile(info *fileInfo) { info.includedLineIndices = nil } -func (p *PatchBuilder) AddFileWhole(filename string) error { - info, err := p.getFileInfo(filename) +func (p *PatchBuilder) AddFileWhole(filename string, previousPath string) error { + info, err := p.getFileInfo(filename, previousPath) if err != nil { return err } @@ -113,8 +118,8 @@ func (p *PatchBuilder) AddFileWhole(filename string) error { return nil } -func (p *PatchBuilder) RemoveFile(filename string) error { - info, err := p.getFileInfo(filename) +func (p *PatchBuilder) RemoveFile(filename string, previousPath string) error { + info, err := p.getFileInfo(filename, previousPath) if err != nil { return err } @@ -124,19 +129,20 @@ func (p *PatchBuilder) RemoveFile(filename string) error { return nil } -func (p *PatchBuilder) getFileInfo(filename string) (*fileInfo, error) { +func (p *PatchBuilder) getFileInfo(filename string, previousPath string) (*fileInfo, error) { info, ok := p.fileInfoMap[filename] if ok { return info, nil } - diff, err := p.loadFileDiff(p.From, p.To, p.reverse, filename, true) + diff, err := p.loadFileDiff(p.From, p.To, p.reverse, filename, previousPath, true) if err != nil { return nil, err } info = &fileInfo{ - mode: UNSELECTED, - diff: diff, + mode: UNSELECTED, + diff: diff, + previousPath: previousPath, } p.fileInfoMap[filename] = info @@ -144,8 +150,8 @@ func (p *PatchBuilder) getFileInfo(filename string) (*fileInfo, error) { return info, nil } -func (p *PatchBuilder) AddFileLineRange(filename string, lineIndices []int) error { - info, err := p.getFileInfo(filename) +func (p *PatchBuilder) AddFileLineRange(filename string, previousPath string, lineIndices []int) error { + info, err := p.getFileInfo(filename, previousPath) if err != nil { return err } @@ -155,8 +161,8 @@ func (p *PatchBuilder) AddFileLineRange(filename string, lineIndices []int) erro return nil } -func (p *PatchBuilder) RemoveFileLineRange(filename string, lineIndices []int) error { - info, err := p.getFileInfo(filename) +func (p *PatchBuilder) RemoveFileLineRange(filename string, previousPath string, lineIndices []int) error { + info, err := p.getFileInfo(filename, previousPath) if err != nil { return err } @@ -171,13 +177,14 @@ func (p *PatchBuilder) RemoveFileLineRange(filename string, lineIndices []int) e type RenderPatchForFileOpts struct { Filename string + PreviousPath string Plain bool Reverse bool TurnAddedFilesIntoDiffAgainstEmptyFile bool } func (p *PatchBuilder) RenderPatchForFile(opts RenderPatchForFileOpts) string { - info, err := p.getFileInfo(opts.Filename) + info, err := p.getFileInfo(opts.Filename, opts.PreviousPath) if err != nil { p.Log.Error(err) return "" @@ -198,7 +205,12 @@ func (p *PatchBuilder) RenderPatchForFile(opts RenderPatchForFileOpts) string { Transform(TransformOpts{ Reverse: opts.Reverse, TurnAddedFilesIntoDiffAgainstEmptyFile: opts.TurnAddedFilesIntoDiffAgainstEmptyFile, - IncludedLineIndices: info.includedLineIndices, + // For a partial selection of a renamed file we keep only the + // content change and drop the rename, so that the rename stays in + // the commit. A whole-file selection keeps the rename (and short- + // circuits before this for plain output). + StripRename: info.mode == PART && info.previousPath != "", + IncludedLineIndices: info.includedLineIndices, }) if opts.Plain { @@ -215,6 +227,7 @@ func (p *PatchBuilder) renderEachFilePatch(plain bool) []string { patches := lo.Map(filenames, func(filename string, _ int) string { return p.RenderPatchForFile(RenderPatchForFileOpts{ Filename: filename, + PreviousPath: p.fileInfoMap[filename].previousPath, Plain: plain, Reverse: false, TurnAddedFilesIntoDiffAgainstEmptyFile: true, @@ -244,8 +257,8 @@ func (p *PatchBuilder) GetFileStatus(filename string, parent string) PatchStatus return info.mode } -func (p *PatchBuilder) GetFileIncLineIndices(filename string) ([]int, error) { - info, err := p.getFileInfo(filename) +func (p *PatchBuilder) GetFileIncLineIndices(filename string, previousPath string) ([]int, error) { + info, err := p.getFileInfo(filename, previousPath) if err != nil { return nil, err } diff --git a/pkg/commands/patch/patch_test.go b/pkg/commands/patch/patch_test.go index 91a65db68..4f84041d6 100644 --- a/pkg/commands/patch/patch_test.go +++ b/pkg/commands/patch/patch_test.go @@ -19,6 +19,22 @@ index dcd3485..1ba5540 100644 ... ` +const renameWithModificationDiff = `diff --git a/oldname b/newname +similarity index 62% +rename from oldname +rename to newname +index dcd3485..1ba5540 100644 +--- a/oldname ++++ b/newname +@@ -1,5 +1,5 @@ + apple +-orange ++grape + ... + ... + ... +` + const addNewlineToEndOfFile = `diff --git a/filename b/filename index 80a73f1..e48a11c 100644 --- a/filename @@ -152,6 +168,7 @@ func TestTransform(t *testing.T) { firstLineIndex int lastLineIndex int reverse bool + stripRename bool expected string } @@ -515,6 +532,43 @@ func TestTransform(t *testing.T) { orange banana lemon +`, + }, + { + testName: "renamed file, whole change selected, strips the rename so only the content change is applied", + firstLineIndex: 9, + lastLineIndex: 10, + stripRename: true, + diffText: renameWithModificationDiff, + expected: `diff --git a/newname b/newname +index dcd3485..1ba5540 100644 +--- a/newname ++++ b/newname +@@ -1,5 +1,5 @@ + apple +-orange ++grape + ... + ... + ... +`, + }, + { + testName: "renamed file, only removal selected, strips the rename", + firstLineIndex: 9, + lastLineIndex: 9, + stripRename: true, + diffText: renameWithModificationDiff, + expected: `diff --git a/newname b/newname +index dcd3485..1ba5540 100644 +--- a/newname ++++ b/newname +@@ -1,5 +1,4 @@ + apple +-orange + ... + ... + ... `, }, } @@ -527,6 +581,7 @@ func TestTransform(t *testing.T) { Transform(TransformOpts{ Reverse: s.reverse, FileNameOverride: s.filename, + StripRename: s.stripRename, IncludedLineIndices: lineIndices, }). FormatPlain() diff --git a/pkg/commands/patch/transform.go b/pkg/commands/patch/transform.go index cdf453939..31456bfc7 100644 --- a/pkg/commands/patch/transform.go +++ b/pkg/commands/patch/transform.go @@ -33,6 +33,13 @@ type TransformOpts struct { // treat it as a diff against an empty file. TurnAddedFilesIntoDiffAgainstEmptyFile bool + // When building a partial patch for a renamed file, strip the rename + // metadata from the header and point it at the new path. Applying the + // resulting patch then only changes the file's contents and leaves the + // rename itself in place. (For a whole-file selection we keep the rename + // so that it moves or is discarded together with the contents.) + StripRename bool + // The indices of lines that should be included in the patch. IncludedLineIndices []int } @@ -72,21 +79,61 @@ func (self *patchTransformer) transformHeader() []string { "--- a/" + self.opts.FileNameOverride, "+++ b/" + self.opts.FileNameOverride, } - } else if self.opts.TurnAddedFilesIntoDiffAgainstEmptyFile { - result := make([]string, 0, len(self.patch.header)) - for idx, line := range self.patch.header { + } + + header := self.patch.header + if self.opts.StripRename { + header = stripRenameFromHeader(header) + } + + if self.opts.TurnAddedFilesIntoDiffAgainstEmptyFile { + result := make([]string, 0, len(header)) + for idx, line := range header { if strings.HasPrefix(line, "new file mode") { continue } - if line == "--- /dev/null" && strings.HasPrefix(self.patch.header[idx+1], "+++ b/") { - line = "--- a/" + self.patch.header[idx+1][6:] + if line == "--- /dev/null" && strings.HasPrefix(header[idx+1], "+++ b/") { + line = "--- a/" + header[idx+1][6:] } result = append(result, line) } return result } - return self.patch.header + return header +} + +// stripRenameFromHeader rewrites a rename diff header so that it looks like a +// plain modification of the new path: it drops the rename metadata and points +// the diff at the new path on both sides, while keeping the blob index line so +// that `git apply --3way` can still fall back to a blob merge. See the +// StripRename option for why we do this. +func stripRenameFromHeader(header []string) []string { + newPath := "" + for _, line := range header { + if path, ok := strings.CutPrefix(line, "+++ b/"); ok { + newPath = path + break + } + } + + result := make([]string, 0, len(header)) + for _, line := range header { + switch { + case strings.HasPrefix(line, "similarity index "), + strings.HasPrefix(line, "dissimilarity index "), + strings.HasPrefix(line, "rename from "), + strings.HasPrefix(line, "rename to "): + // drop the rename metadata + case strings.HasPrefix(line, "diff --git "): + result = append(result, "diff --git a/"+newPath+" b/"+newPath) + case strings.HasPrefix(line, "--- "): + result = append(result, "--- a/"+newPath) + default: + result = append(result, line) + } + } + return result } func (self *patchTransformer) transformHunks() []*Hunk { diff --git a/pkg/gui/context/setup.go b/pkg/gui/context/setup.go index 8f498e6a9..ef1211313 100644 --- a/pkg/gui/context/setup.go +++ b/pkg/gui/context/setup.go @@ -60,8 +60,11 @@ func NewContextTree(c *ContextCommon) *ContextTree { "main", PATCH_BUILDING_MAIN_CONTEXT_KEY, func() []int { - filename := commitFilesContext.GetSelectedPath() - includedLineIndices, err := c.Git().Patch.PatchBuilder.GetFileIncLineIndices(filename) + file := commitFilesContext.GetSelectedFile() + if file == nil { + return nil + } + includedLineIndices, err := c.Git().Patch.PatchBuilder.GetFileIncLineIndices(file.Path, file.PreviousPath) if err != nil { c.Log.Error(err) return nil diff --git a/pkg/gui/controllers/commits_files_controller.go b/pkg/gui/controllers/commits_files_controller.go index eed9d02b9..f9fda0b93 100644 --- a/pkg/gui/controllers/commits_files_controller.go +++ b/pkg/gui/controllers/commits_files_controller.go @@ -191,11 +191,11 @@ func (self *CommitFilesController) GetOnRenderToMain() func() { } } -func (self *CommitFilesController) copyDiffToClipboard(path string, toastMessage string) error { +func (self *CommitFilesController) copyDiffToClipboard(paths []string, toastMessage string) error { from, to := self.context().GetFromAndToForDiff() from, reverse := self.c.Modes().Diffing.GetFromAndReverseArgsForDiff(from) - cmdObj := self.c.Git().WorkingTree.ShowFileDiffCmdObj(from, to, reverse, []string{path}, true) + cmdObj := self.c.Git().WorkingTree.ShowFileDiffCmdObj(from, to, reverse, paths, true) diff, err := cmdObj.RunWithOutput() if err != nil { return err @@ -263,7 +263,7 @@ func (self *CommitFilesController) openCopyMenu() error { copyFileDiffItem := &types.MenuItem{ Label: self.c.Tr.CopySelectedDiff, OnPress: func() error { - return self.copyDiffToClipboard(node.GetPath(), self.c.Tr.FileDiffCopiedToast) + return self.copyDiffToClipboard(self.pathsForDiff(node), self.c.Tr.FileDiffCopiedToast) }, DisabledReason: self.require(self.singleItemSelected())(), Keys: menuKey('s'), @@ -271,7 +271,7 @@ func (self *CommitFilesController) openCopyMenu() error { copyAllDiff := &types.MenuItem{ Label: self.c.Tr.CopyAllFilesDiff, OnPress: func() error { - return self.copyDiffToClipboard(".", self.c.Tr.AllFilesDiffCopiedToast) + return self.copyDiffToClipboard([]string{"."}, self.c.Tr.AllFilesDiffCopiedToast) }, DisabledReason: self.require(self.itemsSelected())(), Keys: menuKey('a'), @@ -348,7 +348,10 @@ func (self *CommitFilesController) discard(selectedNodes []*filetree.CommitFileN for _, node := range selectedNodes { _ = node.ForEachFile(func(file *models.CommitFile) error { - filePaths = append(filePaths, file.GetPath()) + // For a rename we discard both the new and the old path, + // so that the new file is removed and the old one is + // restored. + filePaths = append(filePaths, file.Names()...) return nil }) } @@ -465,7 +468,7 @@ func (self *CommitFilesController) toggleForPatch(selectedNodes []*filetree.Comm for _, node := range selectedNodes { err := node.ForEachFile(func(file *models.CommitFile) error { - return patchOperationFunction(file.Path) + return patchOperationFunction(file.Path, file.PreviousPath) }) if err != nil { return err @@ -610,11 +613,16 @@ func (self *CommitFilesController) pathsForDiff(node *filetree.CommitFileNode) [ if !node.IsFile() && self.context().IsFiltering() { var paths []string _ = node.ForEachFile(func(file *models.CommitFile) error { - paths = append(paths, file.Path) + // For a rename we need to pass both paths so that git detects it as + // a rename rather than an unrelated delete and add. + paths = append(paths, file.Names()...) return nil }) return paths } + if file := node.GetFile(); file != nil { + return file.Names() + } return []string{node.GetPath()} } diff --git a/pkg/gui/controllers/helpers/patch_building_helper.go b/pkg/gui/controllers/helpers/patch_building_helper.go index 9369cca93..ac79ee8d7 100644 --- a/pkg/gui/controllers/helpers/patch_building_helper.go +++ b/pkg/gui/controllers/helpers/patch_building_helper.go @@ -66,20 +66,21 @@ func (self *PatchBuildingHelper) RefreshPatchBuildingPanel(opts types.OnFocusOpt } // get diff from commit file that's currently selected - path := self.c.Contexts().CommitFiles.GetSelectedPath() - if path == "" { + file := self.c.Contexts().CommitFiles.GetSelectedFile() + if file == nil { return } from, to := self.c.Contexts().CommitFiles.GetFromAndToForDiff() from, reverse := self.c.Modes().Diffing.GetFromAndReverseArgsForDiff(from) - diff, err := self.c.Git().WorkingTree.ShowFileDiff(from, to, reverse, path, true) + diff, err := self.c.Git().WorkingTree.ShowFileDiff(from, to, reverse, file.Path, file.PreviousPath, true) if err != nil { return } secondaryDiff := self.c.Git().Patch.PatchBuilder.RenderPatchForFile(patch.RenderPatchForFileOpts{ - Filename: path, + Filename: file.Path, + PreviousPath: file.PreviousPath, Plain: false, Reverse: false, TurnAddedFilesIntoDiffAgainstEmptyFile: true, diff --git a/pkg/gui/controllers/patch_building_controller.go b/pkg/gui/controllers/patch_building_controller.go index dd8c89fff..5e4a17169 100644 --- a/pkg/gui/controllers/patch_building_controller.go +++ b/pkg/gui/controllers/patch_building_controller.go @@ -138,8 +138,8 @@ func (self *PatchBuildingController) toggleSelection() error { self.context().GetMutex().Lock() defer self.context().GetMutex().Unlock() - filename := self.c.Contexts().CommitFiles.GetSelectedPath() - if filename == "" { + file := self.c.Contexts().CommitFiles.GetSelectedFile() + if file == nil { return nil } @@ -152,7 +152,7 @@ func (self *PatchBuildingController) toggleSelection() error { return nil } - includedLineIndices, err := self.c.Git().Patch.PatchBuilder.GetFileIncLineIndices(filename) + includedLineIndices, err := self.c.Git().Patch.PatchBuilder.GetFileIncLineIndices(file.Path, file.PreviousPath) if err != nil { return err } @@ -164,7 +164,7 @@ func (self *PatchBuildingController) toggleSelection() error { } // add range of lines to those set for the file - if err := toggleFunc(filename, lineIndicesToToggle); err != nil { + if err := toggleFunc(file.Path, file.PreviousPath, lineIndicesToToggle); err != nil { // might actually want to return an error here self.c.Log.Error(err) } diff --git a/pkg/gui/controllers/rename_similarity_threshold_controller.go b/pkg/gui/controllers/rename_similarity_threshold_controller.go index 78b8bb7f4..e9bf13027 100644 --- a/pkg/gui/controllers/rename_similarity_threshold_controller.go +++ b/pkg/gui/controllers/rename_similarity_threshold_controller.go @@ -1,6 +1,7 @@ package controllers import ( + "errors" "fmt" "github.com/jesseduffield/lazygit/pkg/gui/context" @@ -49,6 +50,10 @@ func (self *RenameSimilarityThresholdController) Context() types.Context { } func (self *RenameSimilarityThresholdController) Increase() error { + if err := self.checkCanChangeThreshold(); err != nil { + return err + } + old_size := self.c.UserConfig().Git.RenameSimilarityThreshold if old_size < 100 { @@ -59,6 +64,10 @@ func (self *RenameSimilarityThresholdController) Increase() error { } func (self *RenameSimilarityThresholdController) Decrease() error { + if err := self.checkCanChangeThreshold(); err != nil { + return err + } + old_size := self.c.UserConfig().Git.RenameSimilarityThreshold if old_size > 5 { @@ -73,11 +82,23 @@ func (self *RenameSimilarityThresholdController) applyChange() error { currentContext := self.c.Context().CurrentSide() switch currentContext.GetKey() { - // we make an exception for our files context, because it actually need to refresh its state afterwards. + // we make an exception for the files and commit-files contexts, because + // they actually need to refresh their state afterwards: a changed threshold + // can turn a rename into a separate delete and add, or vice versa. case context.FILES_CONTEXT_KEY: self.c.Refresh(types.RefreshOptions{Scope: []types.RefreshableView{types.FILES}}) + case context.COMMIT_FILES_CONTEXT_KEY: + self.c.Refresh(types.RefreshOptions{Scope: []types.RefreshableView{types.COMMIT_FILES}}) default: currentContext.HandleRenderToMain() } return nil } + +func (self *RenameSimilarityThresholdController) checkCanChangeThreshold() error { + if self.c.Git().Patch.PatchBuilder.Active() { + return errors.New(self.c.Tr.CantChangeRenameThresholdError) + } + + return nil +} diff --git a/pkg/gui/presentation/files.go b/pkg/gui/presentation/files.go index cc0a93889..534229905 100644 --- a/pkg/gui/presentation/files.go +++ b/pkg/gui/presentation/files.go @@ -328,7 +328,8 @@ func fileNameAtDepth(node *filetree.Node[models.File], depth int, showRootItem b func commitFileNameAtDepth(node *filetree.Node[models.CommitFile], depth int) string { splitName := split(node.GetInternalPath()) - if depth == 0 && splitName[0] == "." { + showRootItem := splitName[0] == "." + if depth == 0 && showRootItem { if len(splitName) == 1 { return "/" } @@ -336,6 +337,20 @@ func commitFileNameAtDepth(node *filetree.Node[models.CommitFile], depth int) st } name := join(splitName[depth:]) + if node.File != nil && node.File.IsRename() { + splitPrevName := filetree.SplitFileTreePath(node.File.PreviousPath, showRootItem) + + prevName := node.File.PreviousPath + // if the file has just been renamed inside the same directory, we can shave off + // the prefix for the previous path too. Otherwise we'll keep it unchanged + sameParentDir := len(splitName) == len(splitPrevName) && join(splitName[0:depth]) == join(splitPrevName[0:depth]) + if sameParentDir { + prevName = join(splitPrevName[depth:]) + } + + return prevName + " → " + name + } + return name } diff --git a/pkg/gui/presentation/files_test.go b/pkg/gui/presentation/files_test.go index a5b01c156..c7e333682 100644 --- a/pkg/gui/presentation/files_test.go +++ b/pkg/gui/presentation/files_test.go @@ -151,6 +151,14 @@ func TestRenderCommitFileTree(t *testing.T) { showRootItem: true, expected: []string{"A test"}, }, + { + name: "renamed file", + files: []*models.CommitFile{ + {Path: "new.txt", PreviousPath: "old.txt", ChangeStatus: "R"}, + }, + showRootItem: false, + expected: []string{"R old.txt → new.txt"}, + }, { name: "big example", files: []*models.CommitFile{ @@ -219,7 +227,7 @@ M file1 } patchBuilder := patch.NewPatchBuilder( utils.NewDummyLog(), - func(from string, to string, reverse bool, filename string, plain bool) (string, error) { + func(from string, to string, reverse bool, filename string, previousPath string, plain bool) (string, error) { return "", nil }, ) diff --git a/pkg/i18n/english.go b/pkg/i18n/english.go index be3886fbe..6f18842d5 100644 --- a/pkg/i18n/english.go +++ b/pkg/i18n/english.go @@ -819,6 +819,7 @@ type TranslationSet struct { SortCommits string SortCommitsTooltip string CantChangeContextSizeError string + CantChangeRenameThresholdError string OpenCommitInBrowser string ViewBisectOptions string ConfirmRevertCommit string @@ -1966,6 +1967,7 @@ func EnglishTranslationSet() *TranslationSet { SortCommits: "Commit sort order", SortCommitsTooltip: "Change the sort order of the commits in the commit log.\n\nThe default can be changed in the config file with the key 'git.log.sortOrder'.", CantChangeContextSizeError: "Cannot change context while in patch building mode because we were too lazy to support it when releasing the feature. If you really want it, please let us know!", + CantChangeRenameThresholdError: "Cannot change the rename similarity threshold while in patch building mode, because the custom patch can't cope with a rename turning into a delete and add underneath it.", OpenCommitInBrowser: "Open commit in browser", ViewBisectOptions: "View bisect options", ConfirmRevertCommit: "Are you sure you want to revert {{.selectedCommit}}?", diff --git a/pkg/integration/tests/commit/discard_renamed_file.go b/pkg/integration/tests/commit/discard_renamed_file.go new file mode 100644 index 000000000..01e729425 --- /dev/null +++ b/pkg/integration/tests/commit/discard_renamed_file.go @@ -0,0 +1,57 @@ +package commit + +import ( + "github.com/jesseduffield/lazygit/pkg/config" + . "github.com/jesseduffield/lazygit/pkg/integration/components" +) + +var DiscardRenamedFile = NewIntegrationTest(NewIntegrationTestArgs{ + Description: "Discard a renamed file from an old commit; both the new and the old path are handled so the rename is undone", + ExtraCmdArgs: []string{}, + Skip: false, + SetupConfig: func(config *config.AppConfig) {}, + SetupRepo: func(shell *Shell) { + shell.CreateFileAndAdd("original", "line1\nline2\nline3\nline4\nline5\n") + shell.CreateFileAndAdd("other", "other content\n") + shell.Commit("first commit") + + shell.RenameFileInGit("original", "renamed") + shell.UpdateFileAndAdd("renamed", "line1\nline2 changed\nline3\nline4\nline5\n") + shell.Commit("rename with modification") + }, + Run: func(t *TestDriver, keys config.KeybindingConfig) { + t.Views().Commits(). + Focus(). + Lines( + Contains("rename with modification").IsSelected(), + Contains("first commit"), + ). + PressEnter() + + t.Views().CommitFiles(). + IsFocused(). + Lines( + Contains("original → renamed").IsSelected(), + ). + Press(keys.Universal.Remove) + + t.ExpectPopup().Confirmation(). + Title(Equals("Discard file changes")). + Content(Contains("Are you sure you want to discard changes to the selected file(s) from this commit?")). + Confirm() + + // The rename is undone: the commit no longer touches any file. (If only + // the new path were discarded, the commit would still delete the old + // path and show "D original" here instead.) + t.Views().CommitFiles(). + IsFocused(). + Lines( + Contains("(none)"), + ). + PressEscape() + + // The working tree is clean; the original file is back at HEAD. + t.Views().Files(). + IsEmpty() + }, +}) diff --git a/pkg/integration/tests/patch_building/copy_renamed_file_diff.go b/pkg/integration/tests/patch_building/copy_renamed_file_diff.go new file mode 100644 index 000000000..6343527e5 --- /dev/null +++ b/pkg/integration/tests/patch_building/copy_renamed_file_diff.go @@ -0,0 +1,60 @@ +package patch_building + +import ( + "github.com/jesseduffield/lazygit/pkg/config" + . "github.com/jesseduffield/lazygit/pkg/integration/components" +) + +// note: this is required to simulate the clipboard during CI +func expectClipboard(t *TestDriver, matcher *TextMatcher) { + defer t.Shell().DeleteFile("clipboard") + + t.FileSystem().FileContent("clipboard", matcher) +} + +var CopyRenamedFileDiff = NewIntegrationTest(NewIntegrationTestArgs{ + Description: "Copy the diff of a renamed file to the clipboard; the diff shows the rename rather than a delete and add", + ExtraCmdArgs: []string{}, + Skip: false, + SetupConfig: func(config *config.AppConfig) { + config.GetUserConfig().OS.CopyToClipboardCmd = "printf '%s' {{text}} > clipboard" + }, + SetupRepo: func(shell *Shell) { + shell.CreateFileAndAdd("original", "line1\nline2\nline3\nline4\nline5\n") + shell.Commit("first commit") + + shell.RenameFileInGit("original", "renamed") + shell.UpdateFileAndAdd("renamed", "line1\nline2 changed\nline3\nline4\nline5\n") + shell.Commit("rename with modification") + }, + Run: func(t *TestDriver, keys config.KeybindingConfig) { + t.Views().Commits(). + Focus(). + Lines( + Contains("rename with modification").IsSelected(), + Contains("first commit"), + ). + PressEnter() + + t.Views().CommitFiles(). + IsFocused(). + Lines( + Contains("original → renamed").IsSelected(), + ). + Press(keys.Files.CopyFileInfoToClipboard) + + t.ExpectPopup().Menu(). + Title(Equals("Copy to clipboard")). + Select(Contains("Diff of selected file")). + Confirm() + + t.ExpectToast(Contains("File diff copied to clipboard")) + + expectClipboard(t, + Contains("rename from original"). + Contains("rename to renamed"). + Contains("-line2"). + Contains("+line2 changed"), + ) + }, +}) diff --git a/pkg/integration/tests/patch_building/rename_similarity_threshold_change.go b/pkg/integration/tests/patch_building/rename_similarity_threshold_change.go new file mode 100644 index 000000000..d36e0450b --- /dev/null +++ b/pkg/integration/tests/patch_building/rename_similarity_threshold_change.go @@ -0,0 +1,66 @@ +package patch_building + +import ( + "github.com/jesseduffield/lazygit/pkg/config" + . "github.com/jesseduffield/lazygit/pkg/integration/components" +) + +var RenameSimilarityThresholdChange = NewIntegrationTest(NewIntegrationTestArgs{ + Description: "Changing the rename similarity threshold refreshes the commit files panel, but is disabled while building a patch", + ExtraCmdArgs: []string{}, + Skip: false, + SetupConfig: func(config *config.AppConfig) {}, + SetupRepo: func(shell *Shell) { + shell.CreateFileAndAdd("original", "one\ntwo\nthree\nfour\nfive\n") + shell.Commit("add original") + + shell.RenameFileInGit("original", "renamed") + shell.UpdateFileAndAdd("renamed", "one\ntwo\nthree\nfour\nfive\nsix\nseven\neight\nnine\nten\n") + shell.Commit("change name and contents") + }, + Run: func(t *TestDriver, keys config.KeybindingConfig) { + t.Views().Commits(). + Focus(). + Lines( + Contains("change name and contents").IsSelected(), + Contains("add original"), + ). + PressEnter() + + // At the default threshold of 50% the 50%-similar change is not detected + // as a rename. + t.Views().CommitFiles(). + IsFocused(). + Lines( + Equals("▼ /"), + Equals(" D original"), + Equals(" A renamed"), + ). + // Lowering the threshold turns it into a rename; the panel refreshes. + Press(keys.Universal.DecreaseRenameSimilarityThreshold). + Tap(func() { + t.ExpectToast(Equals("Changed rename similarity threshold to 45%")) + }). + Lines( + Equals("R original → renamed"), + ). + // Start building a patch from the renamed file. + PressPrimaryAction(). + Tap(func() { + t.Views().Information().Content(Contains("Building patch")) + + // Changing the threshold is now disabled: the patch builder + // can't cope with the rename turning into a delete and add. + t.Views().CommitFiles(). + Press(keys.Universal.IncreaseRenameSimilarityThreshold) + t.ExpectPopup().Alert(). + Title(Equals("Error")). + Content(Contains("Cannot change the rename similarity threshold while in patch building mode")). + Confirm() + }). + // The file is unchanged: still a rename, still in the patch. + Lines( + Contains("original → renamed").IsSelected(), + ) + }, +}) diff --git a/pkg/integration/tests/patch_building/renamed_file_partial.go b/pkg/integration/tests/patch_building/renamed_file_partial.go new file mode 100644 index 000000000..3c37c13a2 --- /dev/null +++ b/pkg/integration/tests/patch_building/renamed_file_partial.go @@ -0,0 +1,73 @@ +package patch_building + +import ( + "github.com/jesseduffield/lazygit/pkg/config" + . "github.com/jesseduffield/lazygit/pkg/integration/components" +) + +var RenamedFilePartial = NewIntegrationTest(NewIntegrationTestArgs{ + Description: "Select part of a renamed file's changes into a custom patch and remove it from the commit, keeping the rename in place", + ExtraCmdArgs: []string{}, + Skip: false, + SetupConfig: func(config *config.AppConfig) {}, + SetupRepo: func(shell *Shell) { + shell.CreateFileAndAdd("original", "line1\nline2\nline3\nline4\nline5\n") + shell.Commit("first commit") + + shell.RenameFileInGit("original", "renamed") + shell.UpdateFileAndAdd("renamed", "line1\nline2 changed\nline3\nline4\nline5\n") + shell.Commit("rename with modification") + }, + Run: func(t *TestDriver, keys config.KeybindingConfig) { + t.Views().Commits(). + Focus(). + Lines( + Contains("rename with modification").IsSelected(), + Contains("first commit"), + ). + PressEnter() + + t.Views().CommitFiles(). + IsFocused(). + Lines( + Contains("original → renamed").IsSelected(), + ). + PressEnter() + + // The main view shows the rename together with its content change. + t.Views().PatchBuilding(). + IsFocused(). + Content(Contains("rename from original").Contains("rename to renamed")). + ContainsLines( + Contains(" line1"), + Contains("-line2"), + Contains("+line2 changed"), + Contains(" line3"), + ). + // Add the hunk (a line selection, as opposed to adding the whole + // file), so this is a partial patch. + PressPrimaryAction() + + t.Views().Information().Content(Contains("Building patch")) + + t.Common().SelectPatchOption(Contains("Remove patch from original commit")) + + // The rename is preserved; only the content change is gone, so the file + // is still shown as a rename but now has no content change. + t.Views().CommitFiles(). + IsFocused(). + Lines( + Contains("original → renamed").IsSelected(), + ) + + t.Views().Main(). + Content(DoesNotContain("line2 changed")) + + t.Views().Commits(). + Focus(). + Lines( + Contains("rename with modification").IsSelected(), + Contains("first commit"), + ) + }, +}) diff --git a/pkg/integration/tests/patch_building/renamed_file_whole.go b/pkg/integration/tests/patch_building/renamed_file_whole.go new file mode 100644 index 000000000..f4151a766 --- /dev/null +++ b/pkg/integration/tests/patch_building/renamed_file_whole.go @@ -0,0 +1,62 @@ +package patch_building + +import ( + "github.com/jesseduffield/lazygit/pkg/config" + . "github.com/jesseduffield/lazygit/pkg/integration/components" +) + +var RenamedFileWhole = NewIntegrationTest(NewIntegrationTestArgs{ + Description: "Add a whole renamed file to a custom patch and remove it from the commit, taking the rename with it", + ExtraCmdArgs: []string{}, + Skip: false, + SetupConfig: func(config *config.AppConfig) {}, + SetupRepo: func(shell *Shell) { + shell.CreateFileAndAdd("original", "line1\nline2\nline3\nline4\nline5\n") + shell.Commit("first commit") + + shell.RenameFileInGit("original", "renamed") + shell.UpdateFileAndAdd("renamed", "line1\nline2 changed\nline3\nline4\nline5\n") + shell.Commit("rename with modification") + }, + Run: func(t *TestDriver, keys config.KeybindingConfig) { + t.Views().Commits(). + Focus(). + Lines( + Contains("rename with modification").IsSelected(), + Contains("first commit"), + ). + PressEnter() + + t.Views().CommitFiles(). + IsFocused(). + Lines( + Contains("original → renamed").IsSelected(), + ). + PressPrimaryAction() + + t.Views().Information().Content(Contains("Building patch")) + + // The whole file is added, so the patch carries the rename itself. + t.Views().Secondary(). + ContainsLines( + Contains("rename from original"), + Contains("rename to renamed"), + ) + + t.Common().SelectPatchOption(Contains("Remove patch from original commit")) + + // The rename went with the patch, so the commit no longer touches the file. + t.Views().CommitFiles(). + IsFocused(). + Lines( + Contains("(none)"), + ) + + t.Views().Commits(). + Focus(). + Lines( + Contains("rename with modification").IsSelected(), + Contains("first commit"), + ) + }, +}) diff --git a/pkg/integration/tests/test_list.go b/pkg/integration/tests/test_list.go index 380aca2b6..abf13073e 100644 --- a/pkg/integration/tests/test_list.go +++ b/pkg/integration/tests/test_list.go @@ -128,6 +128,7 @@ var tests = []*components.IntegrationTest{ commit.CreateTag, commit.DisableCopyCommitMessageBody, commit.DiscardOldFileChanges, + commit.DiscardRenamedFile, commit.DiscardSubmoduleChanges, commit.DoNotShowBranchMarkerForHeadCommit, commit.FailHooksThenCommitNoHooks, @@ -351,6 +352,7 @@ var tests = []*components.IntegrationTest{ patch_building.ApplyInReverseWithConflict, patch_building.ApplyWithModifiedFileConflict, patch_building.ApplyWithModifiedFileNoConflict, + patch_building.CopyRenamedFileDiff, patch_building.DiscardLinesFromCommit, patch_building.EditLineInPatchBuildingPanel, patch_building.MoveRangeToIndex, @@ -373,6 +375,9 @@ var tests = []*components.IntegrationTest{ patch_building.MoveToNewCommitPartialHunk, patch_building.RemoveFromCommit, patch_building.RemovePartsOfAddedFile, + patch_building.RenameSimilarityThresholdChange, + patch_building.RenamedFilePartial, + patch_building.RenamedFileWhole, patch_building.ResetWithEscape, patch_building.SelectAllFiles, patch_building.SpecificSelection,