Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

PostRun is an awkward fit for CLI output #115

Open
Stebalien opened this issue Aug 10, 2018 · 2 comments · May be fixed by #215
Open

PostRun is an awkward fit for CLI output #115

Stebalien opened this issue Aug 10, 2018 · 2 comments · May be fixed by #215
Assignees
Labels
kind/enhancement A net-new feature or improvement to an existing feature

Comments

@Stebalien
Copy link
Member

When writing CLI output, we often want to write to stdout/stderr, sometimes even controlling terminal output. For commands that just need to format to stdout, we can use a text marshaler. However, for commands that need to write to some parts to stderr and/or control the terminal, that doesn't really fit.

So, in practice, we use a PostRun function. However, that isn't a fit either as it expects us to transform a response emitter.

Currently, we just hack it by writing to stdout/stderr anyways but I'd like a better option.


So, I'd like to:

  1. Deprecate PostRun. If you look at how we use it, we don't actually use it to transform one response emitter into another, we use it to write to the console. We could keep it, but, IMO, the current form doesn't fit very well. I can't think of a case where I'd want to transform the results based on the output mode.
  2. Encourage using Encoders when we don't need anything fancy (just text and a terminal error).
  3. Provide a DisplayCLI field that takes a func(res cmds.Response, stdout, stderr io.Writer) error.

In the future, we could add additional display functions for different interfaces (e.g., browsers) but this seems like a good starting point. Thoughts?

@Stebalien Stebalien added the kind/enhancement A net-new feature or improvement to an existing feature label Aug 10, 2018
@kevina

This comment has been minimized.

@Stebalien

This comment has been minimized.

jbouwman pushed a commit to jbouwman/go-ipfs-cmds that referenced this issue Aug 19, 2021
Several open issues mention problems with interaction of the
global `--encoding=` flag and the Encoders and PostRun fields of
command structs. This branch contains experimental refactors that
explore approaches to consistent command execution patterns across
offline, online and http modes.

Specific tickets:

- ipfs/kubo#7050 json encoding for `ls`
- ipfs/kubo#1121 json encoding for `add`
- ipfs/kubo#5594 json encoding for `stats bw`
- ipfs#115 postrun design

Possibly related:

- ipfs/kubo#6640 global flags on subcommands

Incomplete PRs:

- ipfs/kubo#5620 json for 'stat'
jbouwman added a commit to jbouwman/go-ipfs-cmds that referenced this issue Aug 21, 2021
Following the approach described in ipfs#115, define a new method signature on `Command` that supports full processing of the `Response` object when text encoding is requested.

Add an encoding check and dispatch to DisplayCLI in local, http client, and http handler code paths.

Unblocks resolution of `encoding` option processing in multiple go-ipfs issues.

- ipfs/kubo#7050 json encoding for `ls`
- ipfs/kubo#1121 json encoding for `add`
- ipfs/kubo#5594 json encoding for `stats bw`
jbouwman added a commit to jbouwman/go-ipfs-cmds that referenced this issue Sep 8, 2021
Following the approach described in ipfs#115, define a new method signature on `Command` that supports full processing of the `Response` object when text encoding is requested.

Add an encoding check and dispatch to DisplayCLI in local, http client, and http handler code paths.

Unblocks resolution of `encoding` option processing in multiple go-ipfs issues.

- ipfs/kubo#7050 json encoding for `ls`
- ipfs/kubo#1121 json encoding for `add`
- ipfs/kubo#5594 json encoding for `stats bw`
@BigLep BigLep added this to the Best Effort Track milestone Mar 10, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/enhancement A net-new feature or improvement to an existing feature
Projects
No open projects
Status: 🏃‍♀️ In Progress
Development

Successfully merging a pull request may close this issue.

3 participants