diff --git a/pkg/commands/git_commands/commit.go b/pkg/commands/git_commands/commit.go index 387a1fdc7..c269749ab 100644 --- a/pkg/commands/git_commands/commit.go +++ b/pkg/commands/git_commands/commit.go @@ -6,6 +6,7 @@ import ( "github.com/go-errors/errors" "github.com/jesseduffield/lazygit/pkg/commands/oscommands" + "github.com/samber/lo" ) var ErrInvalidCommitIndex = errors.New("invalid commit index") @@ -155,13 +156,39 @@ func (self *CommitCommands) signoffFlag() string { } func (self *CommitCommands) GetCommitMessage(commitHash string) (string, error) { + messages, err := self.GetCommitMessages([]string{commitHash}) + if err != nil { + return "", err + } + + return messages[0], nil +} + +// GetCommitMessages returns the messages of the given commits, in the order in +// which the hashes were passed in. +func (self *CommitCommands) GetCommitMessages(commitHashes []string) ([]string, error) { cmdArgs := NewGitCmd("log"). - Arg("--format=%B", "--max-count=1", commitHash). + Arg("--no-walk=unsorted", "--format=%B%x00"). + Arg(commitHashes...). Config("log.showsignature=false"). ToArgv() - message, err := self.cmd.New(cmdArgs).DontLog().RunWithOutput() - return strings.ReplaceAll(strings.TrimSpace(message), "\r\n", "\n"), err + output, err := self.cmd.New(cmdArgs).DontLog().RunWithOutput() + if err != nil { + return nil, err + } + + // The messages are NUL-terminated, so the split gives us one more element + // than we asked for (holding the newline that git prints after the last + // message). + messages := strings.Split(output, "\x00") + if len(messages) <= len(commitHashes) { + return nil, errors.New("unexpected output from git log") + } + + return lo.Map(messages[:len(commitHashes)], func(message string, _ int) string { + return strings.ReplaceAll(strings.TrimSpace(message), "\r\n", "\n") + }), nil } func (self *CommitCommands) GetCommitSubject(commitHash string) (string, error) { diff --git a/pkg/commands/git_commands/commit_test.go b/pkg/commands/git_commands/commit_test.go index 6f01b9827..19434d0c8 100644 --- a/pkg/commands/git_commands/commit_test.go +++ b/pkg/commands/git_commands/commit_test.go @@ -378,7 +378,7 @@ func TestGetCommitMsg(t *testing.T) { for _, s := range scenarios { t.Run(s.testName, func(t *testing.T) { instance := buildCommitCommands(commonDeps{ - runner: oscommands.NewFakeRunner(t).ExpectGitArgs([]string{"-c", "log.showsignature=false", "log", "--format=%B", "--max-count=1", "deadbeef"}, s.input, nil), + runner: oscommands.NewFakeRunner(t).ExpectGitArgs([]string{"-c", "log.showsignature=false", "log", "--no-walk=unsorted", "--format=%B%x00", "deadbeef"}, s.input+"\x00\n", nil), }) output, err := instance.GetCommitMessage("deadbeef") @@ -390,6 +390,21 @@ func TestGetCommitMsg(t *testing.T) { } } +func TestGetCommitMessages(t *testing.T) { + instance := buildCommitCommands(commonDeps{ + runner: oscommands.NewFakeRunner(t).ExpectGitArgs( + []string{"-c", "log.showsignature=false", "log", "--no-walk=unsorted", "--format=%B%x00", "deadbeef", "1234567"}, + "first subject\n\nfirst body\n\x00\nsecond subject\n\x00\n", + nil, + ), + }) + + output, err := instance.GetCommitMessages([]string{"deadbeef", "1234567"}) + + assert.NoError(t, err) + assert.Equal(t, []string{"first subject\n\nfirst body", "second subject"}, output) +} + func TestGetCommitMessageFromHistory(t *testing.T) { type scenario struct { testName string @@ -406,7 +421,7 @@ func TestGetCommitMessageFromHistory(t *testing.T) { }, { "Default case to retrieve a commit in history", - oscommands.NewFakeRunner(t).ExpectGitArgs([]string{"log", "-1", "--skip=2", "--pretty=%H"}, "hash3 \n", nil).ExpectGitArgs([]string{"-c", "log.showsignature=false", "log", "--format=%B", "--max-count=1", "hash3"}, `use generics to DRY up context code`, nil), + oscommands.NewFakeRunner(t).ExpectGitArgs([]string{"log", "-1", "--skip=2", "--pretty=%H"}, "hash3 \n", nil).ExpectGitArgs([]string{"-c", "log.showsignature=false", "log", "--no-walk=unsorted", "--format=%B%x00", "hash3"}, "use generics to DRY up context code\x00\n", nil), func(output string, err error) { assert.NoError(t, err) assert.Equal(t, "use generics to DRY up context code", output) diff --git a/pkg/commands/git_commands/deps_test.go b/pkg/commands/git_commands/deps_test.go index 91332cff6..b0f13417a 100644 --- a/pkg/commands/git_commands/deps_test.go +++ b/pkg/commands/git_commands/deps_test.go @@ -121,6 +121,11 @@ func buildSubmoduleCommands(deps commonDeps) *SubmoduleCommands { return NewSubmoduleCommands(gitCommon) } +func buildDiffCommands(deps commonDeps) *DiffCommands { + gitCommon := buildGitCommon(deps) + return NewDiffCommands(gitCommon) +} + func buildCommitCommands(deps commonDeps) *CommitCommands { gitCommon := buildGitCommon(deps) return NewCommitCommands(gitCommon) diff --git a/pkg/commands/git_commands/diff.go b/pkg/commands/git_commands/diff.go index eb8b60a7a..d9b009ec1 100644 --- a/pkg/commands/git_commands/diff.go +++ b/pkg/commands/git_commands/diff.go @@ -3,10 +3,12 @@ package git_commands import ( "fmt" "os" + "path/filepath" "strings" "github.com/jesseduffield/lazygit/pkg/commands/oscommands" "github.com/jesseduffield/lazygit/pkg/config" + "github.com/jesseduffield/lazygit/pkg/utils" "github.com/mgutz/str" ) @@ -189,6 +191,92 @@ func (self *DiffCommands) GetDiff(staged bool, additionalArgs ...string) (string ).DontLog().RunWithOutput() } +// NamedText is a text to be diffed by RenderedTextDiff, along with the name to +// show for it in the diff. The name doubles as the name of the temporary file +// holding the text, so it has to be usable as a file name. +type NamedText struct { + Name string + Content string +} + +// RenderedTextDiff returns a diff of the two given texts, produced by the +// configured diff renderer in the same way as every other diff we show. width +// and height are the size of the view the diff is going to be shown in; a diff +// renderer lays its output out for them. +// +// git can only diff files, so the texts are written to temporary files named +// after them. Under a diff renderer those names are what the diff calls the two +// sides; under git's own diff they go away with the rest of the header. +func (self *DiffCommands) RenderedTextDiff(before NamedText, after NamedText, width int, height int) (string, error) { + dir, err := os.MkdirTemp(self.os.GetTempDir(), "textdiff-") + if err != nil { + return "", err + } + defer os.RemoveAll(dir) + + for _, text := range []NamedText{before, after} { + content := text.Content + // End the file with a newline, or git says that it doesn't, which tells + // a reader of the diff nothing about the two texts. + if content != "" && !strings.HasSuffix(content, "\n") { + content += "\n" + } + if err := self.os.CreateFileWithContent(filepath.Join(dir, text.Name), content); err != nil { + return "", err + } + } + + cmdObj := self.cmd.New( + NewGitCmd("diff"). + AddCommonDiffArgs(self.diffRendererConfigManager, self.UserConfig(), DiffModeRendered). + Arg("--no-index", "--no-prefix"). + Arg(fmt.Sprintf("--color=%s", self.diffRendererConfigManager.GetColorArg())). + Arg("--", before.Name, after.Name). + Dir(dir). + ToArgv(), + ).DontLog() + + // --no-index implies --exit-code, so git exits with a non-zero status + // whenever the two texts differ; that is only an error if it left us with + // nothing to show. + if self.diffRendererConfigManager.GetDiffRendererType() == config.DiffRendererType_RawGit { + // git renders the diff itself here, so no terminal is needed to get it. + output, err := cmdObj.RunWithOutput() + if output == "" && err != nil { + return "", err + } + + return stripDiffHeaders(output), nil + } + + oscommands.SetDumbTerminalEnv(cmdObj.GetCmd()) + cmdObj.AddEnvVars( + // For diff renderer scripts that can't query the terminal width directly. + fmt.Sprintf("LAZYGIT_COLUMNS=%d", width), + "GIT_PAGER="+self.diffRendererConfigManager.GetStdinFilterCommand(width), + ) + + output, err := oscommands.RunInPtyWithOutput(cmdObj.GetCmd(), uint16(width), uint16(height)) + if output == "" && err != nil { + return "", err + } + + return output, nil +} + +// Strips the file header and the first hunk header from a diff. For a diff of +// two temporary files these say nothing that a reader could make sense of. +func stripDiffHeaders(diff string) string { + lines := strings.SplitAfter(diff, "\n") + for i, line := range lines { + if strings.HasPrefix(utils.Decolorise(line), "@@ ") { + return strings.Join(lines[i+1:], "") + } + } + + return diff +} + type DiffToolCmdOptions struct { // The path to show a diff for. Pass "." for the entire repo. Filepath string diff --git a/pkg/commands/git_commands/diff_test.go b/pkg/commands/git_commands/diff_test.go new file mode 100644 index 000000000..ca4b3e72a --- /dev/null +++ b/pkg/commands/git_commands/diff_test.go @@ -0,0 +1,126 @@ +package git_commands + +import ( + "os" + "path/filepath" + "testing" + + "github.com/go-errors/errors" + "github.com/jesseduffield/lazygit/pkg/commands/oscommands" + "github.com/jesseduffield/lazygit/pkg/config" + "github.com/stretchr/testify/assert" +) + +func TestRenderedTextDiff(t *testing.T) { + var args []string + var dir string + var beforeContent, afterContent string + + // The two texts are diffed as files in a temporary directory that is removed + // again afterwards, so read them here, while they are still there. + runner := oscommands.NewFakeRunner(t).ExpectFunc("text diff", + func(cmdObj *oscommands.CmdObj) bool { + args = cmdObj.GetCmd().Args + dir = args[2] // git -C + before, _ := os.ReadFile(filepath.Join(dir, "old message")) + after, _ := os.ReadFile(filepath.Join(dir, "new message")) + beforeContent, afterContent = string(before), string(after) + return true + }, + "\x1b[1mdiff --git old message new message\x1b[m\n"+ + "\x1b[1mindex 9ebe4a6..ec3885a 100644\x1b[m\n"+ + "\x1b[1m--- old message\x1b[m\n"+ + "\x1b[1m+++ new message\x1b[m\n"+ + "\x1b[36m@@ -1,3 +1,3 @@\x1b[m\n"+ + "-Fix the widget\n"+ + "+Fix the widget on startup\n", + // --no-index implies --exit-code + errors.New("exit status 1")) + + instance := buildDiffCommands(commonDeps{runner: runner}) + + output, err := instance.RenderedTextDiff( + NamedText{Name: "old message", Content: "Fix the widget\n"}, + NamedText{Name: "new message", Content: "Fix the widget on startup\n"}, + 80, 24) + + assert.NoError(t, err) + assert.Equal(t, "-Fix the widget\n+Fix the widget on startup\n", output) + + assert.Equal(t, []string{ + "git", "-C", dir, "diff", "--no-ext-diff", "--unified=3", "--find-renames=50%", + "--no-index", "--no-prefix", "--color=always", "--", "old message", "new message", + }, args) + assert.Equal(t, "Fix the widget\n", beforeContent) + assert.Equal(t, "Fix the widget on startup\n", afterContent) + + assert.NoDirExists(t, dir) +} + +func TestRenderedTextDiffPassesOnTheRenderersGitArgs(t *testing.T) { + userConfig := config.GetDefaultConfig() + userConfig.Git.DiffRenderers = []config.DiffRendererConfig{ + {Type: "rawGit", Args: []string{"--color-words"}}, + } + + var args []string + runner := oscommands.NewFakeRunner(t).ExpectFunc("text diff", + func(cmdObj *oscommands.CmdObj) bool { + args = cmdObj.GetCmd().Args + return true + }, "", nil) + + instance := buildDiffCommands(commonDeps{runner: runner, userConfig: userConfig}) + + _, err := instance.RenderedTextDiff( + NamedText{Name: "old message", Content: "one"}, + NamedText{Name: "new message", Content: "two"}, + 80, 24) + + assert.NoError(t, err) + assert.Contains(t, args, "--color-words") +} + +func TestStripDiffHeaders(t *testing.T) { + scenarios := []struct { + name string + diff string + expectedOutput string + }{ + { + name: "colored diff", + diff: "\x1b[1mdiff --git old message new message\x1b[m\n" + + "\x1b[1mindex 9ebe4a6..ec3885a 100644\x1b[m\n" + + "\x1b[1m--- old message\x1b[m\n" + + "\x1b[1m+++ new message\x1b[m\n" + + "\x1b[36m@@ -1,3 +1,3 @@\x1b[m\n" + + "one \x1b[32mtwo\x1b[m\n", + expectedOutput: "one \x1b[32mtwo\x1b[m\n", + }, + { + name: "uncolored diff with several hunks", + diff: "diff --git old message new message\n" + + "index 9ebe4a6..ec3885a 100644\n" + + "--- old message\n" + + "+++ new message\n" + + "@@ -1,3 +1,3 @@\n" + + "one two\n" + + "@@ -20,3 +20,3 @@\n" + + "three four\n", + expectedOutput: "one two\n" + + "@@ -20,3 +20,3 @@\n" + + "three four\n", + }, + { + name: "empty diff", + diff: "", + expectedOutput: "", + }, + } + + for _, s := range scenarios { + t.Run(s.name, func(t *testing.T) { + assert.Equal(t, s.expectedOutput, stripDiffHeaders(s.diff)) + }) + } +} diff --git a/pkg/commands/oscommands/pty.go b/pkg/commands/oscommands/pty.go index a4a369633..29ba6f06d 100644 --- a/pkg/commands/oscommands/pty.go +++ b/pkg/commands/oscommands/pty.go @@ -3,6 +3,10 @@ package oscommands import ( "io" "os" + "os/exec" + "strings" + + "github.com/samber/lo" ) // Pty is the master side of a pseudo-terminal running a subprocess. The @@ -32,3 +36,51 @@ type StartedPty struct { // Implemented per-platform in pty_unix.go / pty_windows.go. // // func StartPty(cmd *exec.Cmd, cols, rows uint16) (StartedPty, error) + +// RunInPtyWithOutput runs cmd in a pseudo-terminal of the given size and returns +// everything it wrote. Commands that render for a terminal need one to render at +// all: git only pipes its output through a pager when it thinks it is talking to +// a terminal, and the pager itself commonly decides whether to use color the same +// way. +func RunInPtyWithOutput(cmd *exec.Cmd, cols uint16, rows uint16) (string, error) { + startedPty, err := StartPty(cmd, cols, rows) + if err != nil { + return "", err + } + defer startedPty.Pty.Close() + + // Reading from the master side of a pty fails as soon as the child has closed + // the other side, so an error here only tells us that the command is done. + // What it wrote before that is what we came for. + output, _ := io.ReadAll(startedPty.Pty) + + // A terminal ends each line with a carriage return, which is of no use to a + // caller that treats the output as text. + return strings.ReplaceAll(string(output), "\r\n", "\n"), startedPty.Wait() +} + +// SetDumbTerminalEnv tells a command that runs in a pty that we're in a very +// simple terminal that they should not expect to have much capabilities. +// Moving the cursor, clearing the screen, or querying for colors are among such +// "advanced" capabilities. +// Context: https://github.com/jesseduffield/lazygit/issues/3419 +func SetDumbTerminalEnv(cmd *exec.Cmd) { + cmd.Env = append(removeExistingTermEnvVars(cmd.Env), "TERM=dumb") +} + +func removeExistingTermEnvVars(env []string) []string { + return lo.Filter(env, func(envVar string, _ int) bool { + return !isTermEnvVar(envVar) + }) +} + +// Terminals set a variety of different environment variables +// to identify themselves to processes. This list should catch the most common among them. +func isTermEnvVar(envVar string) bool { + return strings.HasPrefix(envVar, "TERM=") || + strings.HasPrefix(envVar, "TERM_PROGRAM=") || + strings.HasPrefix(envVar, "TERM_PROGRAM_VERSION=") || + strings.HasPrefix(envVar, "TERMINAL_EMULATOR=") || + strings.HasPrefix(envVar, "TERMINAL_NAME=") || + strings.HasPrefix(envVar, "TERMINAL_VERSION_") +} diff --git a/pkg/config/diff_renderer_config_manager.go b/pkg/config/diff_renderer_config_manager.go index b23933962..b5e21eebb 100644 --- a/pkg/config/diff_renderer_config_manager.go +++ b/pkg/config/diff_renderer_config_manager.go @@ -1,6 +1,7 @@ package config import ( + "fmt" "strconv" "strings" @@ -108,6 +109,18 @@ func (self *DiffRendererConfigManager) GetRawGitArgs() []string { return currentDiffRendererConfig.Args } +// Signature is what identifies the current diff renderer, so that something we +// remembered about what it produced can be dropped once it no longer describes +// the renderer we have. The width a command is asked for is no part of its +// identity, so a fixed one is used. +func (self *DiffRendererConfigManager) Signature() string { + return fmt.Sprintf("%d\x00%s\x00%s\x00%s", + self.diffRendererIndex, + self.GetExternalDiffCommand(3), + self.GetStdinFilterCommand(0), + strings.Join(self.GetRawGitArgs(), "\x00")) +} + func (self *DiffRendererConfigManager) CycleDiffRenderers() { self.diffRendererIndex = (self.diffRendererIndex + 1) % len(self.getUserConfig().Git.DiffRenderers) } diff --git a/pkg/gui/controllers/helpers/diff_helper.go b/pkg/gui/controllers/helpers/diff_helper.go index da79ac479..01a14a182 100644 --- a/pkg/gui/controllers/helpers/diff_helper.go +++ b/pkg/gui/controllers/helpers/diff_helper.go @@ -1,6 +1,7 @@ package helpers import ( + "fmt" "strings" "github.com/jesseduffield/lazygit/pkg/commands/git_commands" @@ -10,6 +11,7 @@ import ( "github.com/jesseduffield/lazygit/pkg/gui/modes/diffing" "github.com/jesseduffield/lazygit/pkg/gui/style" "github.com/jesseduffield/lazygit/pkg/gui/types" + "github.com/jesseduffield/lazygit/pkg/utils" "github.com/samber/lo" ) @@ -18,12 +20,19 @@ type DiffHelper struct { // diffLineHelper says how a diff for the main view is to be produced, which depends // on whether the focused main view could act on what a diff renderer would make of it. diffLineHelper *DiffLineHelper + + // Diffs of the messages of "amend!" commits, keyed by everything that + // shapes them: the two commits, and the diff renderer and width they were + // rendered by. An empty diff means that the commit doesn't change the + // message. Only accessed on the UI thread, while rendering the main view. + commitMessageDiffs map[string]string } func NewDiffHelper(c *HelperCommon, diffLineHelper *DiffLineHelper) *DiffHelper { return &DiffHelper{ - c: c, - diffLineHelper: diffLineHelper, + c: c, + diffLineHelper: diffLineHelper, + commitMessageDiffs: make(map[string]string), } } @@ -56,7 +65,13 @@ func (self *DiffHelper) DiffArgs() []string { // and the refRange for a range selection. If the refRange is nil (meaning that // either there's no range, or it can't be diffed for some reason), then we want // to fall back to rendering the diff for the single commit. -func (self *DiffHelper) GetUpdateTaskForRenderingCommitsDiff(commit *models.Commit, refRange *types.RefRange) types.UpdateTask { +// In addition, we need to pass the list of all commits; this is needed for +// showing the commit message diff for "amend!" commits. +func (self *DiffHelper) GetUpdateTaskForRenderingCommitsDiff( + commits []*models.Commit, + commit *models.Commit, + refRange *types.RefRange, +) types.UpdateTask { mode := self.diffLineHelper.MainViewDiffMode() if refRange != nil { @@ -84,7 +99,81 @@ func (self *DiffHelper) GetUpdateTaskForRenderingCommitsDiff(commit *models.Comm } cmdObj := self.c.Git().Commit.ShowCmdObj(commit.Hash(), self.FilterPathsForCommit(commit), mode) - return types.NewMainViewDiffTask(cmdObj.GetCmd(), mode) + return types.NewMainViewDiffTaskWithPrefix(cmdObj.GetCmd(), self.commitMessageDiffPrefix(commits, commit), mode) +} + +// For an "amend!" commit, returns a diff of the commit message it sets against +// the message it replaces, to be shown above the commit's own diff. Returns an +// empty string for any other commit, and for an "amend!" commit that only +// changes the contents of the commit it applies to. +func (self *DiffHelper) commitMessageDiffPrefix(commits []*models.Commit, commit *models.Commit) string { + previousCommit, ok := findCommitWithPreviousMessage(commits, commit) + if !ok { + return "" + } + + width, height := self.c.Contexts().Normal.GetView().InnerSize() + + diff := self.commitMessageDiff(commit, previousCommit, width, height) + if diff == "" { + return "" + } + + header := style.FgYellow.Sprintf("%s\n", utils.ResolvePlaceholderString( + self.c.Tr.CommitMessageChanges, + map[string]string{"hash": previousCommit.ShortHash()}, + )) + rule := strings.Repeat("─", width) + "\n" + + return header + diff + rule +} + +// The names the two messages are diffed under. A diff renderer shows them as the +// names of the files being diffed, so they are what tells the reader which side +// is which. They are not translated because they end up as file names, and git +// mangles paths outside of ASCII when it states them in a diff. +const ( + oldMessageName = "old message" + newMessageName = "new message" +) + +func (self *DiffHelper) commitMessageDiff( + commit *models.Commit, previousCommit *models.Commit, width int, height int, +) string { + // The diff renderer lays the diff out, and lays it out for the width it is + // shown at, so both belong in the key along with the two messages. + key := fmt.Sprintf("%s\x00%s\x00%d\x00%s", + commit.Hash(), + previousCommit.Hash(), + width, + self.c.State().GetDiffRendererConfigManager().Signature()) + if diff, ok := self.commitMessageDiffs[key]; ok { + return diff + } + + messages, err := self.c.Git().Commit.GetCommitMessages([]string{previousCommit.Hash(), commit.Hash()}) + if err != nil { + self.c.Log.Error(err) + return "" + } + + before := messageAfterAmending(messages[0]) + after := messageAfterAmending(messages[1]) + + diff := "" + if before != after { + diff, err = self.c.Git().Diff.RenderedTextDiff( + git_commands.NamedText{Name: oldMessageName, Content: before}, + git_commands.NamedText{Name: newMessageName, Content: after}, + width, height) + if err != nil { + self.c.Log.Error(err) + return "" + } + } + + self.commitMessageDiffs[key] = diff + return diff } // PlainDiffBetweenRefs returns the diff of the given files between two refs as git diff --git a/pkg/gui/controllers/helpers/fixup_helper.go b/pkg/gui/controllers/helpers/fixup_helper.go index e998c2ad1..2d1f6ab90 100644 --- a/pkg/gui/controllers/helpers/fixup_helper.go +++ b/pkg/gui/controllers/helpers/fixup_helper.go @@ -409,3 +409,62 @@ func IsFixupCommit(subject string) (string, bool) { return subject, false } + +// Check whether the given subject line is the subject of an "amend!" commit, +// i.e. of a commit that replaces the message of the commit it applies to, and +// return the subject of that commit if so. Note that a commit with a subject +// like "fixup! amend! Bla" is not an "amend!" commit; only the outermost +// prefix decides what happens to the message. +func isAmendCommit(subject string) (string, bool) { + if !strings.HasPrefix(subject, "amend! ") { + return subject, false + } + + return IsFixupCommit(subject) +} + +// For an "amend!" commit, find the commit that holds the message it replaces. +// This is the nearest "amend!" commit below it that applies to the same commit, +// or, if there is none, the commit it applies to itself. Returns false if the +// given commit isn't an "amend!" commit, or if the commit it applies to isn't +// in the given list. +func findCommitWithPreviousMessage(commits []*models.Commit, commit *models.Commit) (*models.Commit, bool) { + baseSubject, isAmend := isAmendCommit(commit.Name) + if !isAmend { + return nil, false + } + + _, index, ok := lo.FindIndexOf(commits, func(c *models.Commit) bool { + return c.Hash() == commit.Hash() + }) + if !ok { + return nil, false + } + + for _, previousCommit := range commits[index+1:] { + if previousCommit.Name == baseSubject { + return previousCommit, true + } + if subject, isAmend := isAmendCommit(previousCommit.Name); isAmend && subject == baseSubject { + return previousCommit, true + } + } + + return nil, false +} + +// Return the message that the given commit leaves on the commit it applies to: +// for an "amend!" commit this is its message without the "amend! " +// line at the top, and for any other commit it is simply its own message. +func messageAfterAmending(message string) string { + subject, body, found := strings.Cut(message, "\n") + if !found { + return message + } + + if _, isAmend := isAmendCommit(subject); !isAmend { + return message + } + + return strings.TrimLeft(body, "\n") +} diff --git a/pkg/gui/controllers/helpers/fixup_helper_test.go b/pkg/gui/controllers/helpers/fixup_helper_test.go index 466e90087..543260dd9 100644 --- a/pkg/gui/controllers/helpers/fixup_helper_test.go +++ b/pkg/gui/controllers/helpers/fixup_helper_test.go @@ -205,6 +205,187 @@ func TestFixupHelper_IsFixupCommit(t *testing.T) { } } +func TestFixupHelper_findCommitWithPreviousMessage(t *testing.T) { + hashPool := &utils.StringPool{} + + type commitDesc struct { + Hash string + Name string + } + + scenarios := []struct { + name string + commits []commitDesc + index int + expectedHash string + }{ + { + name: "not an amend commit", + commits: []commitDesc{ + {"abc123", "Some feature"}, + }, + index: 0, + expectedHash: "", + }, + { + name: "fixup commits don't change the message", + commits: []commitDesc{ + {"abc123", "fixup! Some feature"}, + {"def456", "Some feature"}, + }, + index: 0, + expectedHash: "", + }, + { + name: "a fixup of an amend commit doesn't change the message either", + commits: []commitDesc{ + {"abc123", "fixup! amend! Some feature"}, + {"def456", "amend! Some feature"}, + {"ghi789", "Some feature"}, + }, + index: 0, + expectedHash: "", + }, + { + name: "base commit right below the amend commit", + commits: []commitDesc{ + {"abc123", "amend! Some feature"}, + {"def456", "Some feature"}, + }, + index: 0, + expectedHash: "def456", + }, + { + name: "base commit further down the list", + commits: []commitDesc{ + {"abc123", "amend! Some feature"}, + {"def456", "Unrelated commit"}, + {"ghi789", "Some feature"}, + }, + index: 0, + expectedHash: "ghi789", + }, + { + name: "amend commit in the middle of the list", + commits: []commitDesc{ + {"abc123", "Unrelated commit"}, + {"def456", "amend! Some feature"}, + {"ghi789", "Some feature"}, + }, + index: 1, + expectedHash: "ghi789", + }, + { + name: "the nearest earlier amend commit holds the message", + commits: []commitDesc{ + {"abc123", "amend! Some feature"}, + {"def456", "amend! Some feature"}, + {"ghi789", "Some feature"}, + }, + index: 0, + expectedHash: "def456", + }, + { + name: "fixup and squash commits in between are skipped", + commits: []commitDesc{ + {"abc123", "amend! Some feature"}, + {"def456", "fixup! Some feature"}, + {"ghi789", "squash! Some feature"}, + {"jkl012", "amend! Some feature"}, + {"mno345", "Some feature"}, + }, + index: 0, + expectedHash: "jkl012", + }, + { + name: "amend commit with several prefixes applies to the innermost subject", + commits: []commitDesc{ + {"abc123", "amend! amend! Some feature"}, + {"def456", "amend! Some feature"}, + {"ghi789", "Some feature"}, + }, + index: 0, + expectedHash: "def456", + }, + { + name: "base commit is not in the list", + commits: []commitDesc{ + {"abc123", "amend! Some feature"}, + {"def456", "Unrelated commit"}, + }, + index: 0, + expectedHash: "", + }, + { + name: "base commit is above the amend commit", + commits: []commitDesc{ + {"abc123", "Some feature"}, + {"def456", "amend! Some feature"}, + }, + index: 1, + expectedHash: "", + }, + } + + for _, s := range scenarios { + t.Run(s.name, func(t *testing.T) { + commits := lo.Map(s.commits, func(desc commitDesc, _ int) *models.Commit { + return models.NewCommit(hashPool, models.NewCommitOpts{Hash: desc.Hash, Name: desc.Name}) + }) + + result, ok := findCommitWithPreviousMessage(commits, commits[s.index]) + + if s.expectedHash == "" { + assert.False(t, ok) + assert.Nil(t, result) + } else { + assert.True(t, ok) + assert.Equal(t, s.expectedHash, result.Hash()) + } + }) + } +} + +func TestFixupHelper_messageAfterAmending(t *testing.T) { + scenarios := []struct { + name string + message string + expectedMessage string + }{ + { + name: "subject only", + message: "Some feature", + expectedMessage: "Some feature", + }, + { + name: "subject and body", + message: "Some feature\n\nSome description", + expectedMessage: "Some feature\n\nSome description", + }, + { + name: "amend commit", + message: "amend! Some feature\n\nA better subject\n\nSome description", + expectedMessage: "A better subject\n\nSome description", + }, + { + name: "amend commit without a replacement message", + message: "amend! Some feature", + expectedMessage: "amend! Some feature", + }, + { + name: "fixup commit", + message: "fixup! amend! Some feature\n\nSome description", + expectedMessage: "fixup! amend! Some feature\n\nSome description", + }, + } + + for _, s := range scenarios { + t.Run(s.name, func(t *testing.T) { + assert.Equal(t, s.expectedMessage, messageAfterAmending(s.message)) + }) + } +} + func TestFixupHelper_removeFixupCommits(t *testing.T) { hashPool := &utils.StringPool{} diff --git a/pkg/gui/controllers/local_commits_controller.go b/pkg/gui/controllers/local_commits_controller.go index 983ca789f..2c638205d 100644 --- a/pkg/gui/controllers/local_commits_controller.go +++ b/pkg/gui/controllers/local_commits_controller.go @@ -706,7 +706,8 @@ func (self *LocalCommitsController) GetOnRenderToMain() func() { self.c.Tr.ExecCommandHere + "\n\n" + commit.Name) } else { refRange := self.context().GetSelectedRefRangeForDiffFiles() - task = self.c.Helpers().Diff.GetUpdateTaskForRenderingCommitsDiff(commit, refRange) + task = self.c.Helpers().Diff.GetUpdateTaskForRenderingCommitsDiff( + self.c.Model().Commits, commit, refRange) } self.c.RenderToMainViews(types.RefreshMainOpts{ diff --git a/pkg/gui/controllers/sub_commits_controller.go b/pkg/gui/controllers/sub_commits_controller.go index 7f7a163c9..665663827 100644 --- a/pkg/gui/controllers/sub_commits_controller.go +++ b/pkg/gui/controllers/sub_commits_controller.go @@ -46,7 +46,8 @@ func (self *SubCommitsController) GetOnRenderToMain() func() { task = types.NewRenderStringTask("No commits") } else { refRange := self.context().GetSelectedRefRangeForDiffFiles() - task = self.c.Helpers().Diff.GetUpdateTaskForRenderingCommitsDiff(commit, refRange) + task = self.c.Helpers().Diff.GetUpdateTaskForRenderingCommitsDiff( + self.c.Model().SubCommits, commit, refRange) } self.c.RenderToMainViews(types.RefreshMainOpts{ diff --git a/pkg/gui/pty.go b/pkg/gui/pty.go index eda2aa295..50ab68da6 100644 --- a/pkg/gui/pty.go +++ b/pkg/gui/pty.go @@ -14,7 +14,6 @@ import ( "github.com/jesseduffield/lazygit/pkg/gocui" "github.com/jesseduffield/lazygit/pkg/tasks" "github.com/jesseduffield/lazygit/pkg/utils" - "github.com/samber/lo" ) func (gui *Gui) desiredPtySize(view *gocui.View) (cols, rows uint16) { @@ -104,12 +103,7 @@ func (gui *Gui) newPtyTask(view *gocui.View, cmd *exec.Cmd, prefix string) error width = view.InnerWidth() pager := gui.stateAccessor.GetDiffRendererConfigManager().GetStdinFilterCommand(width) - // This communicates to diff renderers that we're in a very simple - // terminal that they should not expect to have much capabilities. - // Moving the cursor, clearing the screen, or querying for colors are among such "advanced" capabilities. - // Context: https://github.com/jesseduffield/lazygit/issues/3419 - cmd.Env = removeExistingTermEnvVars(cmd.Env) - cmd.Env = append(cmd.Env, "TERM=dumb") + oscommands.SetDumbTerminalEnv(cmd) cmd.Env = append(cmd.Env, "GIT_PAGER="+pager) @@ -202,20 +196,3 @@ func withPtyGitConfig(args []string, goos string) []string { result = append(result, "-c", "diff.autoRefreshIndex=false") return append(result, args[1:]...) } - -func removeExistingTermEnvVars(env []string) []string { - return lo.Filter(env, func(envVar string, _ int) bool { - return !isTermEnvVar(envVar) - }) -} - -// Terminals set a variety of different environment variables -// to identify themselves to processes. This list should catch the most common among them. -func isTermEnvVar(envVar string) bool { - return strings.HasPrefix(envVar, "TERM=") || - strings.HasPrefix(envVar, "TERM_PROGRAM=") || - strings.HasPrefix(envVar, "TERM_PROGRAM_VERSION=") || - strings.HasPrefix(envVar, "TERMINAL_EMULATOR=") || - strings.HasPrefix(envVar, "TERMINAL_NAME=") || - strings.HasPrefix(envVar, "TERMINAL_VERSION_") -} diff --git a/pkg/i18n/english.go b/pkg/i18n/english.go index 1115f5f56..242bb16a8 100644 --- a/pkg/i18n/english.go +++ b/pkg/i18n/english.go @@ -693,6 +693,7 @@ type TranslationSet struct { OpenCommandLogMenuTooltip string ShowingGitDiff string ShowingDiffForRange string + CommitMessageChanges string CommitDiff string CopyCommitHashToClipboard string CommitHash string @@ -1846,6 +1847,7 @@ func EnglishTranslationSet() *TranslationSet { OpenCommandLogMenuTooltip: "View options for the command log e.g. show/hide the command log and focus the command log.", ShowingGitDiff: "Showing output for:", ShowingDiffForRange: "Showing diff for range", + CommitMessageChanges: "Commit message changes compared to {{.hash}}:", CommitDiff: "Commit diff", CopyCommitHashToClipboard: "Copy abbreviated commit hash to clipboard", CommitHash: "Commit hash", diff --git a/pkg/integration/tests/commit/show_amend_commit_message_diff.go b/pkg/integration/tests/commit/show_amend_commit_message_diff.go new file mode 100644 index 000000000..1694ef532 --- /dev/null +++ b/pkg/integration/tests/commit/show_amend_commit_message_diff.go @@ -0,0 +1,74 @@ +package commit + +import ( + "github.com/jesseduffield/lazygit/pkg/config" + . "github.com/jesseduffield/lazygit/pkg/integration/components" +) + +var ShowAmendCommitMessageDiff = NewIntegrationTest(NewIntegrationTestArgs{ + Description: "Show a diff of the commit message when selecting an amend! commit", + ExtraCmdArgs: []string{}, + Skip: false, + SetupConfig: func(config *config.AppConfig) {}, + SetupRepo: func(shell *Shell) { + shell. + EmptyCommitWithBody("Fix the widget", + "The frobnicator was not initialised properly at all."). + EmptyCommitWithBody("amend! Fix the widget", + "Fix the widget on startup\n\nThe frobnicator was not initialised at all."). + EmptyCommitWithBody("amend! Fix the widget", + "Fix the widget on startup\n\nThe frobnicator was not properly initialised at all.") + }, + Run: func(t *TestDriver, keys config.KeybindingConfig) { + // git's default colors for added and removed lines + green := "#008000" + red := "#800000" + + t.Views().Commits(). + Focus(). + Lines( + Contains("amend! Fix the widget").IsSelected(), + Contains("amend! Fix the widget"), + Contains("Fix the widget"), + ) + + // The message of the topmost amend! commit is compared with the message + // of the amend! commit below it, not with the one of the commit they + // both apply to. + t.Views().Main(). + TopLines( + Contains("Commit message changes compared to"), + Equals(" Fix the widget on startup"), + Equals(" "), + Equals("-The frobnicator was not initialised at all."), + Equals("+The frobnicator was not properly initialised at all."), + Contains("───"), + ). + ContainsColoredText(green, "+The frobnicator was not properly initialised at all."). + ContainsColoredText(red, "-The frobnicator was not initialised at all.") + + t.Views().Commits(). + SelectNextItem() + + // The other amend! commit is compared with the commit it applies to. + t.Views().Main(). + TopLines( + Contains("Commit message changes compared to"), + Equals("-Fix the widget"), + Equals("+Fix the widget on startup"), + Equals(" "), + Equals("-The frobnicator was not initialised properly at all."), + Equals("+The frobnicator was not initialised at all."), + Contains("───"), + ) + + t.Views().Commits(). + SelectNextItem() + + // The commit that they both apply to gets no such header. + t.Views().Main(). + TopLines( + Contains("commit "), + ) + }, +}) diff --git a/pkg/integration/tests/commit/show_amend_commit_message_diff_with_renderer.go b/pkg/integration/tests/commit/show_amend_commit_message_diff_with_renderer.go new file mode 100644 index 000000000..ee501ffe7 --- /dev/null +++ b/pkg/integration/tests/commit/show_amend_commit_message_diff_with_renderer.go @@ -0,0 +1,48 @@ +package commit + +import ( + "github.com/jesseduffield/lazygit/pkg/config" + . "github.com/jesseduffield/lazygit/pkg/integration/components" +) + +var ShowAmendCommitMessageDiffWithRenderer = NewIntegrationTest(NewIntegrationTestArgs{ + Description: "Show the commit message diff of an amend! commit through a diff renderer", + ExtraCmdArgs: []string{}, + Skip: false, + SetupConfig: func(cfg *config.AppConfig) { + // cat does nothing to the diff, but it is a diff renderer as far as + // lazygit is concerned, so the diff takes the same route through it as + // it would for a real one. + cfg.GetUserConfig().Git.DiffRenderers = []config.DiffRendererConfig{{Command: "cat"}} + }, + SetupRepo: func(shell *Shell) { + shell. + EmptyCommitWithBody("Fix the widget", + "The frobnicator was not initialised."). + EmptyCommitWithBody("amend! Fix the widget", + "Fix the widget on startup\n\nThe frobnicator was not initialised.") + }, + Run: func(t *TestDriver, keys config.KeybindingConfig) { + t.Views().Commits(). + Focus(). + Lines( + Contains("amend! Fix the widget").IsSelected(), + Contains("Fix the widget"), + ) + + // A diff renderer states the two sides of the diff in its own way, so + // unlike for git's own diff, the header naming them is kept. + t.Views().Main(). + TopLines( + Contains("Commit message changes compared to"), + Equals("diff --git old message new message"), + ). + ContainsLines( + Contains("--- old message"), + Contains("+++ new message"), + Contains("@@"), + Equals("-Fix the widget"), + Equals("+Fix the widget on startup"), + ) + }, +}) diff --git a/pkg/integration/tests/test_list.go b/pkg/integration/tests/test_list.go index 2c3e1dbec..e12f5cce7 100644 --- a/pkg/integration/tests/test_list.go +++ b/pkg/integration/tests/test_list.go @@ -158,6 +158,8 @@ var tests = []*components.IntegrationTest{ commit.Search, commit.SetAuthor, commit.SetAuthorRange, + commit.ShowAmendCommitMessageDiff, + commit.ShowAmendCommitMessageDiffWithRenderer, commit.StageRangeOfLines, commit.Staged, commit.StagedWithoutHooks,