ipfs / ipfs/kubo

Add command does not support json encoding

Open
#1,121 17 comments 0 reactions 0 assignees View on GitHub
kind/bug topic/commands
Dominant language
Go
Stars
17.1k
Forks
3.2k
Avg merge
3d 18h
Merged PRs (30d)
11

Description

Currently the add command does not support JSON encoding.

This is due [to setting the output to nil](https://github.com/ipfs/go-ipfs/blob/master/core/commands/add.go#L139).

A fix to this is to detect the output format and skip this `PostRun` if the encoding is anything other than `Text`.

``` go
v, found, err := req.Option(cmds.EncShort).String()
if err == nil && found && v != cmds.Text {
log.Debugf("Encoding is not text, abort and let the marshaller handle it\n")
return
}
```

This however introduced another issue. All output is returned as JSON. If the `previoususerProvidedEncoding` [is not set, we keep the JSON encoding](https://github.com/ipfs/go-ipfs/blob/master/commands/http/client.go#L114).

The reason we don't have a default encoding is due to using two different option keys, [in the client.go file we use](https://github.com/ipfs/go-ipfs/blob/master/commands/http/client.go#L39) `cmds.EncShort`, [while in the cli we use `encoding`](https://github.com/ipfs/go-ipfs/blob/db56c0f16a71b8cf36078a7233397de9d5b5f55a/cmd/ipfs/main.go#L239).

However, there is another issue, since the `add` command does not use a marshaler, [the check on line 240](https://github.com/ipfs/go-ipfs/blob/db56c0f16a71b8cf36078a7233397de9d5b5f55a/cmd/ipfs/main.go#L239) will fail and switches to JSON.

PostRun is used for progress, [and at some point the Marshaler was removed](https://github.com/ipfs/go-ipfs/commit/121dfb10b4b8d3a2fd4268943e65b30e5a8301a9#diff-bd0ca9f1e2d5c7ba128af01ec4c1131c).

I don't know the history of this file, so there might of been a good reason (not used?). Anyways without a Marshaler the original encoding will never get set back.

This is a start of a PR I want to open

``` diff
diff --git a/cmd/ipfs/main.go b/cmd/ipfs/main.go
index 2ad90cf..411b833 100644
--- a/cmd/ipfs/main.go
+++ b/cmd/ipfs/main.go
@@ -236,11 +236,11 @@ func (i *cmdInvocation) Parse(ctx context.Context, args []string) error {

// if no encoding was specified by user, default to plaintext encoding
// (if command doesn't support plaintext, use JSON instead)
- if !i.req.Option("encoding").Found() {
+ if !i.req.Option(cmds.EncShort).Found() {
if i.req.Command().Marshalers != nil && i.req.Command().Marshalers[cmds.Text] != nil {
- i.req.SetOption("encoding", cmds.Text)
+ i.req.SetOption(cmds.EncShort, cmds.Text)
} else {
- i.req.SetOption("encoding", cmds.JSON)
+ i.req.SetOption(cmds.EncShort, cmds.JSON)
}
}

diff --git a/core/commands/add.go b/core/commands/add.go
index 2b3297e..b2fc45f 100644
--- a/core/commands/add.go
+++ b/core/commands/add.go
@@ -127,7 +127,18 @@ remains to be implemented.
}
}()
},
+ Marshalers: cmds.MarshalerMap{
+ cmds.Text: func(res cmds.Response) (io.Reader, error) {
+ return nil, nil
+ },
+ },
PostRun: func(req cmds.Request, res cmds.Response) {
+ v, found, err := req.Option(cmds.EncShort).String()
+ if err == nil && found && v != cmds.Text {
+ log.Debugf("Encoding is not text, abort and let the marshaller handle it\n")
+ return
+ }
+
if res.Error() != nil {
return
}
```

I just want to get some feedback before making any commits.

/cc @jbenet @whyrusleeping @kyledrake

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.