Commands Refactor Part 2 #263

jbenet · 2014-11-04T09:54:41Z

We split #196 in two parts. this is the second. (#262 is the first). This builds on all the changes to commands/.

jbenet · 2014-11-04T09:55:47Z

warning

Rebasing this on top of master once #262 lands.

jbenet · 2014-11-04T10:16:43Z

okay, this should be ready. cc @mappum @maybebtc

btc · 2014-11-04T10:23:51Z

thanks for the git ninja work. very much appreciated.

will have a look

jbenet · 2014-11-04T10:28:54Z

This branch's diff --stat master

 Makefile                   |   5 ++
 cmd/ipfs2/.gitignore       |   2 +
 cmd/ipfs2/Makefile         |   7 ++
 cmd/ipfs2/README.md        |  29 +++++++++
 cmd/ipfs2/daemon.go        |  63 ++++++++++++++++++
 cmd/ipfs2/equinox.yaml     |   6 ++
 cmd/ipfs2/init.go          | 157 ++++++++++++++++++++++++++++++++++++++++++++
 cmd/ipfs2/ipfs.go          |  15 +++++
 cmd/ipfs2/main.go          | 234 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 core/commands2/add.go      |  90 ++++++++++++++++++++++++++
 core/commands2/cat.go      |  39 +++++++++++
 core/commands2/commands.go |  59 +++++++++++++++++
 core/commands2/log.go      |  30 +++++++++
 core/commands2/ls.go       |  78 ++++++++++++++++++++++
 core/commands2/publish.go  |  72 +++++++++++++++++++++
 core/commands2/root.go     | 127 ++++++++++++++++++++++++++++++++++++
 daemon2/daemon.go          |  25 +++++++
 server/http/http.go        |   2 +
 18 files changed, 1040 insertions(+)

jbenet · 2014-11-04T10:29:44Z

btc · 2014-11-04T10:57:24Z

@jbenet @mappum
question regarding ipfs versus ipfs2. ipfs prints help text when no arguments provided. This is kinda helpful. Do we want the same from the new cli? (Presently, shows no message. see image below)

From here forward, will mark my comments with TODO checklists to help track state.

🐯

jbenet · 2014-11-04T11:00:46Z

Yes, we definitely want help text printed out.

btc · 2014-11-04T11:09:20Z

cmd/ipfs2/daemon.go

+		res.SetError(err, cmds.ErrNormal)
+		return
+	}
+	ctx.Node = node


@mappum

~~Does this assignment have a side-effect?~~

Read further and realized the context is passed to the handler with the node in tow.

Is there a plan for removing IPFS-specific concepts from the commands.Context? Doesn't seem like this library could be extracted otherwise.

Yep, that's the reason I kept all the IPFS-specific stuff contained in the Context, but I'm not sure exactly how we should do it. Maybe I should just change it to a map[string]interface{}? Anyway, I'm not too worried about making the library vendorable right away since making it work with IPFS is higher priority.

Yeah I agree. Let's try to not introduce more ipfs specific things and vendor later

—
Sent from Mailbox

On Tue, Nov 4, 2014 at 3:30 PM, Matt Bell [email protected]
wrote:

