diff options
author | Jesse Duffield <jessedduffield@gmail.com> | 2022-01-05 11:57:32 +1100 |
---|---|---|
committer | Jesse Duffield <jessedduffield@gmail.com> | 2022-01-07 10:52:51 +1100 |
commit | 05fa483f4818d45b357187700079c3bdae663df2 (patch) | |
tree | fb4fe7d3fda3e8b3e3d862c7a5e1383de456fe60 | |
parent | e524e398423f8aea2961302287123085dcc5a524 (diff) |
simplify how we log commands
44 files changed, 291 insertions, 233 deletions
diff --git a/pkg/commands/branches.go b/pkg/commands/branches.go index ef1838e9e..2276425cb 100644 --- a/pkg/commands/branches.go +++ b/pkg/commands/branches.go @@ -18,12 +18,12 @@ func (c *GitCommand) NewBranch(name string, base string) error { // the first returned string is the name and the second is the displayname // e.g. name is 123asdf and displayname is '(HEAD detached at 123asdf)' func (c *GitCommand) CurrentBranchName() (string, string, error) { - branchName, err := c.Cmd.New("git symbolic-ref --short HEAD").RunWithOutput() + branchName, err := c.Cmd.New("git symbolic-ref --short HEAD").DontLog().RunWithOutput() if err == nil && branchName != "HEAD\n" { trimmedBranchName := strings.TrimSpace(branchName) return trimmedBranchName, trimmedBranchName, nil } - output, err := c.Cmd.New("git branch --contains").RunWithOutput() + output, err := c.Cmd.New("git branch --contains").DontLog().RunWithOutput() if err != nil { return "", "", err } @@ -74,11 +74,11 @@ func (c *GitCommand) Checkout(branch string, options CheckoutOptions) error { // 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.GetBranchGraphCmdObj(branchName).RunWithOutput() + return c.GetBranchGraphCmdObj(branchName).DontLog().RunWithOutput() } func (c *GitCommand) GetUpstreamForBranch(branchName string) (string, error) { - output, err := c.Cmd.New(fmt.Sprintf("git rev-parse --abbrev-ref --symbolic-full-name %s@{u}", c.OSCommand.Quote(branchName))).RunWithOutput() + output, err := c.Cmd.New(fmt.Sprintf("git rev-parse --abbrev-ref --symbolic-full-name %s@{u}", c.OSCommand.Quote(branchName))).DontLog().RunWithOutput() return strings.TrimSpace(output), err } @@ -87,7 +87,7 @@ func (c *GitCommand) GetBranchGraphCmdObj(branchName string) oscommands.ICmdObj templateValues := map[string]string{ "branchName": c.OSCommand.Quote(branchName), } - return c.Cmd.New(utils.ResolvePlaceholderString(branchLogCmdTemplate, templateValues)) + return c.Cmd.New(utils.ResolvePlaceholderString(branchLogCmdTemplate, templateValues)).DontLog() } func (c *GitCommand) SetUpstreamBranch(upstream string) error { @@ -110,11 +110,11 @@ func (c *GitCommand) GetBranchUpstreamDifferenceCount(branchName string) (string // current branch func (c *GitCommand) GetCommitDifferences(from, to string) (string, string) { command := "git rev-list %s..%s --count" - pushableCount, err := c.Cmd.New(fmt.Sprintf(command, to, from)).RunWithOutput() + pushableCount, err := c.Cmd.New(fmt.Sprintf(command, to, from)).DontLog().RunWithOutput() if err != nil { return "?", "?" } - pullableCount, err := c.Cmd.New(fmt.Sprintf(command, from, to)).RunWithOutput() + pullableCount, err := c.Cmd.New(fmt.Sprintf(command, from, to)).DontLog().RunWithOutput() if err != nil { return "?", "?" } @@ -146,7 +146,7 @@ func (c *GitCommand) AbortMerge() error { } func (c *GitCommand) IsHeadDetached() bool { - err := c.Cmd.New("git symbolic-ref -q HEAD").Run() + err := c.Cmd.New("git symbolic-ref -q HEAD").DontLog().Run() return err != nil } @@ -169,5 +169,5 @@ func (c *GitCommand) RenameBranch(oldName string, newName string) error { } func (c *GitCommand) GetRawBranches() (string, error) { - return c.Cmd.New(`git for-each-ref --sort=-committerdate --format="%(HEAD)|%(refname:short)|%(upstream:short)|%(upstream:track)" refs/heads`).RunWithOutput() + return c.Cmd.New(`git for-each-ref --sort=-committerdate --format="%(HEAD)|%(refname:short)|%(upstream:short)|%(upstream:track)" refs/heads`).DontLog().RunWithOutput() } diff --git a/pkg/commands/commits.go b/pkg/commands/commits.go index 689220529..ec58de297 100644 --- a/pkg/commands/commits.go +++ b/pkg/commands/commits.go @@ -40,19 +40,19 @@ func (c *GitCommand) CommitCmdObj(message string, flags string) oscommands.ICmdO // Get the subject of the HEAD commit func (c *GitCommand) GetHeadCommitMessage() (string, error) { - message, err := c.Cmd.New("git log -1 --pretty=%s").RunWithOutput() + message, err := c.Cmd.New("git log -1 --pretty=%s").DontLog().RunWithOutput() return strings.TrimSpace(message), err } func (c *GitCommand) GetCommitMessage(commitSha string) (string, error) { cmdStr := "git rev-list --format=%B --max-count=1 " + commitSha - messageWithHeader, err := c.Cmd.New(cmdStr).RunWithOutput() + messageWithHeader, err := c.Cmd.New(cmdStr).DontLog().RunWithOutput() message := strings.Join(strings.SplitAfter(messageWithHeader, "\n")[1:], "\n") return strings.TrimSpace(message), err } func (c *GitCommand) GetCommitMessageFirstLine(sha string) (string, error) { - return c.Cmd.New(fmt.Sprintf("git show --no-patch --pretty=format:%%s %s", sha)).RunWithOutput() + return c.Cmd.New(fmt.Sprintf("git show --no-patch --pretty=format:%%s %s", sha)).DontLog().RunWithOutput() } // AmendHead amends HEAD with whatever is staged in your working tree @@ -72,7 +72,7 @@ func (c *GitCommand) ShowCmdObj(sha string, filterPath string) oscommands.ICmdOb } cmdStr := fmt.Sprintf("git show --submodule --color=%s --unified=%d --no-renames --stat -p %s %s", c.colorArg(), contextSize, sha, filterPathArg) - return c.Cmd.New(cmdStr) + return c.Cmd.New(cmdStr).DontLog() } // Revert reverts the selected commit by sha diff --git a/pkg/commands/files.go b/pkg/commands/files.go index ef9c50187..acb503d7f 100644 --- a/pkg/commands/files.go +++ b/pkg/commands/files.go @@ -229,7 +229,7 @@ func (c *GitCommand) WorktreeFileDiffCmdObj(node models.IFile, plain bool, cache cmdStr := fmt.Sprintf("git diff --submodule --no-ext-diff --unified=%d --color=%s %s %s %s %s", contextSize, colorArg, ignoreWhitespaceArg, cachedArg, trackedArg, quotedPath) - return c.Cmd.New(cmdStr) + return c.Cmd.New(cmdStr).DontLog() } func (c *GitCommand) ApplyPatch(patch string, flags ...string) error { @@ -265,7 +265,7 @@ func (c *GitCommand) ShowFileDiffCmdObj(from string, to string, reverse bool, fi reverseFlag = " -R " } - return c.Cmd.New(fmt.Sprintf("git diff --submodule --no-ext-diff --unified=%d --no-renames --color=%s %s %s %s -- %s", contextSize, colorArg, from, to, reverseFlag, c.OSCommand.Quote(fileName))) + return c.Cmd.New(fmt.Sprintf("git diff --submodule --no-ext-diff --unified=%d --no-renames --color=%s %s %s %s -- %s", contextSize, colorArg, from, to, reverseFlag, c.OSCommand.Quote(fileName))).DontLog() } // CheckoutFile checks out the file for the given commit @@ -280,7 +280,7 @@ func (c *GitCommand) DiscardOldFileChanges(commits []*models.Commit, commitIndex } // check if file exists in previous commit (this command returns an error if the file doesn't exist) - if err := c.Cmd.New("git cat-file -e HEAD^:" + c.OSCommand.Quote(fileName)).Run(); err != nil { + if err := c.Cmd.New("git cat-file -e HEAD^:" + c.OSCommand.Quote(fileName)).DontLog().Run(); err != nil { if err := c.OSCommand.Remove(fileName); err != nil { return err } @@ -353,7 +353,7 @@ func (c *GitCommand) EditFileCmdStr(filename string, lineNumber int) (string, er editor = c.OSCommand.Getenv("EDITOR") } if editor == "" { - if err := c.OSCommand.Cmd.New("which vi").Run(); err == nil { + if err := c.OSCommand.Cmd.New("which vi").DontLog().Run(); err == nil { editor = "vi" } } diff --git a/pkg/commands/git.go b/pkg/commands/git.go index aa06e6367..1026da242 100644 --- a/pkg/commands/git.go +++ b/pkg/commands/git.go @@ -111,29 +111,6 @@ func NewGitCommand( return gitCommand, nil } -func (c *GitCommand) WithSpan(span string) *GitCommand { - // sometimes .WithSpan(span) will be called where span actually is empty, in - // which case we don't need to log anything so we can just return early here - // with the original struct - if span == "" { - return c - } - - newGitCommand := &GitCommand{} - *newGitCommand = *c - newGitCommand.OSCommand = c.OSCommand.WithSpan(span) - - newGitCommand.Cmd = NewGitCmdObjBuilder(c.Log, newGitCommand.OSCommand.Cmd) - - // NOTE: unlike the other things here which create shallow clones, this will - // actually update the PatchManager on the original struct to have the new span. - // This means each time we call ApplyPatch in PatchManager, we need to ensure - // we've called .WithSpan() ahead of time with the new span value - newGitCommand.PatchManager.ApplyPatch = newGitCommand.ApplyPatch - - return newGitCommand -} - func navigateToRepoRootDirectory(stat func(string) (os.FileInfo, error), chdir func(string) error) error { gitDir := env.GetGitDirEnv() if gitDir != "" { @@ -245,7 +222,7 @@ func findDotGitDir(stat func(string) (os.FileInfo, error), readFile func(filenam } func VerifyInGitRepo(osCommand *oscommands.OSCommand) error { - return osCommand.Cmd.New("git rev-parse --git-dir").Run() + return osCommand.Cmd.New("git rev-parse --git-dir").DontLog().Run() } func (c *GitCommand) GetDotGitDir() string { diff --git a/pkg/commands/loaders/commit_files.go b/pkg/commands/loaders/commit_files.go index 1b65737ec..755db768d 100644 --- a/pkg/commands/loaders/commit_files.go +++ b/pkg/commands/loaders/commit_files.go @@ -28,7 +28,7 @@ func (self *CommitFileLoader) GetFilesInDiff(from string, to string, reverse boo reverseFlag = " -R " } - filenames, err := self.cmd.New(fmt.Sprintf("git diff --submodule --no-ext-diff --name-status -z --no-renames %s %s %s", reverseFlag, from, to)).RunWithOutput() + filenames, err := self.cmd.New(fmt.Sprintf("git diff --submodule --no-ext-diff --name-status -z --no-renames %s %s %s", reverseFlag, from, to)).DontLog().RunWithOutput() if err != nil { return nil, err } diff --git a/pkg/commands/loaders/commits.go b/pkg/commands/loaders/commits.go index 6f2336e3a..28bcbb3a7 100644 --- a/pkg/commands/loaders/commits.go +++ b/pkg/commands/loaders/commits.go @@ -218,7 +218,7 @@ func (self *CommitLoader) getHydratedRebasingCommits(rebaseMode enums.RebaseMode prettyFormat, 20, ), - ) + ).DontLog() hydratedCommits := make([]*models.Commit, 0, len(commits)) i := 0 @@ -384,7 +384,7 @@ func (self *CommitLoader) getMergeBase(refName string) (string, error) { } // swallowing error because it's not a big deal; probably because there are no commits yet - output, _ := self.cmd.New(fmt.Sprintf("git merge-base %s %s", self.cmd.Quote(refName), self.cmd.Quote(baseBranch))).RunWithOutput() + output, _ := self.cmd.New(fmt.Sprintf("git merge-base %s %s", self.cmd.Quote(refName), self.cmd.Quote(baseBranch))).DontLog().RunWithOutput() return ignoringWarnings(output), nil } @@ -405,6 +405,7 @@ func (self *CommitLoader) getFirstPushedCommit(refName string) (string, error) { New( fmt.Sprintf("git merge-base %s %s@{u}", self.cmd.Quote(refName), self.cmd.Quote(refName)), ). + DontLog(). RunWithOutput() if err != nil { return "", err @@ -444,7 +445,7 @@ func (self *CommitLoader) getLogCmd(opts GetCommitsOptions) oscommands.ICmdObj { 20, filterFlag, ), - ) + ).DontLog() } var prettyFormat = fmt.Sprintf( diff --git a/pkg/commands/loaders/files.go b/pkg/commands/loaders/files.go index a1964a2b9..8f6eb32f4 100644 --- a/pkg/commands/loaders/files.go +++ b/pkg/commands/loaders/files.go @@ -98,7 +98,7 @@ func (c *FileLoader) GitStatus(opts GitStatusOptions) ([]FileStatus, error) { noRenamesFlag = " --no-renames" } - statusLines, err := c.cmd.New(fmt.Sprintf("git status %s --porcelain -z%s", opts.UntrackedFilesArg, noRenamesFlag)).RunWithOutput() + statusLines, err := c.cmd.New(fmt.Sprintf("git status %s --porcelain -z%s", opts.UntrackedFilesArg, noRenamesFlag)).DontLog().RunWithOutput() if err != nil { return []FileStatus{}, err } diff --git a/pkg/commands/loaders/reflog_commits.go b/pkg/commands/loaders/reflog_commits.go index 012c7c297..f2cb01aaa 100644 --- a/pkg/commands/loaders/reflog_commits.go +++ b/pkg/commands/loaders/reflog_commits.go @@ -32,7 +32,7 @@ func (self *ReflogCommitLoader) GetReflogCommits(lastReflogCommit *models.Commit filterPathArg = fmt.Sprintf(" --follow -- %s", self.cmd.Quote(filterPath)) } - cmdObj := self.cmd.New(fmt.Sprintf(`git log -g --abbrev=20 --format="%%h %%ct %%gs" %s`, filterPathArg)) + cmdObj := self.cmd.New(fmt.Sprintf(`git log -g --abbrev=20 --format="%%h %%ct %%gs" %s`, filterPathArg)).DontLog() onlyObtainedNewReflogCommits := false err := cmdObj.RunAndProcessLines(func(line string) (bool, error) { fields := strings.SplitN(line, " ", 3) diff --git a/pkg/commands/loaders/remotes.go b/pkg/commands/loaders/remotes.go index c0734ab32..bd1fe0b6a 100644 --- a/pkg/commands/loaders/remotes.go +++ b/pkg/commands/loaders/remotes.go @@ -31,7 +31,7 @@ func NewRemoteLoader( } func (self *RemoteLoader) GetRemotes() ([]*models.Remote, error) { - remoteBranchesStr, err := self.cmd.New("git branch -r").RunWithOutput() + remoteBranchesStr, err := self.cmd.New("git branch -r").DontLog().RunWithOutput() if err != nil { return nil, err } diff --git a/pkg/commands/loaders/stash.go b/pkg/commands/loaders/stash.go index f9509cdb8..689bf30ce 100644 --- a/pkg/commands/loaders/stash.go +++ b/pkg/commands/loaders/stash.go @@ -31,7 +31,7 @@ func (self *StashLoader) GetStashEntries(filterPath string) []*models.StashEntry return self.getUnfilteredStashEntries() } - rawString, err := self.cmd.New("git stash list --name-only").RunWithOutput() + rawString, err := self.cmd.New("git stash list --name-only").DontLog().RunWithOutput() if err != nil { return self.getUnfilteredStashEntries() } @@ -64,7 +64,7 @@ outer: } func (self *StashLoader) getUnfilteredStashEntries() []*models.StashEntry { - rawString, _ := self.cmd.New("git stash list --pretty='%gs'").RunWithOutput() + rawString, _ := self.cmd.New("git stash list --pretty='%gs'").DontLog().RunWithOutput() stashEntries := []*models.StashEntry{} for i, line := range utils.SplitLines(rawString) { stashEntries = append(stashEntries, self.stashEntryFromLine(line, i)) diff --git a/pkg/commands/loaders/tags.go b/pkg/commands/loaders/tags.go index 116eab900..45b08a002 100644 --- a/pkg/commands/loaders/tags.go +++ b/pkg/commands/loaders/tags.go @@ -27,7 +27,7 @@ func NewTagLoader( func (self *TagLoader) GetTags() ([]*models.Tag, error) { // get remote branches, sorted by creation date (descending) // see: https://git-scm.com/docs/git-tag#Documentation/git-tag.txt---sortltkeygt - remoteBranchesStr, err := self.cmd.New(`git tag --list --sort=-creatordate`).RunWithOutput() + remoteBranchesStr, err := self.cmd.New(`git tag --list --sort=-creatordate`).DontLog().RunWithOutput() if err != nil { return nil, err } diff --git a/pkg/commands/oscommands/cmd_obj.go b/pkg/commands/oscommands/cmd_obj.go index a9a869bd6..192871af0 100644 --- a/pkg/commands/oscommands/cmd_obj.go +++ b/pkg/commands/oscommands/cmd_obj.go @@ -22,16 +22,20 @@ type ICmdObj interface { // runs the command and runs a callback function on each line of the output. If the callback returns true for the boolean value, we kill the process and return. RunAndProcessLines(onLine func(line string) (bool, error)) error - // logs command - Log() ICmdObj + // Marks the command object as readonly, so that when it is run, we don't log it to the user. + // We only want to log commands to the user which change state in some way. + DontLog() ICmdObj + ShouldLog() bool } type CmdObj struct { cmdStr string cmd *exec.Cmd - runner ICmdObjRunner - logCommand func(ICmdObj) + runner ICmdObjRunner + + // if set to true, we don't want to log the command to the user. + dontLog bool } func (self *CmdObj) GetCmd() *exec.Cmd { @@ -52,12 +56,15 @@ func (self *CmdObj) GetEnvVars() []string { return self.cmd.Env } -func (self *CmdObj) Log() ICmdObj { - self.logCommand(self) - +func (self *CmdObj) DontLog() ICmdObj { + self.dontLog = true return self } +func (self *CmdObj) ShouldLog() bool { + return !self.dontLog +} + func (self *CmdObj) Run() error { return self.runner.Run(self) } diff --git a/pkg/commands/oscommands/cmd_obj_builder.go b/pkg/commands/oscommands/cmd_obj_builder.go index 633cee322..5929d6faf 100644 --- a/pkg/commands/oscommands/cmd_obj_builder.go +++ b/pkg/commands/oscommands/cmd_obj_builder.go @@ -35,10 +35,9 @@ func (self *CmdObjBuilder) New(cmdStr string) ICmdObj { cmd.Env = os.Environ() return &CmdObj{ - cmdStr: cmdStr, - cmd: cmd, - runner: self.runner, - logCommand: self.logCmdObj, + cmdStr: cmdStr, + cmd: cmd, + runner: self.runner, } } @@ -47,10 +46,9 @@ func (self *CmdObjBuilder) NewFromArgs(args []string) ICmdObj { cmd.Env = os.Environ() return &CmdObj{ - cmdStr: strings.Join(args, " "), - cmd: cmd, - runner: self.runner, - logCommand: self.logCmdObj, + cmdStr: strings.Join(args, " "), + cmd: cmd, + runner: self.runner, } } diff --git a/pkg/commands/oscommands/cmd_obj_runner.go b/pkg/commands/oscommands/cmd_obj_runner.go index 5c7d47323..0c6bf086e 100644 --- a/pkg/commands/oscommands/cmd_obj_runner.go +++ b/pkg/commands/oscommands/cmd_obj_runner.go @@ -22,12 +22,19 @@ type cmdObjRunner struct { var _ ICmdObjRunner = &cmdObjRunner{} func (self *cmdObjRunner) Run(cmdObj ICmdObj) error { + if cmdObj.ShouldLog() { + self.logCmdObj(cmdObj) + } + _, err := self.RunWithOutput(cmdObj) return err } func (self *cmdObjRunner) RunWithOutput(cmdObj ICmdObj) (string, error) { - self.logCmdObj(cmdObj) + if cmdObj.ShouldLog() { + self.logCmdObj(cmdObj) + } + output, err := sanitisedCommandOutput(cmdObj.GetCmd().CombinedOutput()) if err != nil { self.log.WithField("command", cmdObj.ToString()).Error(output) @@ -36,6 +43,10 @@ func (self *cmdObjRunner) RunWithOutput(cmdObj ICmdObj) (string, error) { } func (self *cmdObjRunner) RunAndProcessLines(cmdObj ICmdObj, onLine func(line string) (bool, error)) error { + if cmdObj.ShouldLog() { + self.logCmdObj(cmdObj) + } + cmd := cmdObj.GetCmd() stdoutPipe, err := cmd.StdoutPipe() if err != nil { diff --git a/pkg/commands/oscommands/exec_live.go b/pkg/commands/oscommands/exec_live.go index 6ca70a5ec..36a7ad0ac 100644 --- a/pkg/commands/oscommands/exec_live.go +++ b/pkg/commands/oscommands/exec_live.go @@ -66,7 +66,9 @@ func RunCommandWithOutputLiveAux( startCmd func(cmd *exec.Cmd) (*cmdHandler, error), ) error { c.Log.WithField("command", cmdObj.ToString()).Info("RunCommand") - c.LogCommand(cmdObj.ToString(), true) + if cmdObj.ShouldLog() { + c.LogCommand(cmdObj.ToString(), true) + } cmd := cmdObj.AddEnvVars("LANG=en_US.UTF-8", "LC_ALL=en_US.UTF-8").GetCmd() var stderr bytes.Buffer diff --git a/pkg/commands/oscommands/os.go b/pkg/commands/oscommands/os.go index c5c6e3649..0d2a936ef 100644 --- a/pkg/commands/oscommands/os.go +++ b/pkg/commands/oscommands/os.go @@ -87,22 +87,6 @@ func NewOSCommand(common *common.Common, platform *Platform) *OSCommand { return c } -func (c *OSCommand) WithSpan(span string) *OSCommand { - // sometimes .WithSpan(span) will be called where span actually is empty, in - // which case we don't need to log anything so we can just return early here - // with the original struct - if span == "" { - return c - } - - newOSCommand := &OSCommand{} - *newOSCommand = *c - newOSCommand.CmdLogSpan = span - newOSCommand.Cmd.logCmdObj = newOSCommand.LogCmdObj - newOSCommand.Cmd.runner = &cmdObjRunner{log: c.Log, logCmdObj: newOSCommand.LogCmdObj} - return newOSCommand -} - func (c *OSCommand) LogCmdObj(cmdObj ICmdObj) { c.LogCommand(cmdObj.ToString(), true) } @@ -110,7 +94,7 @@ func (c *OSCommand) LogCmdObj(cmdObj ICmdObj) { func (c *OSCommand) LogCommand(cmdStr string, commandLine bool) { c.Log.WithField("command", cmdStr).Info("RunCommand") - if c.onRunCommand != nil && c.CmdLogSpan != "" { + if c.onRunCommand != nil { c.onRunCommand(NewCmdLogEntry(cmdStr, c.CmdLogSpan, commandLine)) } } diff --git a/pkg/commands/remotes.go b/pkg/commands/remotes.go index 3ae9693c7..c79512dd9 100644 --- a/pkg/commands/remotes.go +++ b/pkg/commands/remotes.go @@ -47,7 +47,9 @@ func (c *GitCommand) CheckRemoteBranchExists(branchName string) bool { New( fmt.Sprintf("git show-ref --verify -- refs/remotes/origin/%s", c.Cmd.Quote(branchName), - )). + ), + ). + DontLog(). RunWithOutput() return err == nil diff --git a/pkg/commands/stash_entries.go b/pkg/commands/stash_entries.go index 498431103..b4d16adb7 100644 --- a/pkg/commands/stash_entries.go +++ b/pkg/commands/stash_entries.go @@ -4,6 +4,7 @@ import ( "fmt" "github.com/jesseduffield/lazygit/pkg/commands/loaders" + "github.com/jesseduffield/lazygit/pkg/commands |