commands/refs: use new cmds #5679
Conversation
core/commands/refs.go
Outdated
| if err != nil { | ||
| res.SetError(err, cmdkit.ErrNormal) | ||
| return | ||
| return err | ||
| } | ||
|
|
||
| out := make(chan interface{}) |
There was a problem hiding this comment.
We should likely drop this channel and go routine and use res.Emit directly for each value.
There was a problem hiding this comment.
Due to an Oddity with how parsing arguments from Stdin works when ever you have a command that can accept more than one argument from Stdin you must either call req.ParseBodyArgs() before using req.Arguments or iterate over them with req.BodyArgs(), with the former being preferred for now. If you don't only the first argument will be read from Stdin.
You can test this my trying to pass two arguments via the command line to ipfs refs and then try again via Stdin.
overbool
force-pushed
the
refactor/commands/refs
branch
from
a35828a
to
e90b2f4
Compare
|
@overbool I would also like to get this p.r. in ASAP, let me know if you need any help with this. |
License: MIT Signed-off-by: Overbool <[email protected]>
License: MIT Signed-off-by: Overbool <[email protected]>
overbool
force-pushed
the
refactor/commands/refs
branch
3 times, most recently
from
b858d06
to
bbbaf38
Compare
License: MIT Signed-off-by: Overbool <[email protected]>
overbool
force-pushed
the
refactor/commands/refs
branch
from
bbbaf38
to
30c6dd9
Compare
License: MIT Signed-off-by: Overbool <[email protected]>
overbool
changed the title
|
@kevina I had fixed them, could u help me review again? |
| ctx := req.Context() | ||
| n, err := req.InvocContext().GetNode() | ||
| Run: func(req *cmds.Request, res cmds.ResponseEmitter, env cmds.Environment) error { | ||
| err := req.ParseBodyArgs() |
There was a problem hiding this comment.
Meh? Honestly, if we're going to do this up-front anyways, we might as well finish parsing the request as early as possible.
There was a problem hiding this comment.
We can change this later if you'd like.
There was a problem hiding this comment.
I don't really have a strong preference but we should at least try and be consistent about when we do this.
In any case fixing it now is a low priority.
Refer: #5664
License: MIT
Signed-off-by: Overbool [email protected]