From e68b50b9648c37a299d61f18840e4a4aed02286c Mon Sep 17 00:00:00 2001 From: Lucas Molas Date: Tue, 18 Jan 2022 13:11:30 -0300 Subject: [PATCH 1/4] feat: add command output option --- cli/run.go | 23 +++++++++++++++++++---- executor.go | 2 ++ opts.go | 12 ++++++++---- 3 files changed, 29 insertions(+), 8 deletions(-) diff --git a/cli/run.go b/cli/run.go index 46b68280..cd223e9f 100644 --- a/cli/run.go +++ b/cli/run.go @@ -24,7 +24,7 @@ type Closer interface { } func Run(ctx context.Context, root *cmds.Command, - cmdline []string, stdin, stdout, stderr *os.File, + cmdline []string, stdin, stderr *os.File, buildEnv cmds.MakeEnvironment, makeExecutor cmds.MakeExecutor) error { printErr := func(err error) { @@ -33,6 +33,21 @@ func Run(ctx context.Context, root *cmds.Command, req, errParse := Parse(ctx, cmdline[1:], stdin, root) + var out *os.File + if outFile, _ := req.Options[cmds.OutputFile].(string); outFile != "" { + // FIXME: Consider exporting the extract file logic (used in `ipfs get`) + // and using it here (https://github.com/ipfs/tar-utils/blob/16821db/extractor.go#L277). + var err error + out, err = os.Create(outFile) + if err != nil { + printErr(err) + return err + } + // FIXME(BLOCKING): Where do we close the file? When the resp emitter closes? + } else { + out = os.Stdout + } + // Handle the timeout up front. var cancel func() if timeoutStr, ok := req.Options[cmds.TimeoutOpt]; ok { @@ -72,7 +87,7 @@ func Run(ctx context.Context, root *cmds.Command, // BEFORE handling the parse error, if we have enough information // AND the user requested help, print it out and exit - err := HandleHelp(cmdline[0], req, stdout) + err := HandleHelp(cmdline[0], req, out) if err == nil { return nil } else if err != ErrNoHelpRequested { @@ -98,7 +113,7 @@ func Run(ctx context.Context, root *cmds.Command, // - commands with no Run func are invoked directly. // - the main command is invoked. if req == nil || req.Command == nil || req.Command.Run == nil { - printHelp(false, stdout) + printHelp(false, out) return nil } @@ -127,7 +142,7 @@ func Run(ctx context.Context, root *cmds.Command, req.Options[cmds.EncLong] = cmds.JSON } - re, err := NewResponseEmitter(stdout, stderr, req) + re, err := NewResponseEmitter(out, stderr, req) if err != nil { printErr(err) return err diff --git a/executor.go b/executor.go index 63a7fc44..a73fe95e 100644 --- a/executor.go +++ b/executor.go @@ -28,6 +28,8 @@ func NewExecutor(root *Command) Executor { } } +// This is the *CLI* Executor (though is not in that package). The other one is +// the `client` HTTP Executor (defined in the HTTP package). type executor struct { root *Command } diff --git a/opts.go b/opts.go index 11c6b2f6..8d8466bb 100644 --- a/opts.go +++ b/opts.go @@ -12,10 +12,13 @@ const ( OptLongHelp = "help" DerefLong = "dereference-args" StdinName = "stdin-name" - Hidden = "hidden" - HiddenShort = "H" - Ignore = "ignore" - IgnoreRules = "ignore-rules-path" + // FIXME(BLOCKING): Review names. (We may be able to use "output" once + // it's cleaned from subcommands.) + OutputFile = "command-output" + Hidden = "hidden" + HiddenShort = "H" + Ignore = "ignore" + IgnoreRules = "ignore-rules-path" ) // options that are used by this package @@ -25,6 +28,7 @@ var OptionStreamChannels = BoolOption(ChanOpt, "Stream channel output") var OptionTimeout = StringOption(TimeoutOpt, "Set a global timeout on the command") var OptionDerefArgs = BoolOption(DerefLong, "Symlinks supplied in arguments are dereferenced") var OptionStdinName = StringOption(StdinName, "Assign a name if the file source is stdin.") +var OptionOutputFile = StringOption(OutputFile, "Filename to save the output in (by default it's redirected to stdout).") var OptionHidden = BoolOption(Hidden, HiddenShort, "Include files that are hidden. Only takes effect on recursive add.") var OptionIgnore = StringsOption(Ignore, "A rule (.gitignore-stype) defining which file(s) should be ignored (variadic, experimental)") var OptionIgnoreRules = StringOption(IgnoreRules, "A path to a file with .gitignore-style ignore rules (experimental)") From 8e207aa95f580c13f5aff091cebdf33fb066c049 Mon Sep 17 00:00:00 2001 From: Lucas Molas Date: Tue, 18 Jan 2022 13:21:58 -0300 Subject: [PATCH 2/4] fix test --- cli/run_test.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/cli/run_test.go b/cli/run_test.go index d318bc11..0b5ff525 100644 --- a/cli/run_test.go +++ b/cli/run_test.go @@ -41,7 +41,8 @@ func TestRunWaits(t *testing.T) { context.Background(), root, []string{"test", "test"}, - devnull, devnull, devnull, + // FIXME(BLOCKING): Redirect to devnull instead of stdout. + devnull, devnull, func(ctx context.Context, req *cmds.Request) (cmds.Environment, error) { return env{flag}, nil }, From 4ec6ee8cf6c9a8259288db5a556b3fd3ee853012 Mon Sep 17 00:00:00 2001 From: Lucas Molas Date: Fri, 28 Jan 2022 10:48:31 -0300 Subject: [PATCH 3/4] add error output option --- cli/run.go | 60 ++++++++++++++++++++++++++++++------------------- cli/run_test.go | 2 +- opts.go | 2 ++ 3 files changed, 40 insertions(+), 24 deletions(-) diff --git a/cli/run.go b/cli/run.go index cd223e9f..6e3d2cca 100644 --- a/cli/run.go +++ b/cli/run.go @@ -24,28 +24,18 @@ type Closer interface { } func Run(ctx context.Context, root *cmds.Command, - cmdline []string, stdin, stderr *os.File, + cmdline []string, stdin *os.File, buildEnv cmds.MakeEnvironment, makeExecutor cmds.MakeExecutor) error { - printErr := func(err error) { - fmt.Fprintf(stderr, "Error: %s\n", err) - } - req, errParse := Parse(ctx, cmdline[1:], stdin, root) - var out *os.File - if outFile, _ := req.Options[cmds.OutputFile].(string); outFile != "" { - // FIXME: Consider exporting the extract file logic (used in `ipfs get`) - // and using it here (https://github.com/ipfs/tar-utils/blob/16821db/extractor.go#L277). - var err error - out, err = os.Create(outFile) - if err != nil { - printErr(err) - return err - } - // FIXME(BLOCKING): Where do we close the file? When the resp emitter closes? - } else { - out = os.Stdout + out, errFile, err := createDumpFiles(req) + if err != nil { + return err + } + + printErr := func(err error) { + fmt.Fprintf(errFile, "Error: %s\n", err) } // Handle the timeout up front. @@ -87,7 +77,7 @@ func Run(ctx context.Context, root *cmds.Command, // BEFORE handling the parse error, if we have enough information // AND the user requested help, print it out and exit - err := HandleHelp(cmdline[0], req, out) + err = HandleHelp(cmdline[0], req, out) if err == nil { return nil } else if err != ErrNoHelpRequested { @@ -102,8 +92,8 @@ func Run(ctx context.Context, root *cmds.Command, // this was a user error, print help if req != nil && req.Command != nil { - fmt.Fprintln(stderr) // i need some space - printHelp(false, stderr) + fmt.Fprintln(errFile) // i need some space + printHelp(false, errFile) } return errParse @@ -142,7 +132,7 @@ func Run(ctx context.Context, root *cmds.Command, req.Options[cmds.EncLong] = cmds.JSON } - re, err := NewResponseEmitter(out, stderr, req) + re, err := NewResponseEmitter(out, errFile, req) if err != nil { printErr(err) return err @@ -159,7 +149,7 @@ func Run(ctx context.Context, root *cmds.Command, err = *kiterr } if kiterr, ok := err.(cmds.Error); ok && kiterr.Code == cmds.ErrClient { - printMetaHelp(stderr) + printMetaHelp(errFile) } return err @@ -170,3 +160,27 @@ func Run(ctx context.Context, root *cmds.Command, } return nil } + +func createDumpFiles(req *cmds.Request) (*os.File, *os.File, error) { + var err error + outFile := os.Stdout + if outPath, _ := req.Options[cmds.OutputFile].(string); outPath != "" { + // FIXME: Consider exporting the extract file logic (used in `ipfs get`) + // and using it here (https://github.com/ipfs/tar-utils/blob/16821db/extractor.go#L277). + outFile, err = os.Create(outPath) + if err != nil { + return nil, nil, err + } + // FIXME(BLOCKING): Where do we close the file? When the resp emitter closes? + } + + errorFile := os.Stderr + if errPath, _ := req.Options[cmds.ErrorFile].(string); errPath != "" { + errorFile, err = os.Create(errPath) + if err != nil { + return nil, nil, err + } + } + + return outFile, errorFile, nil +} diff --git a/cli/run_test.go b/cli/run_test.go index 0b5ff525..ded7dd41 100644 --- a/cli/run_test.go +++ b/cli/run_test.go @@ -42,7 +42,7 @@ func TestRunWaits(t *testing.T) { root, []string{"test", "test"}, // FIXME(BLOCKING): Redirect to devnull instead of stdout. - devnull, devnull, + devnull, func(ctx context.Context, req *cmds.Request) (cmds.Environment, error) { return env{flag}, nil }, diff --git a/opts.go b/opts.go index 8d8466bb..9addfb75 100644 --- a/opts.go +++ b/opts.go @@ -15,6 +15,7 @@ const ( // FIXME(BLOCKING): Review names. (We may be able to use "output" once // it's cleaned from subcommands.) OutputFile = "command-output" + ErrorFile = "command-error" Hidden = "hidden" HiddenShort = "H" Ignore = "ignore" @@ -29,6 +30,7 @@ var OptionTimeout = StringOption(TimeoutOpt, "Set a global timeout on the comman var OptionDerefArgs = BoolOption(DerefLong, "Symlinks supplied in arguments are dereferenced") var OptionStdinName = StringOption(StdinName, "Assign a name if the file source is stdin.") var OptionOutputFile = StringOption(OutputFile, "Filename to save the output in (by default it's redirected to stdout).") +var OptionErrorFile = StringOption(OutputFile, "Filename to save the error in (by default it's redirected to stderr).") var OptionHidden = BoolOption(Hidden, HiddenShort, "Include files that are hidden. Only takes effect on recursive add.") var OptionIgnore = StringsOption(Ignore, "A rule (.gitignore-stype) defining which file(s) should be ignored (variadic, experimental)") var OptionIgnoreRules = StringOption(IgnoreRules, "A path to a file with .gitignore-style ignore rules (experimental)") From a5cefcec29d0d70fff6d3df20f6ae0ceac13d842 Mon Sep 17 00:00:00 2001 From: Lucas Molas Date: Fri, 22 Apr 2022 12:25:32 -0300 Subject: [PATCH 4/4] Update opts.go Co-authored-by: Marcin Rataj --- opts.go | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/opts.go b/opts.go index 9addfb75..38089f2a 100644 --- a/opts.go +++ b/opts.go @@ -12,10 +12,8 @@ const ( OptLongHelp = "help" DerefLong = "dereference-args" StdinName = "stdin-name" - // FIXME(BLOCKING): Review names. (We may be able to use "output" once - // it's cleaned from subcommands.) - OutputFile = "command-output" - ErrorFile = "command-error" + StdoutFile = "stdout" + StderrFile = "stderr" Hidden = "hidden" HiddenShort = "H" Ignore = "ignore"