+func daemonFunc(res cmds.Response, req cmds.Request) {

ctx := req.Context()

lk, err := daemon.Lock(ctx.ConfigRoot)

if err != nil {

res.SetError(fmt.Errorf("Couldn't obtain lock. Is another daemon already running?"), cmds.ErrNormal)

return

}

defer lk.Close()

node, err := core.NewIpfsNode(ctx.Config, true)

if err != nil {

res.SetError(err, cmds.ErrNormal)

return

}

ctx.Node = node
Yep, that's the reason I kept all the IPFS-specific stuff contained in the Context, but I'm not sure exactly how we should do it. Maybe I should just change it to a map[string]interface{}? Anyway, I'm not too worried about making the library vendorable right away since making it work with IPFS is higher priority.

Reply to this email directly or view it on GitHub:
https://github.com/jbenet/go-ipfs/pull/263/files#r19845198

btc · 2014-11-04T11:28:50Z

@mappum

These argument hints are quite helpful.

ipfs

    cmdIpfsInit.Flag.Int("b", 4096, "number of bits for keypair")
    cmdIpfsInit.Flag.String("p", "", "passphrase for encrypting keys")
    cmdIpfsInit.Flag.Bool("f", false, "force overwrite of existing config")
    cmdIpfsInit.Flag.String("d", "", "Change default datastore location")

ipfs2

        cmds.Option{[]string{"bits", "b"}, cmds.Int},
        cmds.Option{[]string{"passphrase", "p"}, cmds.String},
        cmds.Option{[]string{"force", "f"}, cmds.Bool},
        cmds.Option{[]string{"datastore", "d"}, cmds.String},

🐯 displaying argument hints in UX
🐯 providing argument hint in commands.Option

btc · 2014-11-04T12:13:11Z

cmd/ipfs2/main.go

+// log is the command logger
+var log = u.Logger("cmd/ipfs")
+
+func main() {


jbenet · 2014-11-05T18:39:11Z

@mappum @maybebtc how's it going over here?

mappum · 2014-11-05T22:59:09Z

btc · 2014-11-06T00:13:59Z

cmd/ipfs2/main.go

+func createRequest(args []string) (cmds.Request, *cmds.Command) {
+	req, root, cmd, err := cmdsCli.Parse(args, Root, commands.Root)
+	if err != nil {
+		fmt.Println(err)


@mappum Is the class of errors returned by Parse designed around user readability?

Yes, since its input is coming from the user.

@mappum sgtm

btc · 2014-11-06T00:24:47Z

core/commands2/ls.go

+		output := make([]Object, len(req.Arguments()))
+
+		for i, arg := range req.Arguments() {
+			path := arg.(string)


cast safely

jbenet · 2014-11-06T06:46:42Z

Other than generating the proper help text, a lot of the commands are done:

Awesome! I'll CR in a bit.

License: MIT Signed-off-by: Brian Tiger Chow <[email protected]>

(The file path is omitted, but since only one object can be added at a time, I think this is ok)

… argument or string argument)

…have any argument definitions

addresses #263 (comment)

jbenet · 2014-11-14T11:22:55Z

Closed in favor of #332

gx publish 4.4.24

jbenet mentioned this pull request Nov 4, 2014

Refactoring Commands #196

Closed

jbenet added the status/in-progress In progress label Nov 4, 2014

jbenet force-pushed the cmd-ref-part2 branch from 37b3c95 to 7855c13 Compare November 4, 2014 10:16

btc reviewed Nov 4, 2014
View reviewed changes

cmd/ipfs2/main.go

// log is the command logger

var log = u.Logger("cmd/ipfs")

func main() {

Copy link

Contributor

btc Nov 4, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

❤️

jbenet mentioned this pull request Nov 4, 2014

Make webui skeleton #226

Closed

btc referenced this pull request Nov 5, 2014

add in a default file hash and cleaned up init functiona bit

eaa68be

mappum force-pushed the cmd-ref-part2 branch from 7535d06 to 0d337de Compare November 5, 2014 08:23

btc reviewed Nov 6, 2014
View reviewed changes

core/commands2: Added 'config' command

d7a4915

btc reviewed Nov 6, 2014
View reviewed changes

core/commands2: Added 'bootstrap' command

1ebb3e1

btc mentioned this pull request Nov 6, 2014

add in a default file hash and cleaned up init function a bit #266

Merged

jbenet and others added 23 commits November 13, 2014 22:54

cmds2: use cmdDetails on level cmds

b5cb2a5

ignore binary

046c749

License: MIT Signed-off-by: Brian Tiger Chow <[email protected]>

fix(2/update) newlines

7529e66

License: MIT Signed-off-by: Brian Tiger Chow <[email protected]>

fix(2/update) insert line break

35c88f1

License: MIT Signed-off-by: Brian Tiger Chow <[email protected]>

object: Added a synopsis

dfb7649

object get: Fixed protobuf marshaling

585ba6c

object put: Formatted helptext

dcc4f81

object put: Fixed putting objects with JSON encoding

d674fb4

object put: Added plaintext marshaler

291eee8

object put: Made output (almost) match original output

6c66081

(The file path is omitted, but since only one object can be added at a time, I think this is ok)

object data: Moved some helptext into LongDescription

4e5c64c

object links: Added text marshaling

0aff1ed

cmds2: mount exmplae

1a5cd24

object links: Fixed description

38e2e45

commands/http: Fixed client erroring on nil command output

b9163cc

cmds2: change add arg name

5fb8df0

commands/cli: Take an optional Stdin value in Parse (read as a reader…

8ad347a

… argument or string argument)

main: Pass Stdin to CLI request parser

d252e97

commands/cli: Fixed bug when parsing args for a command that doesn't …

1cda153

…have any argument definitions

commands: Added a flag to enable stdin arguments

4cbca91

block put: Support stdin input on 'block put'

118b084

cmds2: check for updates

23242f9

cmds: nicer error on no mountpoint

981c5ab

btc pushed a commit that referenced this pull request Nov 14, 2014

pin/unpin -> add rm

3dcae0e

addresses #263 (comment)

jbenet mentioned this pull request Nov 14, 2014

Commands Refactor Part 2 - rebased #332

Merged

jbenet closed this Nov 14, 2014

jbenet removed the status/in-progress In progress label Nov 14, 2014

btc deleted the cmd-ref-part2 branch January 21, 2015 01:02

ariescodescream pushed a commit to ariescodescream/go-ipfs that referenced this pull request Oct 23, 2021

Merge pull request ipfs#263 from libp2p/gx/update

931ed22

gx publish 4.4.24

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Commands Refactor Part 2 #263

Commands Refactor Part 2 #263

jbenet commented Nov 4, 2014

jbenet commented Nov 4, 2014

jbenet commented Nov 4, 2014

btc commented Nov 4, 2014

jbenet commented Nov 4, 2014

jbenet commented Nov 4, 2014

btc commented Nov 4, 2014

jbenet commented Nov 4, 2014

btc Nov 4, 2014

mappum Nov 4, 2014

jbenet Nov 4, 2014

btc commented Nov 4, 2014

btc Nov 4, 2014

jbenet commented Nov 5, 2014

mappum commented Nov 5, 2014

btc Nov 6, 2014

mappum Nov 6, 2014

btc Nov 6, 2014

btc Nov 6, 2014

jbenet commented Nov 6, 2014

jbenet commented Nov 14, 2014

Commands Refactor Part 2 #263

Commands Refactor Part 2 #263

Conversation

jbenet commented Nov 4, 2014

jbenet commented Nov 4, 2014

warning

jbenet commented Nov 4, 2014

btc commented Nov 4, 2014

jbenet commented Nov 4, 2014

jbenet commented Nov 4, 2014

btc commented Nov 4, 2014

jbenet commented Nov 4, 2014

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

btc commented Nov 4, 2014

Choose a reason for hiding this comment

jbenet commented Nov 5, 2014

mappum commented Nov 5, 2014

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jbenet commented Nov 6, 2014

jbenet commented Nov 14, 2014