diff --git a/pkg/commands/git.go b/pkg/commands/git.go index f7a80dc63..5744fa6aa 100644 --- a/pkg/commands/git.go +++ b/pkg/commands/git.go @@ -7,7 +7,6 @@ import ( "os/exec" "strings" - "github.com/jesseduffield/gocui" "github.com/jesseduffield/lazygit/pkg/i18n" "github.com/jesseduffield/lazygit/pkg/utils" "github.com/sirupsen/logrus" @@ -59,11 +58,13 @@ func setupRepositoryAndWorktree(openGitRepository func(string) (*gogit.Repositor // GitCommand is our main git interface type GitCommand struct { - Log *logrus.Entry - OSCommand *OSCommand - Worktree *gogit.Worktree - Repo *gogit.Repository - Tr *i18n.Localizer + Log *logrus.Entry + OSCommand *OSCommand + Worktree *gogit.Worktree + Repo *gogit.Repository + Tr *i18n.Localizer + getGlobalGitConfig func(string) (string, error) + getLocalGitConfig func(string) (string, error) } // NewGitCommand it runs git commands @@ -92,11 +93,13 @@ func NewGitCommand(log *logrus.Entry, osCommand *OSCommand, tr *i18n.Localizer) } return &GitCommand{ - Log: log, - OSCommand: osCommand, - Tr: tr, - Worktree: worktree, - Repo: repo, + Log: log, + OSCommand: osCommand, + Tr: tr, + Worktree: worktree, + Repo: repo, + getGlobalGitConfig: gitconfig.Global, + getLocalGitConfig: gitconfig.Local, }, nil } @@ -170,28 +173,37 @@ func (c *GitCommand) MergeStatusFiles(oldFiles, newFiles []File) []File { return newFiles } - headResults := []File{} - tailResults := []File{} + appendedIndexes := []int{} - for _, newFile := range newFiles { - var isHeadResult bool - - for _, oldFile := range oldFiles { + // retain position of files we already could see + result := []File{} + for _, oldFile := range oldFiles { + for newIndex, newFile := range newFiles { if oldFile.Name == newFile.Name { - isHeadResult = true + result = append(result, newFile) + appendedIndexes = append(appendedIndexes, newIndex) break } } - - if isHeadResult { - headResults = append(headResults, newFile) - continue - } - - tailResults = append(tailResults, newFile) } - return append(headResults, tailResults...) + // append any new files to the end + for index, newFile := range newFiles { + if !includesInt(appendedIndexes, index) { + result = append(result, newFile) + } + } + + return result +} + +func includesInt(list []int, a int) bool { + for _, b := range list { + if b == a { + return true + } + } + return false } // GetBranchName branch name @@ -274,40 +286,41 @@ func (c *GitCommand) AbortMerge() error { return c.OSCommand.RunCommand("git merge --abort") } -// UsingGpg tells us whether the user has gpg enabled so that we can know +// usingGpg tells us whether the user has gpg enabled so that we can know // whether we need to run a subprocess to allow them to enter their password -func (c *GitCommand) UsingGpg() bool { - gpgsign, _ := gitconfig.Global("commit.gpgsign") +func (c *GitCommand) usingGpg() bool { + gpgsign, _ := c.getLocalGitConfig("commit.gpgsign") if gpgsign == "" { - gpgsign, _ = gitconfig.Local("commit.gpgsign") + gpgsign, _ = c.getGlobalGitConfig("commit.gpgsign") } - if gpgsign == "" { - return false - } - return true + value := strings.ToLower(gpgsign) + + return value == "true" || value == "1" || value == "yes" || value == "on" } -// Commit commit to git -func (c *GitCommand) Commit(g *gocui.Gui, message string) (*exec.Cmd, error) { - command := "git commit -m " + c.OSCommand.Quote(message) - if c.UsingGpg() { +// Commit commits to git +func (c *GitCommand) Commit(message string) (*exec.Cmd, error) { + command := fmt.Sprintf("git commit -m %s", c.OSCommand.Quote(message)) + if c.usingGpg() { return c.OSCommand.PrepareSubProcess(c.OSCommand.Platform.shell, c.OSCommand.Platform.shellArg, command), nil } + return nil, c.OSCommand.RunCommand(command) } -// Pull pull from repo +// Pull pulls from repo func (c *GitCommand) Pull() error { return c.OSCommand.RunCommand("git pull --no-edit") } -// Push push to a branch +// Push pushes to a branch func (c *GitCommand) Push(branchName string, force bool) error { forceFlag := "" if force { forceFlag = "--force-with-lease " } - return c.OSCommand.RunCommand("git push " + forceFlag + "-u origin " + branchName) + + return c.OSCommand.RunCommand(fmt.Sprintf("git push %s -u origin %s", forceFlag, branchName)) } // SquashPreviousTwoCommits squashes a commit down to the one below it diff --git a/pkg/commands/git_test.go b/pkg/commands/git_test.go index 7bc7a8910..bda3ea225 100644 --- a/pkg/commands/git_test.go +++ b/pkg/commands/git_test.go @@ -56,9 +56,11 @@ func newDummyLog() *logrus.Entry { func newDummyGitCommand() *GitCommand { return &GitCommand{ - Log: newDummyLog(), - OSCommand: newDummyOSCommand(), - Tr: i18n.NewLocalizer(newDummyLog()), + Log: newDummyLog(), + OSCommand: newDummyOSCommand(), + Tr: i18n.NewLocalizer(newDummyLog()), + getGlobalGitConfig: func(string) (string, error) { return "", nil }, + getLocalGitConfig: func(string) (string, error) { return "", nil }, } } @@ -730,6 +732,227 @@ func TestGitCommandMerge(t *testing.T) { assert.NoError(t, gitCmd.Merge("test")) } +func TestGitCommandUsingGpg(t *testing.T) { + type scenario struct { + testName string + getLocalGitConfig func(string) (string, error) + getGlobalGitConfig func(string) (string, error) + test func(bool) + } + + scenarios := []scenario{ + { + "Option global and local config commit.gpgsign is not set", + func(string) (string, error) { + return "", nil + }, + func(string) (string, error) { + return "", nil + }, + func(gpgEnabled bool) { + assert.False(t, gpgEnabled) + }, + }, + { + "Option global config commit.gpgsign is not set, fallback on local config", + func(string) (string, error) { + return "", nil + }, + func(string) (string, error) { + return "true", nil + }, + func(gpgEnabled bool) { + assert.True(t, gpgEnabled) + }, + }, + { + "Option commit.gpgsign is true", + func(string) (string, error) { + return "True", nil + }, + func(string) (string, error) { + return "", nil + }, + func(gpgEnabled bool) { + assert.True(t, gpgEnabled) + }, + }, + { + "Option commit.gpgsign is on", + func(string) (string, error) { + return "ON", nil + }, + func(string) (string, error) { + return "", nil + }, + func(gpgEnabled bool) { + assert.True(t, gpgEnabled) + }, + }, + { + "Option commit.gpgsign is yes", + func(string) (string, error) { + return "YeS", nil + }, + func(string) (string, error) { + return "", nil + }, + func(gpgEnabled bool) { + assert.True(t, gpgEnabled) + }, + }, + { + "Option commit.gpgsign is 1", + func(string) (string, error) { + return "1", nil + }, + func(string) (string, error) { + return "", nil + }, + func(gpgEnabled bool) { + assert.True(t, gpgEnabled) + }, + }, + } + + for _, s := range scenarios { + t.Run(s.testName, func(t *testing.T) { + gitCmd := newDummyGitCommand() + gitCmd.getGlobalGitConfig = s.getGlobalGitConfig + gitCmd.getLocalGitConfig = s.getLocalGitConfig + s.test(gitCmd.usingGpg()) + }) + } +} + +func TestGitCommandCommit(t *testing.T) { + type scenario struct { + testName string + command func(string, ...string) *exec.Cmd + getGlobalGitConfig func(string) (string, error) + test func(*exec.Cmd, error) + } + + scenarios := []scenario{ + { + "Commit using gpg", + func(cmd string, args ...string) *exec.Cmd { + assert.EqualValues(t, "bash", cmd) + assert.EqualValues(t, []string{"-c", `git commit -m 'test'`}, args) + + return exec.Command("echo") + }, + func(string) (string, error) { + return "true", nil + }, + func(cmd *exec.Cmd, err error) { + assert.NotNil(t, cmd) + assert.Nil(t, err) + }, + }, + { + "Commit without using gpg", + func(cmd string, args ...string) *exec.Cmd { + assert.EqualValues(t, "git", cmd) + assert.EqualValues(t, []string{"commit", "-m", "test"}, args) + + return exec.Command("echo") + }, + func(string) (string, error) { + return "false", nil + }, + func(cmd *exec.Cmd, err error) { + assert.Nil(t, cmd) + assert.Nil(t, err) + }, + }, + { + "Commit without using gpg with an error", + func(cmd string, args ...string) *exec.Cmd { + assert.EqualValues(t, "git", cmd) + assert.EqualValues(t, []string{"commit", "-m", "test"}, args) + + return exec.Command("exit", "1") + }, + func(string) (string, error) { + return "false", nil + }, + func(cmd *exec.Cmd, err error) { + assert.Nil(t, cmd) + assert.Error(t, err) + }, + }, + } + + for _, s := range scenarios { + t.Run(s.testName, func(t *testing.T) { + gitCmd := newDummyGitCommand() + gitCmd.getGlobalGitConfig = s.getGlobalGitConfig + gitCmd.OSCommand.command = s.command + s.test(gitCmd.Commit("test")) + }) + } +} + +func TestGitCommandPush(t *testing.T) { + type scenario struct { + testName string + command func(string, ...string) *exec.Cmd + forcePush bool + test func(error) + } + + scenarios := []scenario{ + { + "Push with force disabled", + func(cmd string, args ...string) *exec.Cmd { + assert.EqualValues(t, "git", cmd) + assert.EqualValues(t, []string{"push", "-u", "origin", "test"}, args) + + return exec.Command("echo") + }, + false, + func(err error) { + assert.Nil(t, err) + }, + }, + { + "Push with force enable", + func(cmd string, args ...string) *exec.Cmd { + assert.EqualValues(t, "git", cmd) + assert.EqualValues(t, []string{"push", "--force-with-lease", "-u", "origin", "test"}, args) + + return exec.Command("echo") + }, + true, + func(err error) { + assert.Nil(t, err) + }, + }, + { + "Push with an error occurring", + func(cmd string, args ...string) *exec.Cmd { + assert.EqualValues(t, "git", cmd) + assert.EqualValues(t, []string{"push", "-u", "origin", "test"}, args) + + return exec.Command("exit", "1") + }, + false, + func(err error) { + assert.Error(t, err) + }, + }, + } + + for _, s := range scenarios { + t.Run(s.testName, func(t *testing.T) { + gitCmd := newDummyGitCommand() + gitCmd.OSCommand.command = s.command + s.test(gitCmd.Push("test", s.forcePush)) + }) + } +} + func TestGitCommandDiff(t *testing.T) { gitCommand := newDummyGitCommand() assert.NoError(t, test.GenerateRepo("lots_of_diffs.sh")) diff --git a/pkg/commands/os.go b/pkg/commands/os.go index 834c45376..c8ca40f29 100644 --- a/pkg/commands/os.go +++ b/pkg/commands/os.go @@ -17,11 +17,12 @@ import ( // Platform stores the os state type Platform struct { - os string - shell string - shellArg string - escapedQuote string - openCommand string + os string + shell string + shellArg string + escapedQuote string + openCommand string + fallbackEscapedQuote string } // OSCommand holds all the os commands @@ -140,7 +141,11 @@ func (c *OSCommand) PrepareSubProcess(cmdName string, commandArgs ...string) *ex // Quote wraps a message in platform-specific quotation marks func (c *OSCommand) Quote(message string) string { message = strings.Replace(message, "`", "\\`", -1) - return c.Platform.escapedQuote + message + c.Platform.escapedQuote + escapedQuote := c.Platform.escapedQuote + if strings.Contains(message, c.Platform.escapedQuote) { + escapedQuote = c.Platform.fallbackEscapedQuote + } + return escapedQuote + message + escapedQuote } // Unquote removes wrapping quotations marks if they are present diff --git a/pkg/commands/os_default_platform.go b/pkg/commands/os_default_platform.go index f106bbd62..7b063417b 100644 --- a/pkg/commands/os_default_platform.go +++ b/pkg/commands/os_default_platform.go @@ -8,10 +8,11 @@ import ( func getPlatform() *Platform { return &Platform{ - os: runtime.GOOS, - shell: "bash", - shellArg: "-c", - escapedQuote: "\"", - openCommand: "open {{filename}}", + os: runtime.GOOS, + shell: "bash", + shellArg: "-c", + escapedQuote: "'", + openCommand: "open {{filename}}", + fallbackEscapedQuote: "\"", } } diff --git a/pkg/commands/os_test.go b/pkg/commands/os_test.go index 01173fb15..aeef4a6e5 100644 --- a/pkg/commands/os_test.go +++ b/pkg/commands/os_test.go @@ -265,6 +265,32 @@ func TestOSCommandQuote(t *testing.T) { assert.EqualValues(t, expected, actual) } +// TestOSCommandQuoteSingleQuote tests the quote function with ' quotes explicitly for Linux +func TestOSCommandQuoteSingleQuote(t *testing.T) { + osCommand := newDummyOSCommand() + + osCommand.Platform.os = "linux" + + actual := osCommand.Quote("hello 'test'") + + expected := osCommand.Platform.fallbackEscapedQuote + "hello 'test'" + osCommand.Platform.fallbackEscapedQuote + + assert.EqualValues(t, expected, actual) +} + +// TestOSCommandQuoteSingleQuote tests the quote function with " quotes explicitly for Linux +func TestOSCommandQuoteDoubleQuote(t *testing.T) { + osCommand := newDummyOSCommand() + + osCommand.Platform.os = "linux" + + actual := osCommand.Quote(`hello "test"`) + + expected := osCommand.Platform.escapedQuote + "hello \"test\"" + osCommand.Platform.escapedQuote + + assert.EqualValues(t, expected, actual) +} + func TestOSCommandUnquote(t *testing.T) { osCommand := newDummyOSCommand() diff --git a/pkg/commands/os_windows.go b/pkg/commands/os_windows.go index 1658e5f36..8fa9ce1c2 100644 --- a/pkg/commands/os_windows.go +++ b/pkg/commands/os_windows.go @@ -2,9 +2,10 @@ package commands func getPlatform() *Platform { return &Platform{ - os: "windows", - shell: "cmd", - shellArg: "/c", - escapedQuote: `\"`, + os: "windows", + shell: "cmd", + shellArg: "/c", + escapedQuote: `\"`, + fallbackEscapedQuote: "\\'", } } diff --git a/pkg/gui/commit_message_panel.go b/pkg/gui/commit_message_panel.go index 99d649102..e23c47da0 100644 --- a/pkg/gui/commit_message_panel.go +++ b/pkg/gui/commit_message_panel.go @@ -12,7 +12,7 @@ func (gui *Gui) handleCommitConfirm(g *gocui.Gui, v *gocui.View) error { if message == "" { return gui.createErrorPanel(g, gui.Tr.SLocalize("CommitWithoutMessageErr")) } - sub, err := gui.GitCommand.Commit(g, message) + sub, err := gui.GitCommand.Commit(message) if err != nil { // TODO need to find a way to send through this error if err != gui.Errors.ErrSubProcess {