Skip to content

Commit

Permalink
allow fast flicking through any list panel
Browse files Browse the repository at this point in the history 
Up till now our approach to rendering things like file diffs, branch logs, and
commit patches, has been to run a command on the command line, wait for it to
complete, take its output as a string, and then write that string to the main
view (or secondary view e.g. when showing both staged and unstaged changes of a
file).

This has caused various issues. For once, if you are flicking through a list of
files and an untracked file is particularly large, not only will this require
lazygit to load that whole file into memory (or more accurately it's equally
large diff), it also will slow down the UI thread while loading that file, and
if the user continued down the list, the original command might eventually
resolve and replace whatever the diff is for the newly selected file.

Following what we've done in lazydocker, I've added a tasks package for when you
need something done but you want it to cancel as soon as something newer comes
up. Given this typically involves running a command to display to a view, I've
added a viewBufferManagerMap struct to the Gui struct which allows you to define
these tasks on a per-view basis.

viewBufferManagers can run files and directly write the output to their view,
meaning we no longer need to use so much memory.

In the tasks package there is a helper method called NewCmdTask which takes a
command, an initial amount of lines to read, and then runs that command, reads
that number of lines, and allows for a readLines channel to tell it to read more
lines. We read more lines when we scroll or resize the window.

There is an adapter for the tasks package in a file called tasks_adapter which
wraps the functions from the tasks package in gui-specific stuff like clearing
the main view before starting the next task that wants to write to the main
view.

I've removed some small features as part of this work, namely the little headers
that were at the top of the main view for some situations. For example, we no
longer show the upstream of a selected branch. I want to re-introduce this in
the future, but I didn't want to make this tasks system too complicated, and in
order to facilitate a header section in the main view we'd need to have a task
that gets the upstream for the current branch, writes it to the header, then
tells another task to write the branch log to the main view, but without
clearing inbetween. So it would get messy. I'm thinking instead of having a
separate 'header' view atop the main view to render that kind of thing (which
can happen in another PR)

I've also simplified the 'git show' to just call 'git show' and not do anything
fancy when it comes to merge commits.

I considered using this tasks approach whenever we write to a view. The only
thing is that the renderString method currently resets the origin of a view and
I don't want to lose that. So I've left some in there that I consider harmless,
but we should probably be just using tasks now for all rendering, even if it's
just strings we can instantly make.
  • Loading branch information
@jesseduffield
jesseduffield committed Jan 12, 2020
1 parent 282f08d commit 23bcc19
Show file tree
Hide file tree
Showing 16 changed files with 490 additions and 244 deletions.
68 changes: 23 additions & 45 deletions pkg/commands/git.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -176,8 +176,8 @@ func stashEntryFromLine(line string, index int) *StashEntry {
}

