From 394e815edda6fe225650e519f04dc9dbb48f9596 Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Mon, 5 Oct 2026 19:22:57 +0200 Subject: [PATCH] [SQUASHED] show-commit-msg-diff-for-amend-commits --- pkg/commands/git_commands/commit.go | 33 +++- pkg/commands/git_commands/commit_test.go | 19 +- pkg/commands/git_commands/deps_test.go | 5 + pkg/commands/git_commands/diff.go | 174 +++++++++++++++++ pkg/commands/git_commands/diff_test.go | 128 +++++++++++++ pkg/commands/oscommands/pty.go | 73 +++++++ pkg/config/diff_renderer_config_manager.go | 18 ++ pkg/gui/controllers/helpers/diff_helper.go | 128 ++++++++++++- pkg/gui/controllers/helpers/fixup_helper.go | 59 ++++++ .../controllers/helpers/fixup_helper_test.go | 181 ++++++++++++++++++ .../controllers/local_commits_controller.go | 3 +- pkg/gui/controllers/sub_commits_controller.go | 3 +- pkg/gui/gui_common.go | 4 + pkg/gui/main_view_render.go | 50 +---- pkg/gui/types/common.go | 4 + pkg/i18n/english.go | 2 + .../commit/show_amend_commit_message_diff.go | 74 +++++++ ...mit_message_diff_beside_the_patch_marks.go | 43 +++++ ...ommit_message_diff_in_a_split_main_view.go | 47 +++++ ...mend_commit_message_diff_through_a_pipe.go | 48 +++++ ...amend_commit_message_diff_with_renderer.go | 48 +++++ ..._commit_message_diff_with_several_hunks.go | 60 ++++++ pkg/integration/tests/test_list.go | 6 + 23 files changed, 1152 insertions(+), 58 deletions(-) create mode 100644 pkg/commands/git_commands/diff_test.go create mode 100644 pkg/integration/tests/commit/show_amend_commit_message_diff.go create mode 100644 pkg/integration/tests/commit/show_amend_commit_message_diff_beside_the_patch_marks.go create mode 100644 pkg/integration/tests/commit/show_amend_commit_message_diff_in_a_split_main_view.go create mode 100644 pkg/integration/tests/commit/show_amend_commit_message_diff_through_a_pipe.go create mode 100644 pkg/integration/tests/commit/show_amend_commit_message_diff_with_renderer.go create mode 100644 pkg/integration/tests/commit/show_amend_commit_message_diff_with_several_hunks.go 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 274385f53..ba799164b 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 01bb670c2..9f535a2ac 100644 --- a/pkg/commands/git_commands/diff.go +++ b/pkg/commands/git_commands/diff.go @@ -2,11 +2,15 @@ package git_commands import ( "fmt" + "io" "os" + "path/filepath" + "slices" "strings" "github.com/jesseduffield/lazygit/pkg/commands/oscommands" "github.com/jesseduffield/lazygit/pkg/config" + "github.com/jesseduffield/lazygit/pkg/utils" "github.com/mgutz/str" ) @@ -197,6 +201,176 @@ func (self *DiffCommands) GetDiff(staged bool, additionalArgs ...string) (string ).DontLog().RunWithOutput() } +// NamedText is a text to be diffed by a TextDiffer, 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 +} + +// A TextDiffer diffs two texts with the configured diff renderer, in the same way +// as every other diff we show. It holds everything it takes from the +// configuration, so that it can render a diff on any goroutine. +type TextDiffer struct { + diffCommands *DiffCommands + + // The git command that diffs the two texts, up to the names of the files + // holding them + args []string + rawGit bool + stdinFilter string + externalDiff string + width int + height int +} + +// NewTextDiffer resolves what a TextDiffer takes from the configuration. values +// are what the renderer's command is resolved with. values.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. +// +// It has to be called on the UI thread, since that is where the user switches +// diff renderers and has the config reloaded. +func (self *DiffCommands) NewTextDiffer(values config.DiffRendererValues, height int) (*TextDiffer, error) { + manager := self.diffRendererConfigManager + stdinFilter, err := manager.GetStdinFilterCommand(values) + if err != nil { + return nil, err + } + externalDiff, err := manager.GetExternalDiffCommand(values) + if err != nil { + return nil, err + } + + return &TextDiffer{ + diffCommands: self, + args: NewGitCmd("diff"). + // git heads a hunk with the nearest line above it that starts with + // a letter, for the name of the function the hunk is in. In a text, + // that is just a line out of context. A pattern that never matches + // leaves it out; "default" is the driver of a file that no + // attribute gives one. + Config("diff.default.xfuncname=x^"). + AddCommonDiffArgs(manager, self.UserConfig(), DiffModeRendered). + Arg("--no-index", "--no-prefix"). + Arg(fmt.Sprintf("--color=%s", DiffModeRendered.colorArg(manager))). + ToArgv(), + rawGit: manager.GetDiffRendererType() == config.DiffRendererType_RawGit, + stdinFilter: stdinFilter, + externalDiff: externalDiff, + width: values.Width, + height: height, + }, nil +} + +// RenderedDiff returns a diff of the two given texts. +// +// 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 *TextDiffer) RenderedDiff(before NamedText, after NamedText) (string, error) { + dir, err := os.MkdirTemp(self.diffCommands.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.diffCommands.os.CreateFileWithContent(filepath.Join(dir, text.Name), content); err != nil { + return "", err + } + } + + cmdObj := self.diffCommands.cmd.New( + append(slices.Clone(self.args), "--", before.Name, after.Name), + ).SetWd(dir).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.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()) + // For diff renderers that don't ask the terminal how wide it is. + cmdObj.AddEnvVars(fmt.Sprintf("COLUMNS=%d", self.width)) + // An empty command means that git's own diff.external config applies, and + // git consults that only while the variable is unset. + if self.externalDiff != "" { + cmdObj.AddEnvVars("GIT_EXTERNAL_DIFF=" + self.externalDiff) + } + + var output string + if oscommands.RendersThroughAPipe() { + output, err = self.runThroughAPipe(cmdObj) + } else { + // git runs the stdin filter itself, as the pager it is told about here. + // Named even when there is none, so that git doesn't reach for the + // user's core.pager instead. + cmdObj.AddEnvVars("GIT_PAGER=" + self.stdinFilter) + output, err = oscommands.RunInPtyWithOutput(cmdObj.GetCmd(), uint16(self.width), uint16(self.height)) + } + if output == "" && err != nil { + return "", err + } + + return output, nil +} + +// runThroughAPipe feeds the output of cmdObj to the stdin filter through a pipe +// and returns what the filter wrote. git only runs a stdin filter itself when it +// thinks it is talking to a terminal, so here the filter is a command of our +// own. An external diff renderer is run by git, so with one the command runs +// alone. +func (self *TextDiffer) runThroughAPipe(cmdObj *oscommands.CmdObj) (string, error) { + if self.stdinFilter == "" { + return cmdObj.RunWithOutput() + } + + // The filter runs in a plain shell, with git's environment, since that is + // how git would have run it. + pipeline, reader, err := self.diffCommands.os.StartPipeline( + cmdObj, + self.diffCommands.cmd.NewShell(self.stdinFilter, "").SetEnviron(cmdObj.GetCmd().Env).DontLog(), + ) + if err != nil { + return "", err + } + defer reader.Close() + + // The output ends once every command in the pipeline has exited, and Wait + // says how they did. + output, _ := io.ReadAll(reader) + return string(output), pipeline.Wait() +} + +// 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..f8b8a88a6 --- /dev/null +++ b/pkg/commands/git_commands/diff_test.go @@ -0,0 +1,128 @@ +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 TestTextDifferRenderedDiff(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 = cmdObj.GetCmd().Dir + 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}) + + differ, err := instance.NewTextDiffer(config.DiffRendererValues{Width: 80, DiffContext: 3}, 24) + assert.NoError(t, err) + output, err := differ.RenderedDiff( + NamedText{Name: "old message", Content: "Fix the widget\n"}, + NamedText{Name: "new message", Content: "Fix the widget on startup\n"}) + + assert.NoError(t, err) + assert.Equal(t, "-Fix the widget\n+Fix the widget on startup\n", output) + + assert.Equal(t, []string{ + "git", "-c", "diff.default.xfuncname=x^", "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 TestTextDifferPassesOnTheRenderersGitArgs(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}) + + differ, err := instance.NewTextDiffer(config.DiffRendererValues{Width: 80, DiffContext: 3}, 24) + assert.NoError(t, err) + _, err = differ.RenderedDiff( + NamedText{Name: "old message", Content: "one"}, + NamedText{Name: "new message", Content: "two"}) + + 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..c5d076c58 100644 --- a/pkg/commands/oscommands/pty.go +++ b/pkg/commands/oscommands/pty.go @@ -3,6 +3,11 @@ package oscommands import ( "io" "os" + "os/exec" + "runtime" + "strings" + + "github.com/samber/lo" ) // Pty is the master side of a pseudo-terminal running a subprocess. The @@ -32,3 +37,71 @@ type StartedPty struct { // Implemented per-platform in pty_unix.go / pty_windows.go. // // func StartPty(cmd *exec.Cmd, cols, rows uint16) (StartedPty, error) + +// renderWithoutPtyEnvVar makes a render take the piped path on a platform that +// would otherwise use a pty, so that tests can exercise it anywhere. +const renderWithoutPtyEnvVar = "LAZYGIT_RENDER_WITHOUT_PTY" + +// RendersThroughAPipe reports whether a render feeds the diff renderer the +// command's output through a pipe rather than running it in a pty. +// +// On Windows it has to. ConPTY doesn't pass a command's output through; it +// parses it into a screen buffer and re-encodes that for the terminal side, +// and it hands a sequence it can't represent there the moment it parses it, +// separately from the text around it. So what a renderer writes is not what +// lazygit reads. A pipe carries the bytes as the renderer wrote them. +// +// Everywhere else the pty is kept, since a renderer can read the width it +// should lay out to off it, and a configuration that doesn't name a width would +// otherwise render at whatever width the renderer falls back to. +func RendersThroughAPipe() bool { + return runtime.GOOS == "windows" || os.Getenv(renderWithoutPtyEnvVar) != "" +} + +// 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 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 +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 8c6517dc8..44983c746 100644 --- a/pkg/config/diff_renderer_config_manager.go +++ b/pkg/config/diff_renderer_config_manager.go @@ -143,6 +143,24 @@ 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 values a command is resolved with are no part of +// its identity, so the command's template stands for it. +func (self *DiffRendererConfigManager) Signature() string { + currentDiffRendererConfig := self.currentDiffRendererConfig() + if currentDiffRendererConfig == nil { + return "" + } + + return fmt.Sprintf("%d\x00%s\x00%s\x00%s\x00%s", + self.diffRendererIndex, + currentDiffRendererConfig.Type, + currentDiffRendererConfig.ColorArg, + currentDiffRendererConfig.Command, + strings.Join(currentDiffRendererConfig.Args, "\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 a6ef3dc28..a17609005 100644 --- a/pkg/gui/controllers/helpers/diff_helper.go +++ b/pkg/gui/controllers/helpers/diff_helper.go @@ -1,27 +1,38 @@ package helpers import ( + "fmt" "strings" "github.com/jesseduffield/lazygit/pkg/commands/git_commands" "github.com/jesseduffield/lazygit/pkg/commands/models" "github.com/jesseduffield/lazygit/pkg/commands/patch" + "github.com/jesseduffield/lazygit/pkg/config" "github.com/jesseduffield/lazygit/pkg/gui/context" "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" ) type DiffHelper struct { c *HelperCommon diffLineHelper *DiffLineHelper + + // Diffs of the messages of "amend!" commits, keyed by everything that + // shapes them: the two commits, the diff renderer, and the values its + // command was resolved with. An empty diff means that the commit doesn't + // change the message. Only accessed on the UI thread, so a diff produced + // on a render's goroutine is stored by way of OnUIThread. + 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), } } @@ -54,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 { @@ -82,7 +99,110 @@ 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 prefix with a diff of the commit message it +// sets against the message it replaces, to be shown above the commit's own diff. +// Returns nil for any other commit. The prefix is empty 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) types.Prefix { + previousCommit, ok := findCommitWithPreviousMessage(commits, commit) + if !ok { + return nil + } + + header := style.FgYellow.Sprintf("%s\n", utils.ResolvePlaceholderString( + self.c.Tr.CommitMessageChanges, + map[string]string{"hash": previousCommit.ShortHash()}, + )) + + return func(width int) func() string { + produceDiff := self.commitMessageDiff(previousCommit.Hash(), commit.Hash(), width) + return func() string { + diff := produceDiff() + if diff == "" { + return "" + } + + return header + diff + strings.Repeat("─", width) + "\n" + } + } +} + +// 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" +) + +// commitMessageDiff returns the function that produces the diff of the messages +// of the two commits, laid out to the given width. It is called on the UI +// thread, and the function it returns on the render's own goroutine (see +// types.Prefix). +func (self *DiffHelper) commitMessageDiff(previousHash string, hash string, width int) func() string { + values := config.DiffRendererValues{ + Width: width, + DiffContext: self.c.UserConfig().Git.DiffContextSize, + LightBackground: self.c.TerminalHasLightBackground(), + } + + // The diff renderer lays the diff out, and lays it out according to the + // values its command is resolved with, so both belong in the key along with + // the two messages. + key := fmt.Sprintf("%s\x00%s\x00%+v\x00%s", + hash, + previousHash, + values, + self.c.State().GetDiffRendererConfigManager().Signature()) + if diff, ok := self.commitMessageDiffs[key]; ok { + return func() string { return diff } + } + + differ, err := self.c.Git().Diff.NewTextDiffer(values, self.c.Contexts().Normal.GetView().InnerHeight()) + if err != nil { + self.c.Log.Error(err) + return func() string { return "" } + } + commitCommands := self.c.Git().Commit + + return func() string { + diff, err := renderCommitMessageDiff(commitCommands, differ, previousHash, hash) + if err != nil { + self.c.Log.Error(err) + return "" + } + + self.c.OnUIThread(func() error { + self.commitMessageDiffs[key] = diff + return nil + }) + return diff + } +} + +// renderCommitMessageDiff returns the diff of the messages of the two commits, or +// an empty string if they are the same. +func renderCommitMessageDiff( + commitCommands *git_commands.CommitCommands, differ *git_commands.TextDiffer, previousHash string, hash string, +) (string, error) { + messages, err := commitCommands.GetCommitMessages([]string{previousHash, hash}) + if err != nil { + return "", err + } + + before := messageAfterAmending(messages[0]) + after := messageAfterAmending(messages[1]) + if before == after { + return "", nil + } + + return differ.RenderedDiff( + git_commands.NamedText{Name: oldMessageName, Content: before}, + git_commands.NamedText{Name: newMessageName, Content: after}) } // 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 b5db039d8..da3791c74 100644 --- a/pkg/gui/controllers/local_commits_controller.go +++ b/pkg/gui/controllers/local_commits_controller.go @@ -702,7 +702,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/gui_common.go b/pkg/gui/gui_common.go index d467a0834..da54bcb68 100644 --- a/pkg/gui/gui_common.go +++ b/pkg/gui/gui_common.go @@ -227,6 +227,10 @@ func (self *guiCommon) InDemo() bool { return self.gui.integrationTest != nil && self.gui.integrationTest.IsDemo() } +func (self *guiCommon) TerminalHasLightBackground() bool { + return self.gui.terminalHasLightBackground() +} + func (self *guiCommon) WithInlineStatus(item types.HasUrn, operation types.ItemOperation, contextKey types.ContextKey, f func(gocui.Task) error) error { self.gui.helpers.InlineStatus.WithInlineStatus(helpers.InlineStatusOpts{Item: item, Operation: operation, ContextKey: contextKey}, f) return nil diff --git a/pkg/gui/main_view_render.go b/pkg/gui/main_view_render.go index 04dd5120a..d8da4cb6c 100644 --- a/pkg/gui/main_view_render.go +++ b/pkg/gui/main_view_render.go @@ -4,16 +4,14 @@ import ( "errors" "fmt" "io" - "os" "os/exec" - "runtime" "strings" + "github.com/jesseduffield/lazygit/pkg/commands/oscommands" "github.com/jesseduffield/lazygit/pkg/config" "github.com/jesseduffield/lazygit/pkg/gocui" "github.com/jesseduffield/lazygit/pkg/gui/types" "github.com/jesseduffield/lazygit/pkg/tasks" - "github.com/samber/lo" ) // renderSpec describes a render of a command's output into a view: what a way @@ -96,12 +94,7 @@ func (gui *Gui) newRenderTask(view *gocui.View, cmd *exec.Cmd, prefix types.Pref gui.c.ErrorToast(err.Error()) } - // 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) // An external diff command is named to git here, in the environment, // because the width it renders at is only known after the layout, and @@ -120,7 +113,7 @@ func (gui *Gui) newRenderTask(view *gocui.View, cmd *exec.Cmd, prefix types.Pref stdinFilter: stdinFilter, } run := gui.ptyRender - if rendersThroughAPipe() { + if oscommands.RendersThroughAPipe() { run = gui.pipedRender } return gui.newTaskForRender(reservation, spec, prefix, cmdStr, run) @@ -174,26 +167,6 @@ func (gui *Gui) newTaskForRender(reservation tasks.TaskReservation, spec renderS return manager.NewReservedTask(reservation, manager.NewCmdTask(start, producePrefix, linesToRead, onClose), key) } -// renderWithoutPtyEnvVar makes a render take the piped path on a platform that -// would otherwise use a pty, so that tests can exercise it anywhere. -const renderWithoutPtyEnvVar = "LAZYGIT_RENDER_WITHOUT_PTY" - -// rendersThroughAPipe reports whether a render feeds the diff renderer the -// command's output through a pipe rather than running it in a pty. -// -// On Windows it has to. ConPTY doesn't pass a command's output through; it -// parses it into a screen buffer and re-encodes that for the terminal side, -// and it hands a sequence it can't represent there the moment it parses it, -// separately from the text around it. So what a renderer writes is not what -// lazygit reads. A pipe carries the bytes as the renderer wrote them. -// -// Everywhere else the pty is kept, since a renderer can read the width it -// should lay out to off it, and a configuration that doesn't name a width would -// otherwise render at whatever width the renderer falls back to. -func rendersThroughAPipe() bool { - return runtime.GOOS == "windows" || os.Getenv(renderWithoutPtyEnvVar) != "" -} - // pipedRender feeds the diff renderer the command's output through a pipe. // // A stdin filter becomes a command of our own here, because git only invokes @@ -254,20 +227,3 @@ func (gui *Gui) pipedRender(spec renderSpec) (startRender, onCloseRender) { func setColumnsEnvVar(cmd *exec.Cmd, width int) { cmd.Env = append(cmd.Env, fmt.Sprintf("COLUMNS=%d", width)) } - -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/gui/types/common.go b/pkg/gui/types/common.go index 6f77c7856..59657f9b7 100644 --- a/pkg/gui/types/common.go +++ b/pkg/gui/types/common.go @@ -159,6 +159,10 @@ type IGuiCommon interface { // Returns true if we're in a demo recording/playback InDemo() bool + + // Returns true if the terminal has a light background, going by + // gui.colorScheme, or by what the terminal tells us if that is 'auto' + TerminalHasLightBackground() bool } type IModeMgr interface { diff --git a/pkg/i18n/english.go b/pkg/i18n/english.go index efd84d722..cb320d721 100644 --- a/pkg/i18n/english.go +++ b/pkg/i18n/english.go @@ -728,6 +728,7 @@ type TranslationSet struct { OpenCommandLogMenuTooltip string ShowingGitDiff string ShowingDiffForRange string + CommitMessageChanges string CommitDiff string CopyCommitHashToClipboard string CommitHash string @@ -1919,6 +1920,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_beside_the_patch_marks.go b/pkg/integration/tests/commit/show_amend_commit_message_diff_beside_the_patch_marks.go new file mode 100644 index 000000000..0d841bd14 --- /dev/null +++ b/pkg/integration/tests/commit/show_amend_commit_message_diff_beside_the_patch_marks.go @@ -0,0 +1,43 @@ +package commit + +import ( + "github.com/jesseduffield/lazygit/pkg/config" + . "github.com/jesseduffield/lazygit/pkg/integration/components" +) + +var ShowAmendCommitMessageDiffBesideThePatchMarks = NewIntegrationTest(NewIntegrationTestArgs{ + Description: "The commit message diff of an amend! commit is laid out to the width that the custom patch's marks leave the diff below it", + ExtraCmdArgs: []string{}, + Skip: false, + SetupConfig: func(config *config.AppConfig) {}, + SetupRepo: func(shell *Shell) { + shell.CreateFileAndAdd("file", "one\n") + shell.Commit("Fix the widget") + shell.UpdateFileAndAdd("file", "one\ntwo\n") + shell.Commit("amend! Fix the widget\n\nFix the widget on startup") + }, + Run: func(t *TestDriver, keys config.KeybindingConfig) { + t.Views().Commits(). + Focus(). + Lines( + Contains("amend! Fix the widget").IsSelected(), + Contains("Fix the widget"), + ). + Press(keys.Universal.FocusMainView) + + // The rule below the message diff is as wide as the diff is laid out to, + // so it only fits on one line beside the marks if the message diff was + // laid out to the width they leave. + t.Views().Main(). + IsFocused(). + PressPrimaryAction(). + MarkedLines( + Contains("+two"), + ). + ContainsViewLines( + Equals("+Fix the widget on startup"), + MatchesRegexp("^─+$"), + Contains("commit "), + ) + }, +}) diff --git a/pkg/integration/tests/commit/show_amend_commit_message_diff_in_a_split_main_view.go b/pkg/integration/tests/commit/show_amend_commit_message_diff_in_a_split_main_view.go new file mode 100644 index 000000000..a49c1d827 --- /dev/null +++ b/pkg/integration/tests/commit/show_amend_commit_message_diff_in_a_split_main_view.go @@ -0,0 +1,47 @@ +package commit + +import ( + "github.com/jesseduffield/lazygit/pkg/config" + . "github.com/jesseduffield/lazygit/pkg/integration/components" +) + +var ShowAmendCommitMessageDiffInASplitMainView = NewIntegrationTest(NewIntegrationTestArgs{ + Description: "The commit message diff of an amend! commit is laid out to the width of the main view once building a custom patch splits it", + ExtraCmdArgs: []string{}, + Skip: false, + SetupConfig: func(cfg *config.AppConfig) { + // The pane showing the custom patch goes beside the diff, so the diff + // gets narrower when the patch is started. + cfg.GetUserConfig().Gui.MainPanelSplitMode = "horizontal" + }, + SetupRepo: func(shell *Shell) { + shell.CreateFileAndAdd("file", "one\n") + shell.Commit("Fix the widget") + shell.UpdateFileAndAdd("file", "one\ntwo\n") + shell.Commit("amend! Fix the widget\n\nFix the widget on startup") + }, + Run: func(t *TestDriver, keys config.KeybindingConfig) { + t.Views().Commits(). + Focus(). + Lines( + Contains("amend! Fix the widget").IsSelected(), + Contains("Fix the widget"), + ). + Press(keys.Universal.FocusMainView) + + // The rule below the message diff is as wide as the diff is laid out to, + // so it only fits on one line if the message diff was laid out to the + // width of the split view. + t.Views().Main(). + IsFocused(). + PressPrimaryAction(). + MarkedLines( + Contains("+two"), + ). + ContainsViewLines( + Equals("+Fix the widget on startup"), + MatchesRegexp("^─+$"), + Contains("commit "), + ) + }, +}) diff --git a/pkg/integration/tests/commit/show_amend_commit_message_diff_through_a_pipe.go b/pkg/integration/tests/commit/show_amend_commit_message_diff_through_a_pipe.go new file mode 100644 index 000000000..c5362ec46 --- /dev/null +++ b/pkg/integration/tests/commit/show_amend_commit_message_diff_through_a_pipe.go @@ -0,0 +1,48 @@ +package commit + +import ( + "github.com/jesseduffield/lazygit/pkg/config" + . "github.com/jesseduffield/lazygit/pkg/integration/components" +) + +var ShowAmendCommitMessageDiffThroughAPipe = NewIntegrationTest(NewIntegrationTestArgs{ + Description: "Show the commit message diff of an amend! commit through a diff renderer that is fed through a pipe rather than run in a pty", + ExtraCmdArgs: []string{}, + Skip: false, + // This is how a render works on Windows, where a pty can't carry a + // renderer's output faithfully. Ask for it here so that the path is + // covered on the platforms the integration tests do run on. + ExtraEnvVars: map[string]string{"LAZYGIT_RENDER_WITHOUT_PTY": "1"}, + SetupConfig: func(cfg *config.AppConfig) { + // Says so if it is writing into a pipe, then passes the diff through. + cfg.GetUserConfig().Git.DiffRenderers = []config.DiffRendererConfig{ + {Command: `[ -t 1 ] || echo "rendered into a pipe"; 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"), + ) + + t.Views().Main(). + TopLines( + Contains("Commit message changes compared to"), + Equals("rendered into a pipe"), + Equals("diff --git old message new message"), + ). + ContainsLines( + Equals("-Fix the widget"), + Equals("+Fix the widget on startup"), + ) + }, +}) 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/commit/show_amend_commit_message_diff_with_several_hunks.go b/pkg/integration/tests/commit/show_amend_commit_message_diff_with_several_hunks.go new file mode 100644 index 000000000..07ddec24b --- /dev/null +++ b/pkg/integration/tests/commit/show_amend_commit_message_diff_with_several_hunks.go @@ -0,0 +1,60 @@ +package commit + +import ( + "strings" + + "github.com/jesseduffield/lazygit/pkg/config" + . "github.com/jesseduffield/lazygit/pkg/integration/components" +) + +const showAmendCommitMessageDiffWithSeveralHunksBody = `The frobnicator was not initialised. + +It has to be initialised before the widget +draws itself for the first time, or the +widget shows garbage until it is resized. + +This only showed on startup, so nobody +noticed it for a long time. + +Fixes #123.` + +var ShowAmendCommitMessageDiffWithSeveralHunks = NewIntegrationTest(NewIntegrationTestArgs{ + Description: "The hunks of the commit message diff of an amend! commit are not headed by a line of the message", + ExtraCmdArgs: []string{}, + Skip: false, + SetupConfig: func(config *config.AppConfig) {}, + SetupRepo: func(shell *Shell) { + shell. + EmptyCommitWithBody("Fix the widget", showAmendCommitMessageDiffWithSeveralHunksBody). + EmptyCommitWithBody("amend! Fix the widget", + "Fix the widget on startup\n\n"+ + strings.Replace(showAmendCommitMessageDiffWithSeveralHunksBody, "#123", "#124", 1)) + }, + Run: func(t *TestDriver, keys config.KeybindingConfig) { + t.Views().Commits(). + Focus(). + Lines( + Contains("amend! Fix the widget").IsSelected(), + Contains("Fix the widget"), + ) + + // git would head the second hunk with the last line above it that + // starts with a letter, as if it named the function the hunk is in. + 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."), + Equals(" "), + Equals("@@ -9,4 +9,4 @@"), + Equals(" This only showed on startup, so nobody"), + Equals(" noticed it for a long time."), + Equals(" "), + Equals("-Fixes #123."), + Equals("+Fixes #124."), + Contains("───"), + ) + }, +}) diff --git a/pkg/integration/tests/test_list.go b/pkg/integration/tests/test_list.go index 54fe8f2b3..5ffe0d182 100644 --- a/pkg/integration/tests/test_list.go +++ b/pkg/integration/tests/test_list.go @@ -161,6 +161,12 @@ var tests = []*components.IntegrationTest{ commit.Search, commit.SetAuthor, commit.SetAuthorRange, + commit.ShowAmendCommitMessageDiff, + commit.ShowAmendCommitMessageDiffBesideThePatchMarks, + commit.ShowAmendCommitMessageDiffInASplitMainView, + commit.ShowAmendCommitMessageDiffThroughAPipe, + commit.ShowAmendCommitMessageDiffWithRenderer, + commit.ShowAmendCommitMessageDiffWithSeveralHunks, commit.StageRangeOfLines, commit.Staged, commit.StagedWithoutHooks,