diff --git a/pkg/commands/git.go b/pkg/commands/git.go index 262a57c2f..1d8e5a10d 100644 --- a/pkg/commands/git.go +++ b/pkg/commands/git.go @@ -3,7 +3,6 @@ package commands import ( "errors" "fmt" - "io/ioutil" "os" "os/exec" "strings" @@ -486,7 +485,6 @@ func (c *GitCommand) getMergeBase() (string, error) { output, err := c.OSCommand.RunCommandWithOutput(fmt.Sprintf("git merge-base HEAD %s", baseBranch)) if err != nil { // swallowing error because it's not a big deal; probably because there are no commits yet - c.Log.Error(err) } return output, nil } @@ -595,24 +593,13 @@ func (c *GitCommand) Diff(file *File, plain bool) string { } func (c *GitCommand) ApplyPatch(patch string) (string, error) { - - content := []byte(patch) - tmpfile, err := ioutil.TempFile("", "patch") + filename, err := c.OSCommand.CreateTempFile("patch", patch) if err != nil { - c.Log.Error(err) - return "", errors.New("Could not create patch file") // TODO: i18n - } - - defer os.Remove(tmpfile.Name()) // clean up - - if _, err := tmpfile.Write(content); err != nil { - c.Log.Error(err) - return "", err - } - if err := tmpfile.Close(); err != nil { c.Log.Error(err) return "", err } - return c.OSCommand.RunCommandWithOutput(fmt.Sprintf("git apply --cached %s", tmpfile.Name())) + defer func() { _ = c.OSCommand.RemoveFile(filename) }() + + return c.OSCommand.RunCommandWithOutput(fmt.Sprintf("git apply --cached %s", filename)) } diff --git a/pkg/commands/git_test.go b/pkg/commands/git_test.go index 0e1390535..206696b22 100644 --- a/pkg/commands/git_test.go +++ b/pkg/commands/git_test.go @@ -1770,6 +1770,7 @@ func TestGitCommandDiff(t *testing.T) { testName string command func(string, ...string) *exec.Cmd file *File + plain bool } scenarios := []scenario{ @@ -1786,6 +1787,22 @@ func TestGitCommandDiff(t *testing.T) { HasStagedChanges: false, Tracked: true, }, + false, + }, + { + "Default case", + func(cmd string, args ...string) *exec.Cmd { + assert.EqualValues(t, "git", cmd) + assert.EqualValues(t, []string{"diff", "--", "test.txt"}, args) + + return exec.Command("echo") + }, + &File{ + Name: "test.txt", + HasStagedChanges: false, + Tracked: true, + }, + true, }, { "All changes staged", @@ -1801,6 +1818,7 @@ func TestGitCommandDiff(t *testing.T) { HasUnstagedChanges: false, Tracked: true, }, + false, }, { "File not tracked and file has no staged changes", @@ -1815,6 +1833,7 @@ func TestGitCommandDiff(t *testing.T) { HasStagedChanges: false, Tracked: false, }, + false, }, } @@ -1822,7 +1841,7 @@ func TestGitCommandDiff(t *testing.T) { t.Run(s.testName, func(t *testing.T) { gitCmd := newDummyGitCommand() gitCmd.OSCommand.command = s.command - gitCmd.Diff(s.file, false) + gitCmd.Diff(s.file, s.plain) }) } } @@ -1979,3 +1998,61 @@ func TestGitCommandCurrentBranchName(t *testing.T) { }) } } + +func TestGitCommandApplyPatch(t *testing.T) { + type scenario struct { + testName string + command func(string, ...string) *exec.Cmd + test func(string, error) + } + + scenarios := []scenario{ + { + "valid case", + func(cmd string, args ...string) *exec.Cmd { + assert.Equal(t, "git", cmd) + assert.EqualValues(t, []string{"apply", "--cached"}, args[0:2]) + filename := args[2] + content, err := ioutil.ReadFile(filename) + assert.NoError(t, err) + + assert.Equal(t, "test", string(content)) + + return exec.Command("echo", "done") + }, + func(output string, err error) { + assert.NoError(t, err) + assert.EqualValues(t, "done\n", output) + }, + }, + { + "command returns error", + func(cmd string, args ...string) *exec.Cmd { + assert.Equal(t, "git", cmd) + assert.EqualValues(t, []string{"apply", "--cached"}, args[0:2]) + filename := args[2] + // TODO: Ideally we want to mock out OSCommand here so that we're not + // double handling testing it's CreateTempFile functionality, + // but it is going to take a bit of work to make a proper mock for it + // so I'm leaving it for another PR + content, err := ioutil.ReadFile(filename) + assert.NoError(t, err) + + assert.Equal(t, "test", string(content)) + + return exec.Command("test") + }, + func(output string, 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.ApplyPatch("test")) + }) + } +} diff --git a/pkg/commands/os.go b/pkg/commands/os.go index 8b4b7879e..6b28a69bb 100644 --- a/pkg/commands/os.go +++ b/pkg/commands/os.go @@ -2,6 +2,7 @@ package commands import ( "errors" + "io/ioutil" "os" "os/exec" "strings" @@ -176,3 +177,28 @@ func (c *OSCommand) AppendLineToFile(filename, line string) error { _, err = f.WriteString("\n" + line) return err } + +// CreateTempFile writes a string to a new temp file and returns the file's name +func (c *OSCommand) CreateTempFile(filename, content string) (string, error) { + tmpfile, err := ioutil.TempFile("", filename) + if err != nil { + c.Log.Error(err) + return "", err + } + + if _, err := tmpfile.Write([]byte(content)); err != nil { + c.Log.Error(err) + return "", err + } + if err := tmpfile.Close(); err != nil { + c.Log.Error(err) + return "", err + } + + return tmpfile.Name(), nil +} + +// RemoveFile removes a file at the specified path +func (c *OSCommand) RemoveFile(filename string) error { + return os.Remove(filename) +} diff --git a/pkg/commands/os_test.go b/pkg/commands/os_test.go index ebb855cbe..a08c4b57d 100644 --- a/pkg/commands/os_test.go +++ b/pkg/commands/os_test.go @@ -1,6 +1,7 @@ package commands import ( + "io/ioutil" "os" "os/exec" "testing" @@ -364,3 +365,34 @@ func TestOSCommandFileType(t *testing.T) { _ = os.RemoveAll(s.path) } } + +func TestOSCommandCreateTempFile(t *testing.T) { + type scenario struct { + testName string + filename string + content string + test func(string, error) + } + + scenarios := []scenario{ + { + "valid case", + "filename", + "content", + func(path string, err error) { + assert.NoError(t, err) + + content, err := ioutil.ReadFile(path) + assert.NoError(t, err) + + assert.Equal(t, "content", string(content)) + }, + }, + } + + for _, s := range scenarios { + t.Run(s.testName, func(t *testing.T) { + s.test(newDummyOSCommand().CreateTempFile(s.filename, s.content)) + }) + } +} diff --git a/pkg/git/patch_modifier.go b/pkg/git/patch_modifier.go index 05afcf5ba..3c523232e 100644 --- a/pkg/git/patch_modifier.go +++ b/pkg/git/patch_modifier.go @@ -6,11 +6,14 @@ import ( "strconv" "strings" + "github.com/jesseduffield/lazygit/pkg/i18n" + "github.com/jesseduffield/lazygit/pkg/utils" "github.com/sirupsen/logrus" ) type PatchModifier struct { Log *logrus.Entry + Tr *i18n.Localizer } // NewPatchModifier builds a new branch list builder @@ -20,11 +23,49 @@ func NewPatchModifier(log *logrus.Entry) (*PatchModifier, error) { }, nil } -// ModifyPatch takes the original patch, which may contain several hunks, -// and the line number of the line we want to stage -func (p *PatchModifier) ModifyPatch(patch string, lineNumber int) (string, error) { +// ModifyPatchForHunk takes the original patch, which may contain several hunks, +// and removes any hunks that aren't the selected hunk +func (p *PatchModifier) ModifyPatchForHunk(patch string, hunkStarts []int, currentLine int) (string, error) { + // get hunk start and end lines := strings.Split(patch, "\n") - headerLength := 4 + hunkStartIndex := utils.PrevIndex(hunkStarts, currentLine) + hunkStart := hunkStarts[hunkStartIndex] + nextHunkStartIndex := utils.NextIndex(hunkStarts, currentLine) + var hunkEnd int + if nextHunkStartIndex == 0 { + hunkEnd = len(lines) - 1 + } else { + hunkEnd = hunkStarts[nextHunkStartIndex] + } + + headerLength, err := p.getHeaderLength(lines) + if err != nil { + return "", err + } + + output := strings.Join(lines[0:headerLength], "\n") + "\n" + output += strings.Join(lines[hunkStart:hunkEnd], "\n") + "\n" + + return output, nil +} + +func (p *PatchModifier) getHeaderLength(patchLines []string) (int, error) { + for index, line := range patchLines { + if strings.HasPrefix(line, "@@") { + return index, nil + } + } + return 0, errors.New(p.Tr.SLocalize("CantFindHunks")) +} + +// ModifyPatchForLine takes the original patch, which may contain several hunks, +// and the line number of the line we want to stage +func (p *PatchModifier) ModifyPatchForLine(patch string, lineNumber int) (string, error) { + lines := strings.Split(patch, "\n") + headerLength, err := p.getHeaderLength(lines) + if err != nil { + return "", err + } output := strings.Join(lines[0:headerLength], "\n") + "\n" hunkStart, err := p.getHunkStart(lines, lineNumber) @@ -55,7 +96,8 @@ func (p *PatchModifier) getHunkStart(patchLines []string, lineNumber int) (int, return hunkStart, nil } } - return 0, errors.New("Could not find hunk") + + return 0, errors.New(p.Tr.SLocalize("CantFindHunk")) } func (p *PatchModifier) getModifiedHunk(patchLines []string, hunkStart int, lineNumber int) ([]string, error) { @@ -101,13 +143,8 @@ func (p *PatchModifier) getModifiedHunk(patchLines []string, hunkStart int, line // @@ -14,8 +14,9 @@ import ( func (p *PatchModifier) updatedHeader(currentHeader string, lineChanges int) (string, error) { // current counter is the number after the second comma - re := regexp.MustCompile(`^[^,]+,[^,]+,(\d+)`) - matches := re.FindStringSubmatch(currentHeader) - if len(matches) < 2 { - re = regexp.MustCompile(`^[^,]+,[^+]+\+(\d+)`) - matches = re.FindStringSubmatch(currentHeader) - } - prevLengthString := matches[1] + re := regexp.MustCompile(`(\d+) @@`) + prevLengthString := re.FindStringSubmatch(currentHeader)[1] prevLength, err := strconv.Atoi(prevLengthString) if err != nil { diff --git a/pkg/git/patch_modifier_test.go b/pkg/git/patch_modifier_test.go index af7be3751..bc2073d55 100644 --- a/pkg/git/patch_modifier_test.go +++ b/pkg/git/patch_modifier_test.go @@ -19,7 +19,7 @@ func newDummyPatchModifier() *PatchModifier { Log: newDummyLog(), } } -func TestModifyPatch(t *testing.T) { +func TestModifyPatchForLine(t *testing.T) { type scenario struct { testName string patchFilename string @@ -43,6 +43,27 @@ func TestModifyPatch(t *testing.T) { false, "testdata/testPatchAfter2.diff", }, + { + "Adding one line in top hunk in diff with multiple hunks", + "testdata/testPatchBefore2.diff", + 20, + false, + "testdata/testPatchAfter3.diff", + }, + { + "Adding one line in top hunk in diff with multiple hunks", + "testdata/testPatchBefore2.diff", + 53, + false, + "testdata/testPatchAfter4.diff", + }, + { + "adding unstaged file with a single line", + "testdata/addedFile.diff", + 6, + false, + "testdata/addedFile.diff", + }, } for _, s := range scenarios { @@ -52,7 +73,7 @@ func TestModifyPatch(t *testing.T) { if err != nil { panic("Cannot open file at " + s.patchFilename) } - afterPatch, err := p.ModifyPatch(string(beforePatch), s.lineNumber) + afterPatch, err := p.ModifyPatchForLine(string(beforePatch), s.lineNumber) if s.shouldError { assert.Error(t, err) } else { diff --git a/pkg/git/patch_parser.go b/pkg/git/patch_parser.go index 8fa4d355b..1dbacd01c 100644 --- a/pkg/git/patch_parser.go +++ b/pkg/git/patch_parser.go @@ -21,15 +21,16 @@ func (p *PatchParser) ParsePatch(patch string) ([]int, []int, error) { lines := strings.Split(patch, "\n") hunkStarts := []int{} stageableLines := []int{} - headerLength := 4 - for offsetIndex, line := range lines[headerLength:] { - index := offsetIndex + headerLength + pastHeader := false + for index, line := range lines { if strings.HasPrefix(line, "@@") { + pastHeader = true hunkStarts = append(hunkStarts, index) } - if strings.HasPrefix(line, "-") || strings.HasPrefix(line, "+") { + if pastHeader && (strings.HasPrefix(line, "-") || strings.HasPrefix(line, "+")) { stageableLines = append(stageableLines, index) } } + p.Log.WithField("staging", "staging").Info(stageableLines) return hunkStarts, stageableLines, nil } diff --git a/pkg/git/patch_parser_test.go b/pkg/git/patch_parser_test.go new file mode 100644 index 000000000..6670aaea2 --- /dev/null +++ b/pkg/git/patch_parser_test.go @@ -0,0 +1,65 @@ +package git + +import ( + "io/ioutil" + "testing" + + "github.com/stretchr/testify/assert" +) + +func newDummyPatchParser() *PatchParser { + return &PatchParser{ + Log: newDummyLog(), + } +} +func TestParsePatch(t *testing.T) { + type scenario struct { + testName string + patchFilename string + shouldError bool + expectedStageableLines []int + expectedHunkStarts []int + } + + scenarios := []scenario{ + { + "Diff with one hunk", + "testdata/testPatchBefore.diff", + false, + []int{8, 9, 10, 11}, + []int{4}, + }, + { + "Diff with two hunks", + "testdata/testPatchBefore2.diff", + false, + []int{8, 9, 10, 11, 12, 13, 20, 21, 22, 23, 24, 25, 26, 27, 28, 33, 34, 35, 36, 37, 45, 46, 47, 48, 49, 50, 51, 52, 53}, + []int{4, 41}, + }, + { + "Unstaged file", + "testdata/addedFile.diff", + false, + []int{6}, + []int{5}, + }, + } + + for _, s := range scenarios { + t.Run(s.testName, func(t *testing.T) { + p := newDummyPatchParser() + beforePatch, err := ioutil.ReadFile(s.patchFilename) + if err != nil { + panic("Cannot open file at " + s.patchFilename) + } + hunkStarts, stageableLines, err := p.ParsePatch(string(beforePatch)) + if s.shouldError { + assert.Error(t, err) + } else { + assert.NoError(t, err) + assert.Equal(t, s.expectedStageableLines, stageableLines) + assert.Equal(t, s.expectedHunkStarts, hunkStarts) + } + }) + } +} diff --git a/pkg/git/testdata/addedFile.diff b/pkg/git/testdata/addedFile.diff new file mode 100644 index 000000000..53966c4a1 --- /dev/null +++ b/pkg/git/testdata/addedFile.diff @@ -0,0 +1,7 @@ +diff --git a/blah b/blah +new file mode 100644 +index 0000000..907b308 +--- /dev/null ++++ b/blah +@@ -0,0 +1 @@ ++blah diff --git a/pkg/git/testdata/testPatchAfter3.diff b/pkg/git/testdata/testPatchAfter3.diff new file mode 100644 index 000000000..03492450d --- /dev/null +++ b/pkg/git/testdata/testPatchAfter3.diff @@ -0,0 +1,25 @@ +diff --git a/pkg/git/patch_modifier.go b/pkg/git/patch_modifier.go +index a8fc600..6d8f7d7 100644 +--- a/pkg/git/patch_modifier.go ++++ b/pkg/git/patch_modifier.go +@@ -36,18 +36,19 @@ func (p *PatchModifier) ModifyPatchForHunk(patch string, hunkStarts []int, curre + hunkEnd = hunkStarts[nextHunkStartIndex] + } + + headerLength := 4 + output := strings.Join(lines[0:headerLength], "\n") + "\n" + output += strings.Join(lines[hunkStart:hunkEnd], "\n") + "\n" + + return output, nil + } + ++func getHeaderLength(patchLines []string) (int, error) { + // ModifyPatchForLine takes the original patch, which may contain several hunks, + // and the line number of the line we want to stage + func (p *PatchModifier) ModifyPatchForLine(patch string, lineNumber int) (string, error) { + lines := strings.Split(patch, "\n") + headerLength := 4 + output := strings.Join(lines[0:headerLength], "\n") + "\n" + + hunkStart, err := p.getHunkStart(lines, lineNumber) + diff --git a/pkg/git/testdata/testPatchAfter4.diff b/pkg/git/testdata/testPatchAfter4.diff new file mode 100644 index 000000000..99f894d9d --- /dev/null +++ b/pkg/git/testdata/testPatchAfter4.diff @@ -0,0 +1,19 @@ +diff --git a/pkg/git/patch_modifier.go b/pkg/git/patch_modifier.go +index a8fc600..6d8f7d7 100644 +--- a/pkg/git/patch_modifier.go ++++ b/pkg/git/patch_modifier.go +@@ -124,13 +140,14 @@ func (p *PatchModifier) getModifiedHunk(patchLines []string, hunkStart int, line + // @@ -14,8 +14,9 @@ import ( + func (p *PatchModifier) updatedHeader(currentHeader string, lineChanges int) (string, error) { + // current counter is the number after the second comma + re := regexp.MustCompile(`^[^,]+,[^,]+,(\d+)`) + matches := re.FindStringSubmatch(currentHeader) + if len(matches) < 2 { + re = regexp.MustCompile(`^[^,]+,[^+]+\+(\d+)`) + matches = re.FindStringSubmatch(currentHeader) + } + prevLengthString := matches[1] ++ prevLengthString := re.FindStringSubmatch(currentHeader)[1] + + prevLength, err := strconv.Atoi(prevLengthString) + if err != nil { diff --git a/pkg/git/testdata/testPatchBefore2.diff b/pkg/git/testdata/testPatchBefore2.diff new file mode 100644 index 000000000..552c04f5e --- /dev/null +++ b/pkg/git/testdata/testPatchBefore2.diff @@ -0,0 +1,57 @@ +diff --git a/pkg/git/patch_modifier.go b/pkg/git/patch_modifier.go +index a8fc600..6d8f7d7 100644 +--- a/pkg/git/patch_modifier.go ++++ b/pkg/git/patch_modifier.go +@@ -36,18 +36,34 @@ func (p *PatchModifier) ModifyPatchForHunk(patch string, hunkStarts []int, curre + hunkEnd = hunkStarts[nextHunkStartIndex] + } + +- headerLength := 4 ++ headerLength, err := getHeaderLength(lines) ++ if err != nil { ++ return "", err ++ } ++ + output := strings.Join(lines[0:headerLength], "\n") + "\n" + output += strings.Join(lines[hunkStart:hunkEnd], "\n") + "\n" + + return output, nil + } + ++func getHeaderLength(patchLines []string) (int, error) { ++ for index, line := range patchLines { ++ if strings.HasPrefix(line, "@@") { ++ return index, nil ++ } ++ } ++ return 0, errors.New("Could not find any hunks in this patch") ++} ++ + // ModifyPatchForLine takes the original patch, which may contain several hunks, + // and the line number of the line we want to stage + func (p *PatchModifier) ModifyPatchForLine(patch string, lineNumber int) (string, error) { + lines := strings.Split(patch, "\n") +- headerLength := 4 ++ headerLength, err := getHeaderLength(lines) ++ if err != nil { ++ return "", err ++ } + output := strings.Join(lines[0:headerLength], "\n") + "\n" + + hunkStart, err := p.getHunkStart(lines, lineNumber) +@@ -124,13 +140,8 @@ func (p *PatchModifier) getModifiedHunk(patchLines []string, hunkStart int, line + // @@ -14,8 +14,9 @@ import ( + func (p *PatchModifier) updatedHeader(currentHeader string, lineChanges int) (string, error) { + // current counter is the number after the second comma +- re := regexp.MustCompile(`^[^,]+,[^,]+,(\d+)`) +- matches := re.FindStringSubmatch(currentHeader) +- if len(matches) < 2 { +- re = regexp.MustCompile(`^[^,]+,[^+]+\+(\d+)`) +- matches = re.FindStringSubmatch(currentHeader) +- } +- prevLengthString := matches[1] ++ re := regexp.MustCompile(`(\d+) @@`) ++ prevLengthString := re.FindStringSubmatch(currentHeader)[1] + + prevLength, err := strconv.Atoi(prevLengthString) + if err != nil { diff --git a/pkg/gui/confirmation_panel.go b/pkg/gui/confirmation_panel.go index 58577e430..afe53ca2c 100644 --- a/pkg/gui/confirmation_panel.go +++ b/pkg/gui/confirmation_panel.go @@ -8,6 +8,7 @@ package gui import ( "strings" + "time" "github.com/fatih/color" "github.com/jesseduffield/gocui" @@ -73,6 +74,7 @@ func (gui *Gui) prepareConfirmationPanel(currentView *gocui.View, title, prompt return nil, err } confirmationView.Title = title + confirmationView.Wrap = true confirmationView.FgColor = gocui.ColorWhite } confirmationView.Clear() @@ -137,7 +139,15 @@ func (gui *Gui) createMessagePanel(g *gocui.Gui, currentView *gocui.View, title, } func (gui *Gui) createErrorPanel(g *gocui.Gui, message string) error { - gui.Log.Error(message) + go func() { + // when reporting is switched on this log call sometimes introduces + // a delay on the error panel popping up. Here I'm adding a second wait + // so that the error is logged while the user is reading the error message + time.Sleep(time.Second) + gui.Log.Error(message) + }() + + // gui.Log.WithField("staging", "staging").Info("creating confirmation panel") currentView := g.CurrentView() colorFunction := color.New(color.FgRed).SprintFunc() coloredMessage := colorFunction(strings.TrimSpace(message)) diff --git a/pkg/gui/files_panel.go b/pkg/gui/files_panel.go index 3404903ef..a16ef4909 100644 --- a/pkg/gui/files_panel.go +++ b/pkg/gui/files_panel.go @@ -57,10 +57,13 @@ func (gui *Gui) handleSwitchToStagingPanel(g *gocui.Gui, v *gocui.View) error { } return nil } - if !file.Tracked || !file.HasUnstagedChanges { + if !file.HasUnstagedChanges { + gui.Log.WithField("staging", "staging").Info("making error panel") return gui.createErrorPanel(g, gui.Tr.SLocalize("FileStagingRequirements")) } - gui.switchFocus(g, v, stagingView) + if err := gui.switchFocus(g, v, stagingView); err != nil { + return err + } return gui.refreshStagingPanel() } diff --git a/pkg/gui/gui.go b/pkg/gui/gui.go index 67415c5ce..9f25121d5 100644 --- a/pkg/gui/gui.go +++ b/pkg/gui/gui.go @@ -222,7 +222,6 @@ func (gui *Gui) layout(g *gocui.Gui) error { return err } v.Title = gui.Tr.SLocalize("StagingTitle") - v.Wrap = true v.Highlight = true v.FgColor = gocui.ColorWhite if _, err := g.SetViewOnBottom("staging"); err != nil { diff --git a/pkg/gui/keybindings.go b/pkg/gui/keybindings.go index 2c47ab4f3..4158bedb7 100644 --- a/pkg/gui/keybindings.go +++ b/pkg/gui/keybindings.go @@ -392,25 +392,64 @@ func (gui *Gui) GetKeybindings() []*Binding { Modifier: gocui.ModNone, Handler: gui.handleMenuClose, }, { - ViewName: "staging", - Key: gocui.KeyEsc, - Modifier: gocui.ModNone, - Handler: gui.handleStagingEscape, + ViewName: "staging", + Key: gocui.KeyEsc, + Modifier: gocui.ModNone, + Handler: gui.handleStagingEscape, + KeyReadable: "esc", + Description: gui.Tr.SLocalize("EscapeStaging"), }, { ViewName: "staging", Key: gocui.KeyArrowUp, Modifier: gocui.ModNone, - Handler: gui.handleStagingKeyUp, + Handler: gui.handleStagingPrevLine, }, { ViewName: "staging", Key: gocui.KeyArrowDown, Modifier: gocui.ModNone, - Handler: gui.handleStagingKeyDown, + Handler: gui.handleStagingNextLine, }, { ViewName: "staging", - Key: gocui.KeySpace, + Key: 'k', Modifier: gocui.ModNone, - Handler: gui.handleStageLine, + Handler: gui.handleStagingPrevLine, + }, { + ViewName: "staging", + Key: 'j', + Modifier: gocui.ModNone, + Handler: gui.handleStagingNextLine, + }, { + ViewName: "staging", + Key: gocui.KeyArrowLeft, + Modifier: gocui.ModNone, + Handler: gui.handleStagingPrevHunk, + }, { + ViewName: "staging", + Key: gocui.KeyArrowRight, + Modifier: gocui.ModNone, + Handler: gui.handleStagingNextHunk, + }, { + ViewName: "staging", + Key: 'h', + Modifier: gocui.ModNone, + Handler: gui.handleStagingPrevHunk, + }, { + ViewName: "staging", + Key: 'l', + Modifier: gocui.ModNone, + Handler: gui.handleStagingNextHunk, + }, { + ViewName: "staging", + Key: gocui.KeySpace, + Modifier: gocui.ModNone, + Handler: gui.handleStageLine, + Description: gui.Tr.SLocalize("StageLine"), + }, { + ViewName: "staging", + Key: 'a', + Modifier: gocui.ModNone, + Handler: gui.handleStageHunk, + Description: gui.Tr.SLocalize("StageHunk"), }, } diff --git a/pkg/gui/staging_panel.go b/pkg/gui/staging_panel.go index be207c2fb..cba7d7638 100644 --- a/pkg/gui/staging_panel.go +++ b/pkg/gui/staging_panel.go @@ -2,19 +2,13 @@ package gui import ( "errors" - "io/ioutil" - - "github.com/davecgh/go-spew/spew" "github.com/jesseduffield/gocui" "github.com/jesseduffield/lazygit/pkg/git" + "github.com/jesseduffield/lazygit/pkg/utils" ) func (gui *Gui) refreshStagingPanel() error { - // get the currently selected file. Get the diff of that file directly, not - // using any custom diff tools. - // parse the file to find out where the chunks and unstaged changes are - file, err := gui.getSelectedFile(gui.g) if err != nil { if err != gui.Errors.ErrNoFiles { @@ -31,10 +25,6 @@ func (gui *Gui) refreshStagingPanel() error { diff := gui.GitCommand.Diff(file, true) colorDiff := gui.GitCommand.Diff(file, false) - gui.Log.WithField("staging", "staging").Info("DIFF IS:") - gui.Log.WithField("staging", "staging").Info(spew.Sdump(diff)) - gui.Log.WithField("staging", "staging").Info("hello") - if len(diff) < 2 { return gui.handleStagingEscape(gui.g, nil) } @@ -73,9 +63,9 @@ func (gui *Gui) refreshStagingPanel() error { return errors.New("No lines to stage") } - stagingView := gui.getStagingView(gui.g) - stagingView.SetCursor(0, stageableLines[currentLineIndex]) - stagingView.SetOrigin(0, 0) + if err := gui.focusLineAndHunk(); err != nil { + return err + } return gui.renderString(gui.g, "staging", colorDiff) } @@ -84,57 +74,130 @@ func (gui *Gui) handleStagingEscape(g *gocui.Gui, v *gocui.View) error { return err } + gui.State.StagingState = nil + return gui.switchFocus(gui.g, nil, gui.getFilesView(gui.g)) } -// nextNumber returns the next index, cycling if we reach the end -func nextIndex(numbers []int, currentNumber int) int { - for index, number := range numbers { - if number > currentNumber { - return index - } - } - return 0 -} - -// prevNumber returns the next number, cycling if we reach the end -func prevIndex(numbers []int, currentNumber int) int { - end := len(numbers) - 1 - for i := end; i >= 0; i -= 1 { - if numbers[i] < currentNumber { - return i - } - } - return end -} - -func (gui *Gui) handleStagingKeyUp(g *gocui.Gui, v *gocui.View) error { +func (gui *Gui) handleStagingPrevLine(g *gocui.Gui, v *gocui.View) error { return gui.handleCycleLine(true) } -func (gui *Gui) handleStagingKeyDown(g *gocui.Gui, v *gocui.View) error { +func (gui *Gui) handleStagingNextLine(g *gocui.Gui, v *gocui.View) error { return gui.handleCycleLine(false) } -func (gui *Gui) handleCycleLine(up bool) error { +func (gui *Gui) handleStagingPrevHunk(g *gocui.Gui, v *gocui.View) error { + return gui.handleCycleHunk(true) +} + +func (gui *Gui) handleStagingNextHunk(g *gocui.Gui, v *gocui.View) error { + return gui.handleCycleHunk(false) +} + +func (gui *Gui) handleCycleHunk(prev bool) error { + state := gui.State.StagingState + lineNumbers := state.StageableLines + currentLine := lineNumbers[state.CurrentLineIndex] + currentHunkIndex := utils.PrevIndex(state.HunkStarts, currentLine) + var newHunkIndex int + if prev { + if currentHunkIndex == 0 { + newHunkIndex = len(state.HunkStarts) - 1 + } else { + newHunkIndex = currentHunkIndex - 1 + } + } else { + if currentHunkIndex == len(state.HunkStarts)-1 { + newHunkIndex = 0 + } else { + newHunkIndex = currentHunkIndex + 1 + } + } + + state.CurrentLineIndex = utils.NextIndex(lineNumbers, state.HunkStarts[newHunkIndex]) + + return gui.focusLineAndHunk() +} + +func (gui *Gui) handleCycleLine(prev bool) error { state := gui.State.StagingState lineNumbers := state.StageableLines currentLine := lineNumbers[state.CurrentLineIndex] var newIndex int - if up { - newIndex = prevIndex(lineNumbers, currentLine) + if prev { + newIndex = utils.PrevIndex(lineNumbers, currentLine) } else { - newIndex = nextIndex(lineNumbers, currentLine) + newIndex = utils.NextIndex(lineNumbers, currentLine) + } + state.CurrentLineIndex = newIndex + + return gui.focusLineAndHunk() +} + +// focusLineAndHunk works out the best focus for the staging panel given the +// selected line and size of the hunk +func (gui *Gui) focusLineAndHunk() error { + stagingView := gui.getStagingView(gui.g) + state := gui.State.StagingState + + lineNumber := state.StageableLines[state.CurrentLineIndex] + + // we want the bottom line of the view buffer to ideally be the bottom line + // of the hunk, but if the hunk is too big we'll just go three lines beyond + // the currently selected line so that the user can see the context + var bottomLine int + nextHunkStartIndex := utils.NextIndex(state.HunkStarts, lineNumber) + if nextHunkStartIndex == 0 { + // for now linesHeight is an efficient means of getting the number of lines + // in the patch. However if we introduce word wrap we'll need to update this + bottomLine = stagingView.LinesHeight() - 1 + } else { + bottomLine = state.HunkStarts[nextHunkStartIndex] - 1 } - state.CurrentLineIndex = newIndex - stagingView := gui.getStagingView(gui.g) - stagingView.SetCursor(0, lineNumbers[newIndex]) - stagingView.SetOrigin(0, 0) + hunkStartIndex := utils.PrevIndex(state.HunkStarts, lineNumber) + hunkStart := state.HunkStarts[hunkStartIndex] + // if it's the first hunk we'll also show the diff header + if hunkStartIndex == 0 { + hunkStart = 0 + } + + _, height := stagingView.Size() + // if this hunk is too big, we will just ensure that the user can at least + // see three lines of context below the cursor + if bottomLine-hunkStart > height { + bottomLine = lineNumber + 3 + } + + return gui.focusLine(lineNumber, bottomLine, stagingView) +} + +// focusLine takes a lineNumber to focus, and a bottomLine to ensure we can see +func (gui *Gui) focusLine(lineNumber int, bottomLine int, v *gocui.View) error { + _, height := v.Size() + overScroll := bottomLine - height + 1 + if overScroll < 0 { + overScroll = 0 + } + if err := v.SetOrigin(0, overScroll); err != nil { + return err + } + if err := v.SetCursor(0, lineNumber-overScroll); err != nil { + return err + } return nil } +func (gui *Gui) handleStageHunk(g *gocui.Gui, v *gocui.View) error { + return gui.handleStageLineOrHunk(true) +} + func (gui *Gui) handleStageLine(g *gocui.Gui, v *gocui.View) error { + return gui.handleStageLineOrHunk(false) +} + +func (gui *Gui) handleStageLineOrHunk(hunk bool) error { state := gui.State.StagingState p, err := git.NewPatchModifier(gui.Log) if err != nil { @@ -142,22 +205,31 @@ func (gui *Gui) handleStageLine(g *gocui.Gui, v *gocui.View) error { } currentLine := state.StageableLines[state.CurrentLineIndex] - patch, err := p.ModifyPatch(state.Diff, currentLine) + var patch string + if hunk { + patch, err = p.ModifyPatchForHunk(state.Diff, state.HunkStarts, currentLine) + } else { + patch, err = p.ModifyPatchForLine(state.Diff, currentLine) + } if err != nil { return err } // for logging purposes - ioutil.WriteFile("patch.diff", []byte(patch), 0600) + // ioutil.WriteFile("patch.diff", []byte(patch), 0600) // apply the patch then refresh this panel // create a new temp file with the patch, then call git apply with that patch _, err = gui.GitCommand.ApplyPatch(patch) if err != nil { - panic(err) + return err } - gui.refreshStagingPanel() - gui.refreshFiles(gui.g) + if err := gui.refreshFiles(gui.g); err != nil { + return err + } + if err := gui.refreshStagingPanel(); err != nil { + return err + } return nil } diff --git a/pkg/gui/view_helpers.go b/pkg/gui/view_helpers.go index e6970d92f..525b05f97 100644 --- a/pkg/gui/view_helpers.go +++ b/pkg/gui/view_helpers.go @@ -259,7 +259,6 @@ func (gui *Gui) renderString(g *gocui.Gui, viewName, s string) error { output := string(bom.Clean([]byte(s))) output = utils.NormalizeLinefeeds(output) fmt.Fprint(v, output) - v.Wrap = true return nil }) return nil diff --git a/pkg/i18n/english.go b/pkg/i18n/english.go index 7a032e105..a3ce14d2e 100644 --- a/pkg/i18n/english.go +++ b/pkg/i18n/english.go @@ -420,6 +420,21 @@ func addEnglish(i18nObject *i18n.Bundle) error { }, &i18n.Message{ ID: "StagingTitle", Other: `Staging`, + }, &i18n.Message{ + ID: "StageHunk", + Other: `stage hunk`, + }, &i18n.Message{ + ID: "StageLine", + Other: `stage line`, + }, &i18n.Message{ + ID: "EscapeStaging", + Other: `return to files panel`, + }, &i18n.Message{ + ID: "CantFindHunks", + Other: `Could not find any hunks in this patch`, + }, &i18n.Message{ + ID: "CantFindHunk", + Other: `Could not find hunk`, }, ) } diff --git a/pkg/utils/utils.go b/pkg/utils/utils.go index 8e481b3a4..e2a5337e3 100644 --- a/pkg/utils/utils.go +++ b/pkg/utils/utils.go @@ -214,3 +214,24 @@ func IncludesString(list []string, a string) bool { } return false } + +// NextIndex returns the index of the element that comes after the given number +func NextIndex(numbers []int, currentNumber int) int { + for index, number := range numbers { + if number > currentNumber { + return index + } + } + return 0 +} + +// PrevIndex returns the index that comes before the given number, cycling if we reach the end +func PrevIndex(numbers []int, currentNumber int) int { + end := len(numbers) - 1 + for i := end; i >= 0; i -= 1 { + if numbers[i] < currentNumber { + return i + } + } + return end +} diff --git a/pkg/utils/utils_test.go b/pkg/utils/utils_test.go index 41774051b..f03bc087a 100644 --- a/pkg/utils/utils_test.go +++ b/pkg/utils/utils_test.go @@ -425,3 +425,95 @@ func TestIncludesString(t *testing.T) { assert.EqualValues(t, s.expected, IncludesString(s.list, s.element)) } } + +func TestNextIndex(t *testing.T) { + type scenario struct { + testName string + list []int + element int + expected int + } + + scenarios := []scenario{ + { + // I'm not really fussed about how it behaves here + "no elements", + []int{}, + 1, + 0, + }, + { + "one element", + []int{1}, + 1, + 0, + }, + { + "two elements", + []int{1, 2}, + 1, + 1, + }, + { + "two elements, giving second one", + []int{1, 2}, + 2, + 0, + }, + { + "three elements, giving second one", + []int{1, 2, 3}, + 2, + 2, + }, + } + + for _, s := range scenarios { + t.Run(s.testName, func(t *testing.T) { + assert.EqualValues(t, s.expected, NextIndex(s.list, s.element)) + }) + } +} + +func TestPrevIndex(t *testing.T) { + type scenario struct { + testName string + list []int + element int + expected int + } + + scenarios := []scenario{ + { + // I'm not really fussed about how it behaves here + "no elements", + []int{}, + 1, + -1, + }, + { + "one element", + []int{1}, + 1, + 0, + }, + { + "two elements", + []int{1, 2}, + 1, + 1, + }, + { + "three elements, giving second one", + []int{1, 2, 3}, + 2, + 0, + }, + } + + for _, s := range scenarios { + t.Run(s.testName, func(t *testing.T) { + assert.EqualValues(t, s.expected, PrevIndex(s.list, s.element)) + }) + } +}