// GetStashEntryDiff stash diff
func (c *GitCommand) GetStashEntryDiff(index int) (string, error) {
return c.OSCommand.RunCommandWithOutput("git stash show -p --color stash@{%d}", index)
func (c *GitCommand) ShowStashEntryCmdStr(index int) string {
return fmt.Sprintf("git stash show -p --color stash@{%d}", index)
}

// GetStatusFiles git status files
Expand Down Expand Up @@ -538,7 +538,8 @@ func (c *GitCommand) PrepareCommitAmendSubProcess() *exec.Cmd {
// Currently it limits the result to 100 commits, but when we get async stuff
// working we can do lazy loading
func (c *GitCommand) GetBranchGraph(branchName string) (string, error) {
return c.OSCommand.RunCommandWithOutput("git log --graph --color --abbrev-commit --decorate --date=relative --pretty=medium -100 %s", branchName)
cmdStr := c.GetBranchGraphCmdStr(branchName)
return c.OSCommand.RunCommandWithOutput(cmdStr)
}

func (c *GitCommand) GetUpstreamForBranch(branchName string) (string, error) {
Expand All @@ -551,41 +552,12 @@ func (c *GitCommand) Ignore(filename string) error {
return c.OSCommand.AppendLineToFile(".gitignore", filename)
}

// Show shows the diff of a commit
func (c *GitCommand) Show(sha string) (string, error) {
show, err := c.OSCommand.RunCommandWithOutput("git show --color --no-renames %s", sha)
if err != nil {
return "", err
}

// if this is a merge commit, we need to go a step further and get the diff between the two branches we merged
revList, err := c.OSCommand.RunCommandWithOutput("git rev-list -1 --merges %s^...%s", sha, sha)
if err != nil {
// turns out we get an error here when it's the first commit. We'll just return the original show
return show, nil
}
if len(revList) == 0 {
return show, nil
}

// we want to pull out 1a6a69a and 3b51d7c from this:
// commit ccc771d8b13d5b0d4635db4463556366470fd4f6
// Merge: 1a6a69a 3b51d7c
lines := utils.SplitLines(show)
if len(lines) < 2 {
return show, nil
}

secondLineWords := strings.Split(lines[1], " ")
if len(secondLineWords) < 3 {
return show, nil
}
func (c *GitCommand) ShowCmdStr(sha string) string {
return fmt.Sprintf("git show --color --no-renames %s", sha)
}

mergeDiff, err := c.OSCommand.RunCommandWithOutput("git diff --color %s...%s", secondLineWords[1], secondLineWords[2])
if err != nil {
return "", err
}
return show + mergeDiff, nil
func (c *GitCommand) GetBranchGraphCmdStr(branchName string) string {
return fmt.Sprintf("git log --graph --color --abbrev-commit --decorate --date=relative --pretty=medium %s", branchName)
}

// GetRemoteURL returns current repo remote url
Expand All @@ -606,6 +578,12 @@ func (c *GitCommand) CheckRemoteBranchExists(branch *Branch) bool {

// Diff returns the diff of a file
func (c *GitCommand) Diff(file *File, plain bool, cached bool) string {
// for now we assume an error means the file was deleted
s, _ := c.OSCommand.RunCommandWithOutput(c.DiffCmdStr(file, plain, cached))
return s
}

func (c *GitCommand) DiffCmdStr(file *File, plain bool, cached bool) string {
cachedArg := ""
trackedArg := "--"
colorArg := "--color"
Expand All @@ -621,9 +599,7 @@ func (c *GitCommand) Diff(file *File, plain bool, cached bool) string {
colorArg = ""
}

// for now we assume an error means the file was deleted
s, _ := c.OSCommand.RunCommandWithOutput("git diff %s %s %s %s", colorArg, cachedArg, trackedArg, fileName)
return s
return fmt.Sprintf("git diff %s %s %s %s", colorArg, cachedArg, trackedArg, fileName)
}

func (c *GitCommand) ApplyPatch(patch string, flags ...string) error {
Expand Down Expand Up @@ -908,11 +884,17 @@ func (c *GitCommand) GetCommitFiles(commitSha string, patchManager *PatchManager

// ShowCommitFile get the diff of specified commit file
func (c *GitCommand) ShowCommitFile(commitSha, fileName string, plain bool) (string, error) {
cmdStr := c.ShowCommitFileCmdStr(commitSha, fileName, plain)
return c.OSCommand.RunCommandWithOutput(cmdStr)
}

func (c *GitCommand) ShowCommitFileCmdStr(commitSha, fileName string, plain bool) string {
colorArg := "--color"
if plain {
colorArg = ""
}
return c.OSCommand.RunCommandWithOutput("git show --no-renames %s %s -- %s", colorArg, commitSha, fileName)

return fmt.Sprintf("git show --no-renames %s %s -- %s", colorArg, commitSha, fileName)
}

// CheckoutFile checks out the file for the given commit
Expand Down Expand Up @@ -1098,10 +1080,6 @@ func (c *GitCommand) CreateLightweightTag(tagName string, commitSha string) erro
return c.OSCommand.RunCommand("git tag %s %s", tagName, commitSha)
}

func (c *GitCommand) ShowTag(tagName string) (string, error) {
return c.OSCommand.RunCommandWithOutput("git tag -n99 %s", tagName)
}

func (c *GitCommand) DeleteTag(tagName string) error {
return c.OSCommand.RunCommand("git tag -d %s", tagName)
}
Expand Down
77 changes: 1 addition & 76 deletions pkg/commands/git_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -318,21 +318,6 @@ func TestGitCommandGetStashEntries(t *testing.T) {
}
}

// TestGitCommandGetStashEntryDiff is a function.
func TestGitCommandGetStashEntryDiff(t *testing.T) {
gitCmd := NewDummyGitCommand()
gitCmd.OSCommand.command = func(cmd string, args ...string) *exec.Cmd {
assert.EqualValues(t, "git", cmd)
assert.EqualValues(t, []string{"stash", "show", "-p", "--color", "stash@{1}"}, args)

return exec.Command("echo")
}

_, err := gitCmd.GetStashEntryDiff(1)

assert.NoError(t, err)
}

// TestGitCommandGetStatusFiles is a function.
func TestGitCommandGetStatusFiles(t *testing.T) {
type scenario struct {
Expand Down Expand Up @@ -1411,66 +1396,6 @@ func TestGitCommandDiscardAllFileChanges(t *testing.T) {
}
}

// TestGitCommandShow is a function.
func TestGitCommandShow(t *testing.T) {
type scenario struct {
testName string
arg string
command func(string, ...string) *exec.Cmd
test func(string, error)
}

scenarios := []scenario{
{
"regular commit",
"456abcde",
test.CreateMockCommand(t, []*test.CommandSwapper{
{
Expect: "git show --color --no-renames 456abcde",
Replace: "echo \"commit ccc771d8b13d5b0d4635db4463556366470fd4f6\nblah\"",
},
{
Expect: "git rev-list -1 --merges 456abcde^...456abcde",
Replace: "echo",
},
}),
func(result string, err error) {
assert.NoError(t, err)
assert.Equal(t, "commit ccc771d8b13d5b0d4635db4463556366470fd4f6\nblah\n", result)
},
},
{
"merge commit",
"456abcde",
test.CreateMockCommand(t, []*test.CommandSwapper{
{
Expect: "git show --color --no-renames 456abcde",
Replace: "echo \"commit ccc771d8b13d5b0d4635db4463556366470fd4f6\nMerge: 1a6a69a 3b51d7c\"",
},
{
Expect: "git rev-list -1 --merges 456abcde^...456abcde",
Replace: "echo aa30e006433628ba9281652952b34d8aacda9c01",
},
{
Expect: "git diff --color 1a6a69a...3b51d7c",
Replace: "echo blah",
},
}),
func(result string, err error) {
assert.NoError(t, err)
assert.Equal(t, "commit ccc771d8b13d5b0d4635db4463556366470fd4f6\nMerge: 1a6a69a 3b51d7c\nblah\n", result)
},
},
}

gitCmd := NewDummyGitCommand()

for _, s := range scenarios {
gitCmd.OSCommand.command = s.command
s.test(gitCmd.Show(s.arg))
}
}

// TestGitCommandCheckout is a function.
func TestGitCommandCheckout(t *testing.T) {
type scenario struct {
Expand Down Expand Up @@ -1523,7 +1448,7 @@ func TestGitCommandGetBranchGraph(t *testing.T) {
gitCmd := NewDummyGitCommand()
gitCmd.OSCommand.command = func(cmd string, args ...string) *exec.Cmd {
assert.EqualValues(t, "git", cmd)
assert.EqualValues(t, []string{"log", "--graph", "--color", "--abbrev-commit", "--decorate", "--date=relative", "--pretty=medium", "-100", "test"}, args)
assert.EqualValues(t, []string{"log", "--graph", "--color", "--abbrev-commit", "--decorate", "--date=relative", "--pretty=medium", "test"}, args)

return exec.Command("echo")
}
Expand Down
54 changes: 29 additions & 25 deletions pkg/gui/branches_panel.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import (
"fmt"
"strings"

"github.com/fatih/color"
"github.com/jesseduffield/gocui"
"github.com/jesseduffield/lazygit/pkg/commands"
"github.com/jesseduffield/lazygit/pkg/utils"
Expand Down Expand Up @@ -37,40 +36,45 @@ func (gui *Gui) handleBranchSelect(g *gocui.Gui, v *gocui.View) error {

// This really shouldn't happen: there should always be a master branch
if len(gui.State.Branches) == 0 {
return gui.renderString(g, "main", gui.Tr.SLocalize("NoBranchesThisRepo"))
return gui.newStringTask("main", gui.Tr.SLocalize("NoBranchesThisRepo"))
}
branch := gui.getSelectedBranch()
if err := gui.focusPoint(0, gui.State.Panels.Branches.SelectedLine, len(gui.State.Branches), v); err != nil {
return err
}
go func() {
_ = gui.RenderSelectedBranchUpstreamDifferences()
}()
go func() {
upstream, _ := gui.GitCommand.GetUpstreamForBranch(branch.Name)
if strings.Contains(upstream, "no upstream configured for branch") || strings.Contains(upstream, "unknown revision or path not in the working tree") {
upstream = gui.Tr.SLocalize("notTrackingRemote")
}
graph, err := gui.GitCommand.GetBranchGraph(branch.Name)
if err != nil && strings.HasPrefix(graph, "fatal: ambiguous argument") {
graph = gui.Tr.SLocalize("NoTrackingThisBranch")
}
_ = gui.renderString(g, "main", fmt.Sprintf("%s → %s\n\n%s", utils.ColoredString(branch.Name, color.FgGreen), utils.ColoredString(upstream, color.FgRed), graph))
}()
if err := gui.RenderSelectedBranchUpstreamDifferences(); err != nil {
return err
}

cmd := gui.OSCommand.ExecutableFromString(
gui.GitCommand.GetBranchGraphCmdStr(branch.Name),
)
if err := gui.newCmdTask("main", cmd); err != nil {
gui.Log.Error(err)
}
return nil
}

func (gui *Gui) RenderSelectedBranchUpstreamDifferences() error {
// here we tell the selected branch that it is selected.
// this is necessary for showing stats on a branch that is selected, because
// the displaystring function doesn't have access to gui state to tell if it's selected
for i, branch := range gui.State.Branches {
branch.Selected = i == gui.State.Panels.Branches.SelectedLine
}
return gui.newTask("branches", func(stop chan struct{}) error {
// here we tell the selected branch that it is selected.
// this is necessary for showing stats on a branch that is selected, because
// the displaystring function doesn't have access to gui state to tell if it's selected
for i, branch := range gui.State.Branches {
branch.Selected = i == gui.State.Panels.Branches.SelectedLine
}

branch := gui.getSelectedBranch()
branch.Pushables, branch.Pullables = gui.GitCommand.GetBranchUpstreamDifferenceCount(branch.Name)
return gui.renderListPanel(gui.getBranchesView(), gui.State.Branches)
branch := gui.getSelectedBranch()
branch.Pushables, branch.Pullables = gui.GitCommand.GetBranchUpstreamDifferenceCount(branch.Name)

select {
case <-stop:
return nil
default:
}

return gui.renderListPanel(gui.getBranchesView(), gui.State.Branches)
})
}

// gui.refreshStatus is called at the end of this because that's when we can
Expand Down
12 changes: 8 additions & 4 deletions pkg/gui/commit_files_panel.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,11 +43,15 @@ func (gui *Gui) handleCommitFileSelect(g *gocui.Gui, v *gocui.View) error {
if err := gui.focusPoint(0, gui.State.Panels.CommitFiles.SelectedLine, len(gui.State.CommitFiles), v); err != nil {
return err
}
commitText, err := gui.GitCommand.ShowCommitFile(commitFile.Sha, commitFile.Name, false)
if err != nil {
return err

cmd := gui.OSCommand.ExecutableFromString(
gui.GitCommand.ShowCommitFileCmdStr(commitFile.Sha, commitFile.Name, false),
)
if err := gui.newCmdTask("main", cmd); err != nil {
gui.Log.Error(err)
}
return gui.renderString(g, "main", commitText)

return nil
}

func (gui *Gui) handleSwitchToCommitsPanel(g *gocui.Gui, v *gocui.View) error {
Expand Down
17 changes: 10 additions & 7 deletions pkg/gui/commits_panel.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ func (gui *Gui) handleCommitSelect(g *gocui.Gui, v *gocui.View) error {

commit := gui.getSelectedCommit(g)
if commit == nil {
return gui.renderString(g, "main", gui.Tr.SLocalize("NoCommitsThisBranch"))
return gui.newStringTask("main", gui.Tr.SLocalize("NoCommitsThisBranch"))
}

if err := gui.focusPoint(0, gui.State.Panels.Commits.SelectedLine, len(gui.State.Commits), v); err != nil {
Expand All @@ -65,11 +65,14 @@ func (gui *Gui) handleCommitSelect(g *gocui.Gui, v *gocui.View) error {
return nil
}

commitText, err := gui.GitCommand.Show(commit.Sha)
if err != nil {
return err
cmd := gui.OSCommand.ExecutableFromString(
gui.GitCommand.ShowCmdStr(commit.Sha),
)
if err := gui.newCmdTask("main", cmd); err != nil {
gui.Log.Error(err)
}
return gui.renderString(g, "main", commitText)

return nil
}

func (gui *Gui) refreshCommits(g *gocui.Gui) error {
Expand Down Expand Up @@ -463,7 +466,7 @@ func (gui *Gui) handleToggleDiffCommit(g *gocui.Gui, v *gocui.View) error {
// get selected commit
commit := gui.getSelectedCommit(g)
if commit == nil {
return gui.renderString(g, "main", gui.Tr.SLocalize("NoCommitsThisBranch"))
return gui.newStringTask("main", gui.Tr.SLocalize("NoCommitsThisBranch"))
}

// if already selected commit delete
Expand All @@ -486,7 +489,7 @@ func (gui *Gui) handleToggleDiffCommit(g *gocui.Gui, v *gocui.View) error {
return gui.createErrorPanel(gui.g, err.Error())
}

return gui.renderString(g, "main", commitText)
return gui.newStringTask("main", commitText)
}

return nil
Expand Down
Loading

0 comments on commit 23bcc19

Please sign in to comment.