Clean up the configuration of where a custom command's output goes (#4525)

- **PR Description**

Previously, custom commands had a `stream` field that was overloaded
with two meanings: 1) it made the command's output appear in the log
view, and 2) it used a pty for running the command. It makes sense to be
able to configure these independently, so add a separate `pty` field
(although that's probably rarely needed in practice).

Also, the `stream` and `showOutput` fields were conflicting; they could
be used together, but when setting them both to true, the popup would
always show "empty output", so this doesn't make sense. Combine them
both into a single `output` property with the possible values "none",
"log", or "popup".

We still have some more redundancy here, for example pty is only used
when output is set to "log", and neither output nor pty are used when
subprocess is true. But I stopped there, because I think this is already
an improvement over the previous state.

- **Please check if the PR fulfills these requirements**

* [x] Cheatsheets are up-to-date (run `go generate ./...`)
* [x] Code has been formatted (see
[here](https://github.com/jesseduffield/lazygit/blob/master/CONTRIBUTING.md#code-formatting))
* [x] Tests have been added/updated (see
[here](https://github.com/jesseduffield/lazygit/blob/master/pkg/integration/README.md)
for the integration test guide)
* [ ] Text is internationalised (see
[here](https://github.com/jesseduffield/lazygit/blob/master/CONTRIBUTING.md#internationalisation))
* [x] If a new UserConfig entry was added, make sure it can be
hot-reloaded (see
[here](https://github.com/jesseduffield/lazygit/blob/master/docs/dev/Codebase_Guide.md#using-userconfig))
* [x] Docs have been updated if necessary
* [x] You've read through your own file changes for silly mistakes etc

Author: stefanhaller

docs/Custom_Command_Keybindings.md

@@ -14,7 +14,7 @@ customCommands:
   - key: 'C'
     context: 'global'
     command: "git commit"
-    subprocess: true
+    output: terminal
   - key: 'n'
     context: 'localBranches'
     prompts:
@@ -53,13 +53,11 @@ For a given custom command, here are the allowed fields:
 | key | The key to trigger the command. Use a single letter or one of the values from [here](https://github.com/jesseduffield/lazygit/blob/master/docs/keybindings/Custom_Keybindings.md). Custom commands without a key specified can be triggered by selecting them from the keybindings (`?`) menu | no |
 | command | The command to run (using Go template syntax for placeholder values) | yes |
 | context | The context in which to listen for the key (see [below](#contexts)) | yes |
-| subprocess | Whether you want the command to run in a subprocess (e.g. if the command requires user input) | no |
 | prompts | A list of prompts that will request user input before running the final command | no |
 | loadingText | Text to display while waiting for command to finish | no |
 | description | Label for the custom command when displayed in the keybindings menu | no |
-| stream | Whether you want to stream the command's output to the Command Log panel | no |
-| showOutput | Whether you want to show the command's output in a popup within Lazygit | no |
-| outputTitle | The title to display in the popup panel if showOutput is true. If left unset, the command will be used as the title. | no |
+| output | Where the output of the command should go. 'none' discards it, 'terminal' suspends lazygit and runs the command in the terminal (useful for commands that require user input), 'log' streams it to the command log, 'logWithPty' is like 'log' but runs the command in a pseudo terminal (can be useful for commands that produce colored output when the output is a terminal), and 'popup' shows it in a popup. | no |
+| outputTitle | The title to display in the popup panel if output is set to 'popup'. If left unset, the command will be used as the title. | no |
 | after | Actions to take after the command has completed | no |
 
 Here are the options for the `after` key:
@@ -365,7 +363,7 @@ If you use the commandMenu property, none of the other properties except key and
 
 ## Debugging
 
-If you want to verify that your command actually does what you expect, you can wrap it in an 'echo' call and set `showOutput: true` so that it doesn't actually execute the command but you can see how the placeholders were resolved.
+If you want to verify that your command actually does what you expect, you can wrap it in an 'echo' call and set `output: popup` so that it doesn't actually execute the command but you can see how the placeholders were resolved.
 
 ## More Examples
 

pkg/commands/git_cmd_obj_builder.go

@@ -30,11 +30,11 @@ func NewGitCmdObjBuilder(log *logrus.Entry, innerBuilder *oscommands.CmdObjBuild
 
 var defaultEnvVar = "GIT_OPTIONAL_LOCKS=0"
 
-func (self *gitCmdObjBuilder) New(args []string) oscommands.ICmdObj {
+func (self *gitCmdObjBuilder) New(args []string) *oscommands.CmdObj {
 	return self.innerBuilder.New(args).AddEnvVars(defaultEnvVar)
 }
 
-func (self *gitCmdObjBuilder) NewShell(cmdStr string, shellFunctionsFile string) oscommands.ICmdObj {
+func (self *gitCmdObjBuilder) NewShell(cmdStr string, shellFunctionsFile string) *oscommands.CmdObj {
 	return self.innerBuilder.NewShell(cmdStr, shellFunctionsFile).AddEnvVars(defaultEnvVar)
 }
 

pkg/commands/git_cmd_obj_runner.go

@@ -20,12 +20,12 @@ type gitCmdObjRunner struct {
 	innerRunner oscommands.ICmdObjRunner
 }
 
-func (self *gitCmdObjRunner) Run(cmdObj oscommands.ICmdObj) error {
+func (self *gitCmdObjRunner) Run(cmdObj *oscommands.CmdObj) error {
 	_, err := self.RunWithOutput(cmdObj)
 	return err
 }
 
-func (self *gitCmdObjRunner) RunWithOutput(cmdObj oscommands.ICmdObj) (string, error) {
+func (self *gitCmdObjRunner) RunWithOutput(cmdObj *oscommands.CmdObj) (string, error) {
 	var output string
 	var err error
 	for i := 0; i < RetryCount; i++ {
@@ -44,7 +44,7 @@ func (self *gitCmdObjRunner) RunWithOutput(cmdObj oscommands.ICmdObj) (string, e
 	return output, err
 }
 
-func (self *gitCmdObjRunner) RunWithOutputs(cmdObj oscommands.ICmdObj) (string, string, error) {
+func (self *gitCmdObjRunner) RunWithOutputs(cmdObj *oscommands.CmdObj) (string, string, error) {
 	var stdout, stderr string
 	var err error
 	for i := 0; i < RetryCount; i++ {
@@ -64,6 +64,6 @@ func (self *gitCmdObjRunner) RunWithOutputs(cmdObj oscommands.ICmdObj) (string,
 }
 
 // Retry logic not implemented here, but these commands typically don't need to obtain a lock.
-func (self *gitCmdObjRunner) RunAndProcessLines(cmdObj oscommands.ICmdObj, onLine func(line string) (bool, error)) error {
+func (self *gitCmdObjRunner) RunAndProcessLines(cmdObj *oscommands.CmdObj, onLine func(line string) (bool, error)) error {
 	return self.innerRunner.RunAndProcessLines(cmdObj, onLine)
 }

pkg/commands/git_commands/branch.go

@@ -154,7 +154,7 @@ func (self *BranchCommands) GetGraph(branchName string) (string, error) {
 	return self.GetGraphCmdObj(branchName).DontLog().RunWithOutput()
 }
 
-func (self *BranchCommands) GetGraphCmdObj(branchName string) oscommands.ICmdObj {
+func (self *BranchCommands) GetGraphCmdObj(branchName string) *oscommands.CmdObj {
 	branchLogCmdTemplate := self.UserConfig().Git.BranchLogCmd
 	templateValues := map[string]string{
 		"branchName": self.cmd.Quote(branchName),
@@ -255,7 +255,7 @@ func (self *BranchCommands) Merge(branchName string, opts MergeOpts) error {
 	return self.cmd.New(cmdArgs).Run()
 }
 
-func (self *BranchCommands) AllBranchesLogCmdObj() oscommands.ICmdObj {
+func (self *BranchCommands) AllBranchesLogCmdObj() *oscommands.CmdObj {
 	// Only choose between non-empty, non-identical commands
 	candidates := lo.Uniq(lo.WithoutEmpty(append([]string{
 		self.UserConfig().Git.AllBranchesLogCmd,

pkg/commands/git_commands/commit.go

@@ -85,7 +85,7 @@ func (self *CommitCommands) ResetToCommit(hash string, strength string, envVars
 		Run()
 }
 
-func (self *CommitCommands) CommitCmdObj(summary string, description string, forceSkipHooks bool) oscommands.ICmdObj {
+func (self *CommitCommands) CommitCmdObj(summary string, description string, forceSkipHooks bool) *oscommands.CmdObj {
 	messageArgs := self.commitMessageArgs(summary, description)
 	skipHookPrefix := self.UserConfig().Git.SkipHookPrefix
 	cmdArgs := NewGitCmd("commit").
@@ -97,16 +97,16 @@ func (self *CommitCommands) CommitCmdObj(summary string, description string, for
 	return self.cmd.New(cmdArgs)
 }
 
-func (self *CommitCommands) RewordLastCommitInEditorCmdObj() oscommands.ICmdObj {
+func (self *CommitCommands) RewordLastCommitInEditorCmdObj() *oscommands.CmdObj {
 	return self.cmd.New(NewGitCmd("commit").Arg("--allow-empty", "--amend", "--only").ToArgv())
 }
 
-func (self *CommitCommands) RewordLastCommitInEditorWithMessageFileCmdObj(tmpMessageFile string) oscommands.ICmdObj {
+func (self *CommitCommands) RewordLastCommitInEditorWithMessageFileCmdObj(tmpMessageFile string) *oscommands.CmdObj {
 	return self.cmd.New(NewGitCmd("commit").
 		Arg("--allow-empty", "--amend", "--only", "--edit", "--file="+tmpMessageFile).ToArgv())
 }
 
-func (self *CommitCommands) CommitInEditorWithMessageFileCmdObj(tmpMessageFile string, forceSkipHooks bool) oscommands.ICmdObj {
+func (self *CommitCommands) CommitInEditorWithMessageFileCmdObj(tmpMessageFile string, forceSkipHooks bool) *oscommands.CmdObj {
 	return self.cmd.New(NewGitCmd("commit").
 		ArgIf(forceSkipHooks, "--no-verify").
 		Arg("--edit").
@@ -116,7 +116,7 @@ func (self *CommitCommands) CommitInEditorWithMessageFileCmdObj(tmpMessageFile s
 }
 
 // RewordLastCommit rewords the topmost commit with the given message
-func (self *CommitCommands) RewordLastCommit(summary string, description string) oscommands.ICmdObj {
+func (self *CommitCommands) RewordLastCommit(summary string, description string) *oscommands.CmdObj {
 	messageArgs := self.commitMessageArgs(summary, description)
 
 	cmdArgs := NewGitCmd("commit").
@@ -138,7 +138,7 @@ func (self *CommitCommands) commitMessageArgs(summary string, description string
 }
 
 // runs git commit without the -m argument meaning it will invoke the user's editor
-func (self *CommitCommands) CommitEditorCmdObj() oscommands.ICmdObj {
+func (self *CommitCommands) CommitEditorCmdObj() *oscommands.CmdObj {
 	cmdArgs := NewGitCmd("commit").
 		ArgIf(self.signoffFlag() != "", self.signoffFlag()).
 		ToArgv()
@@ -246,15 +246,15 @@ func (self *CommitCommands) AmendHead() error {
 	return self.AmendHeadCmdObj().Run()
 }
 
-func (self *CommitCommands) AmendHeadCmdObj() oscommands.ICmdObj {
+func (self *CommitCommands) AmendHeadCmdObj() *oscommands.CmdObj {
 	cmdArgs := NewGitCmd("commit").
 		Arg("--amend", "--no-edit", "--allow-empty").
 		ToArgv()
 
 	return self.cmd.New(cmdArgs)
 }
 
-func (self *CommitCommands) ShowCmdObj(hash string, filterPath string) oscommands.ICmdObj {
+func (self *CommitCommands) ShowCmdObj(hash string, filterPath string) *oscommands.CmdObj {
 	contextSize := self.AppState.DiffContextSize
 
 	extDiffCmd := self.UserConfig().Git.Paging.ExternalDiffCommand
@@ -278,7 +278,7 @@ func (self *CommitCommands) ShowCmdObj(hash string, filterPath string) oscommand
 	return self.cmd.New(cmdArgs).DontLog()
 }
 
-func (self *CommitCommands) ShowFileContentCmdObj(hash string, filePath string) oscommands.ICmdObj {
+func (self *CommitCommands) ShowFileContentCmdObj(hash string, filePath string) *oscommands.CmdObj {
 	cmdArgs := NewGitCmd("show").
 		Arg(fmt.Sprintf("%s:%s", hash, filePath)).
 		ToArgv()

pkg/commands/git_commands/commit_loader.go

@@ -583,7 +583,7 @@ func (self *CommitLoader) getFirstPushedCommit(refName string) (string, error) {
 }
 
 // getLog gets the git log.
-func (self *CommitLoader) getLogCmd(opts GetCommitsOptions) oscommands.ICmdObj {
+func (self *CommitLoader) getLogCmd(opts GetCommitsOptions) *oscommands.CmdObj {
 	gitLogOrder := self.AppState.GitLogOrder
 
 	refSpec := opts.RefName

pkg/commands/git_commands/diff.go

@@ -18,7 +18,7 @@ func NewDiffCommands(gitCommon *GitCommon) *DiffCommands {
 
 // This is for generating diffs to be shown in the UI (e.g. rendering a range
 // diff to the main view). It uses a custom pager if one is configured.
-func (self *DiffCommands) DiffCmdObj(diffArgs []string) oscommands.ICmdObj {
+func (self *DiffCommands) DiffCmdObj(diffArgs []string) *oscommands.CmdObj {
 	extDiffCmd := self.UserConfig().Git.Paging.ExternalDiffCommand
 	useExtDiff := extDiffCmd != ""
 	ignoreWhitespace := self.AppState.IgnoreWhitespaceInDiffView
@@ -83,7 +83,7 @@ type DiffToolCmdOptions struct {
 	Staged bool
 }
 
-func (self *DiffCommands) OpenDiffToolCmdObj(opts DiffToolCmdOptions) oscommands.ICmdObj {
+func (self *DiffCommands) OpenDiffToolCmdObj(opts DiffToolCmdOptions) *oscommands.CmdObj {
 	return self.cmd.New(NewGitCmd("difftool").
 		Arg("--no-prompt").
 		ArgIf(opts.IsDirectory, "--dir-diff").
@@ -95,7 +95,7 @@ func (self *DiffCommands) OpenDiffToolCmdObj(opts DiffToolCmdOptions) oscommands
 		ToArgv())
 }
 
-func (self *DiffCommands) DiffIndexCmdObj(diffArgs ...string) oscommands.ICmdObj {
+func (self *DiffCommands) DiffIndexCmdObj(diffArgs ...string) *oscommands.CmdObj {
 	return self.cmd.New(
 		NewGitCmd("diff-index").
 			Config("diff.noprefix=false").

pkg/commands/git_commands/flow.go

@@ -24,7 +24,7 @@ func (self *FlowCommands) GitFlowEnabled() bool {
 	return self.config.GetGitFlowPrefixes() != ""
 }
 
-func (self *FlowCommands) FinishCmdObj(branchName string) (oscommands.ICmdObj, error) {
+func (self *FlowCommands) FinishCmdObj(branchName string) (*oscommands.CmdObj, error) {
 	prefixes := self.config.GetGitFlowPrefixes()
 
 	// need to find out what kind of branch this is
@@ -54,7 +54,7 @@ func (self *FlowCommands) FinishCmdObj(branchName string) (oscommands.ICmdObj, e
 	return self.cmd.New(cmdArgs), nil
 }
 
-func (self *FlowCommands) StartCmdObj(branchType string, name string) oscommands.ICmdObj {
+func (self *FlowCommands) StartCmdObj(branchType string, name string) *oscommands.CmdObj {
 	cmdArgs := NewGitCmd("flow").Arg(branchType, "start", name).ToArgv()
 
 	return self.cmd.New(cmdArgs)

pkg/commands/git_commands/rebase.go

@@ -53,7 +53,7 @@ func (self *RebaseCommands) RewordCommit(commits []*models.Commit, index int, su
 	return self.ContinueRebase()
 }
 
-func (self *RebaseCommands) RewordCommitInEditor(commits []*models.Commit, index int) (oscommands.ICmdObj, error) {
+func (self *RebaseCommands) RewordCommitInEditor(commits []*models.Commit, index int) (*oscommands.CmdObj, error) {
 	changes := []daemon.ChangeTodoAction{{
 		Hash:      commits[index].Hash(),
 		NewAction: todo.Reword,
@@ -209,7 +209,7 @@ type PrepareInteractiveRebaseCommandOpts struct {
 // PrepareInteractiveRebaseCommand returns the cmd for an interactive rebase
 // we tell git to run lazygit to edit the todo list, and we pass the client
 // lazygit instructions what to do with the todo file
-func (self *RebaseCommands) PrepareInteractiveRebaseCommand(opts PrepareInteractiveRebaseCommandOpts) oscommands.ICmdObj {
+func (self *RebaseCommands) PrepareInteractiveRebaseCommand(opts PrepareInteractiveRebaseCommandOpts) *oscommands.CmdObj {
 	ex := oscommands.GetLazygitPath()
 
 	cmdArgs := NewGitCmd("rebase").
@@ -446,7 +446,7 @@ func (self *RebaseCommands) RebaseBranchFromBaseCommit(targetBranchName string,
 	}).Run()
 }
 
-func (self *RebaseCommands) GenericMergeOrRebaseActionCmdObj(commandType string, command string) oscommands.ICmdObj {
+func (self *RebaseCommands) GenericMergeOrRebaseActionCmdObj(commandType string, command string) *oscommands.CmdObj {
 	cmdArgs := NewGitCmd(commandType).Arg("--" + command).ToArgv()
 
 	return self.cmd.New(cmdArgs)
@@ -485,7 +485,7 @@ func (self *RebaseCommands) GenericMergeOrRebaseAction(commandType string, comma
 	return nil
 }
 
-func (self *RebaseCommands) runSkipEditorCommand(cmdObj oscommands.ICmdObj) error {
+func (self *RebaseCommands) runSkipEditorCommand(cmdObj *oscommands.CmdObj) error {
 	instruction := daemon.NewExitImmediatelyInstruction()
 	lazyGitPath := oscommands.GetLazygitPath()
 	return cmdObj.

pkg/commands/git_commands/rebase_test.go

@@ -69,7 +69,7 @@ func TestRebaseRebaseBranch(t *testing.T) {
 // environment variables that suppress an interactive editor
 func TestRebaseSkipEditorCommand(t *testing.T) {
 	cmdArgs := []string{"git", "blah"}
-	runner := oscommands.NewFakeRunner(t).ExpectFunc("matches editor env var", func(cmdObj oscommands.ICmdObj) bool {
+	runner := oscommands.NewFakeRunner(t).ExpectFunc("matches editor env var", func(cmdObj *oscommands.CmdObj) bool {
 		assert.EqualValues(t, cmdArgs, cmdObj.Args())
 		envVars := cmdObj.GetEnvVars()
 		for _, regexStr := range []string{

pkg/commands/git_commands/stash.go

@@ -80,7 +80,7 @@ func (self *StashCommands) Hash(index int) (string, error) {
 	return strings.Trim(hash, "\r\n"), err
 }
 
-func (self *StashCommands) ShowStashEntryCmdObj(index int) oscommands.ICmdObj {
+func (self *StashCommands) ShowStashEntryCmdObj(index int) *oscommands.CmdObj {
 	// "-u" is the same as "--include-untracked", but the latter fails in older git versions for some reason
 	cmdArgs := NewGitCmd("stash").Arg("show").
 		Arg("-p").

pkg/commands/git_commands/submodule.go

@@ -237,35 +237,35 @@ func (self *SubmoduleCommands) Update(path string) error {
 	return self.cmd.New(cmdArgs).Run()
 }
 
-func (self *SubmoduleCommands) BulkInitCmdObj() oscommands.ICmdObj {
+func (self *SubmoduleCommands) BulkInitCmdObj() *oscommands.CmdObj {
 	cmdArgs := NewGitCmd("submodule").Arg("init").
 		ToArgv()
 
 	return self.cmd.New(cmdArgs)
 }
 
-func (self *SubmoduleCommands) BulkUpdateCmdObj() oscommands.ICmdObj {
+func (self *SubmoduleCommands) BulkUpdateCmdObj() *oscommands.CmdObj {
 	cmdArgs := NewGitCmd("submodule").Arg("update").
 		ToArgv()
 
 	return self.cmd.New(cmdArgs)
 }
 
-func (self *SubmoduleCommands) ForceBulkUpdateCmdObj() oscommands.ICmdObj {
+func (self *SubmoduleCommands) ForceBulkUpdateCmdObj() *oscommands.CmdObj {
 	cmdArgs := NewGitCmd("submodule").Arg("update", "--force").
 		ToArgv()
 
 	return self.cmd.New(cmdArgs)
 }
 
-func (self *SubmoduleCommands) BulkUpdateRecursivelyCmdObj() oscommands.ICmdObj {
+func (self *SubmoduleCommands) BulkUpdateRecursivelyCmdObj() *oscommands.CmdObj {
 	cmdArgs := NewGitCmd("submodule").Arg("update", "--init", "--recursive").
 		ToArgv()
 
 	return self.cmd.New(cmdArgs)
 }
 
-func (self *SubmoduleCommands) BulkDeinitCmdObj() oscommands.ICmdObj {
+func (self *SubmoduleCommands) BulkDeinitCmdObj() *oscommands.CmdObj {
 	cmdArgs := NewGitCmd("submodule").Arg("deinit", "--all", "--force").
 		ToArgv()
 

pkg/commands/git_commands/sync.go

@@ -28,7 +28,7 @@ type PushOpts struct {
 	SetUpstream    bool
 }
 
-func (self *SyncCommands) PushCmdObj(task gocui.Task, opts PushOpts) (oscommands.ICmdObj, error) {
+func (self *SyncCommands) PushCmdObj(task gocui.Task, opts PushOpts) (*oscommands.CmdObj, error) {
 	if opts.UpstreamBranch != "" && opts.UpstreamRemote == "" {
 		return nil, errors.New(self.Tr.MustSpecifyOriginError)
 	}
@@ -62,7 +62,7 @@ func (self *SyncCommands) fetchCommandBuilder(fetchAll bool) *GitCommandBuilder
 		ArgIf(self.version.IsAtLeast(2, 29, 0), "--no-write-fetch-head")
 }
 
-func (self *SyncCommands) FetchCmdObj(task gocui.Task) oscommands.ICmdObj {
+func (self *SyncCommands) FetchCmdObj(task gocui.Task) *oscommands.CmdObj {
 	cmdArgs := self.fetchCommandBuilder(self.UserConfig().Git.FetchAll).ToArgv()
 
 	cmdObj := self.cmd.New(cmdArgs)
@@ -74,7 +74,7 @@ func (self *SyncCommands) Fetch(task gocui.Task) error {
 	return self.FetchCmdObj(task).Run()
 }
 
-func (self *SyncCommands) FetchBackgroundCmdObj() oscommands.ICmdObj {
+func (self *SyncCommands) FetchBackgroundCmdObj() *oscommands.CmdObj {
 	cmdArgs := self.fetchCommandBuilder(self.UserConfig().Git.FetchAll).ToArgv()
 
 	cmdObj := self.cmd.New(cmdArgs)