commands/ls: use new cmds lib #5663
Conversation
overbool
force-pushed
the
refactor/commands/ls
branch
2 times, most recently
from
045efe0
to
f7339eb
Compare
License: MIT Signed-off-by: Overbool <[email protected]>
overbool
force-pushed
the
refactor/commands/ls
branch
from
f7339eb
to
52bb551
Compare
There was a problem hiding this comment.
Couple of small changes but otherwise LGTM.
However, I'd like to delay this until we make a decision on #5611. That PR includes this change (switches to the new commands lib) but also make some significantly more complicated changes that would be a pain to rebase on-top of this PR.
| @@ -93,43 +81,39 @@ The JSON output contains type information. | |||
| dserv = merkledag.NewDAGService(bserv) | |||
| } | |||
|
|
|||
| paths := req.Arguments() | |||
| paths := req.Arguments | |||
There was a problem hiding this comment.
There really isn't a need to set this anymore, we can just use it directly in the loop below.
| for _, object := range output.Objects { | ||
| if len(output.Objects) > 1 { | ||
| fmt.Fprintf(w, "%s:\n", object.Hash) | ||
| tw := tabwriter.NewWriter(buf, 1, 2, 1, ' ', 0) |
There was a problem hiding this comment.
Let's pass w into tabwriter.NewWriter directly (and get rid of buf entirely). We used to put this all in a buffer because we had to return it from the command but that's no longer necessary.
Stebalien
added
the
status/blocked
|
@overbool and #5611 is dependent on ipfs/go-unixfs#39 -- which is almost merged, and then I'll move my focus to #5611 |
|
@Stebalien I would rather we not delay this as this the switch to the new commands lib is blocking any work on #5464 and #5611 may take a while to get in. |
Stebalien
added
status/blocked
Refer: #5664
License: MIT
Signed-off-by: Overbool [email protected]