Merge branch 'hotfix/cursor-positioning' of https://github.com/jesseduffield/lazygit into hotfix/cursor-positioning

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
+	Log                *logrus.Entry
-	OSCommand *OSCommand
+	OSCommand          *OSCommand
-	Worktree  *gogit.Worktree
+	Worktree           *gogit.Worktree
-	Repo      *gogit.Repository
+	Repo               *gogit.Repository
-	Tr        *i18n.Localizer
+	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,
+		Log:                log,
-		OSCommand: osCommand,
+		OSCommand:          osCommand,
-		Tr:        tr,
+		Tr:                 tr,
-		Worktree:  worktree,
+		Worktree:           worktree,
-		Repo:      repo,
+		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{}
+	appendedIndexes := []int{}
-	tailResults := []File{}
 
-	for _, newFile := range newFiles {
+	// retain position of files we already could see
-		var isHeadResult bool
+	result := []File{}
-
+	for _, oldFile := range oldFiles {
-		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 {
+func (c *GitCommand) usingGpg() bool {
-	gpgsign, _ := gitconfig.Global("commit.gpgsign")
+	gpgsign, _ := c.getLocalGitConfig("commit.gpgsign")
 	if gpgsign == "" {
-		gpgsign, _ = gitconfig.Local("commit.gpgsign")
+		gpgsign, _ = c.getGlobalGitConfig("commit.gpgsign")
 	}
-	if gpgsign == "" {
+	value := strings.ToLower(gpgsign)
-		return false
+
-	}
+	return value == "true" || value == "1" || value == "yes" || value == "on"
-	return true
 }
 
-// Commit commit to git
+// Commit commits to git
-func (c *GitCommand) Commit(g *gocui.Gui, message string) (*exec.Cmd, error) {
+func (c *GitCommand) Commit(message string) (*exec.Cmd, error) {
-	command := "git commit -m " + c.OSCommand.Quote(message)
+	command := fmt.Sprintf("git commit -m %s", c.OSCommand.Quote(message))
-	if c.UsingGpg() {
+	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

pkg/commands/git_test.go

@@ -56,9 +56,11 @@ func newDummyLog() *logrus.Entry {
 
 func newDummyGitCommand() *GitCommand {
 	return &GitCommand{
-		Log:       newDummyLog(),
+		Log:                newDummyLog(),
-		OSCommand: newDummyOSCommand(),
+		OSCommand:          newDummyOSCommand(),
-		Tr:        i18n.NewLocalizer(newDummyLog()),
+		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"))

pkg/commands/os.go

@@ -17,11 +17,12 @@ import (
 
 // Platform stores the os state
 type Platform struct {
-	os           string
+	os                   string
-	shell        string
+	shell                string
-	shellArg     string
+	shellArg             string
-	escapedQuote string
+	escapedQuote         string
-	openCommand  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

pkg/commands/os_default_platform.go

@@ -8,10 +8,11 @@ import (
 
 func getPlatform() *Platform {
 	return &Platform{
-		os:           runtime.GOOS,
+		os:                   runtime.GOOS,
-		shell:        "bash",
+		shell:                "bash",
-		shellArg:     "-c",
+		shellArg:             "-c",
-		escapedQuote: "\"",
+		escapedQuote:         "'",
-		openCommand:  "open {{filename}}",
+		openCommand:          "open {{filename}}",
+		fallbackEscapedQuote: "\"",
 	}
 }

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()
 

pkg/commands/os_windows.go

@@ -2,9 +2,10 @@ package commands
 
 func getPlatform() *Platform {
 	return &Platform{
-		os:           "windows",
+		os:                   "windows",
-		shell:        "cmd",
+		shell:                "cmd",
-		shellArg:     "/c",
+		shellArg:             "/c",
-		escapedQuote: `\"`,
+		escapedQuote:         `\"`,
+		fallbackEscapedQuote: "\\'",
 	}
 }

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 {