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.

Sign up for GitHub

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

Jump to bottom

beautify 'ipfs ls' and 'ipfs object links' #833

Merged
merged 4 commits into from
Mar 1, 2015
Merged

beautify 'ipfs ls' and 'ipfs object links' #833

merged 4 commits into from
Mar 1, 2015

Conversation

cryptix
Copy link
Contributor

@cryptix cryptix commented Feb 26, 2015

As discussed in #799, this PR pimps the output of ipfs ls and ipfs object links in text mode to use text/tabwriter which indents it nicely.

In ipfs ls the DAG service is used to lookup the Data of the node to find out it's datatype. If it is a directory, it sets the new Links IsDir field to true, which is then used by the text marshallers to append a / to the name.

I wasn't sure if we want this for object links as well. would be trivial to add.

before:
before

after:
after

@btc btc added the status/in-progress In progress label Feb 26, 2015
@whyrusleeping
Copy link
Member

@cryptix could you rebase on latest master?

@cryptix
Copy link
Contributor Author

cryptix commented Feb 27, 2015

@whyrusleeping done!

@whyrusleeping
Copy link
Member

I like all that green. While were here, and I notice command outputs changing, I realize that there are no sharness tests for ipfs ls. Do you think you could write a couple quick ones? It could probably just go in t0040-add-and-cat.sh. If youre not comfortable writing sharness tests just let me know and ill throw one together for you :)

@cryptix
Copy link
Contributor Author

cryptix commented Feb 28, 2015

I currently have some strange issue with running the shareness suite locally. I would like for you to take that part.

@whyrusleeping
Copy link
Member

@cryptix not a problem! Ill try and get to those today

@cryptix cryptix force-pushed the trailingDirSlash branch from 2226059 to b2034b4 Compare March 1, 2015 01:50
@cryptix
Copy link
Contributor Author

cryptix commented Mar 1, 2015

hm.. I just saw that shareness actually builds the other shareness deps from the Godeps. My last commit might not be an good idea if we want to do that.

@cryptix cryptix force-pushed the trailingDirSlash branch 2 times, most recently from 9d5d6f3 to 0ca206d Compare March 1, 2015 03:16
@jbenet
Copy link
Member

jbenet commented Mar 1, 2015

I just saw that shareness actually builds the other shareness deps from the Godeps. My last commit might not be an good idea if we want to do that.

Yeah, ideally we can keep all sharness deps building from what's in the repo. (make should "just work") Some (like sharness itself) are installed, if we can't redistribute the source with the same/compatible license.

@jbenet
Copy link
Member

jbenet commented Mar 1, 2015

Code LGTM.

Adding the / to unixfs directories brings ipfs ls closer under unixfs and away from dag. This is fine, just noting it because eventually we need to figure out what to do with the distinction.

@cryptix cryptix force-pushed the trailingDirSlash branch from f4e61de to 3134c9d Compare March 1, 2015 12:48
@cryptix cryptix force-pushed the trailingDirSlash branch from 3134c9d to 220aaf2 Compare March 1, 2015 13:04
@jbenet
Copy link
Member

jbenet commented Mar 1, 2015

LGTM. merging

jbenet added a commit that referenced this pull request Mar 1, 2015
@jbenet
Merge pull request #833 from jbenet/trailingDirSlash
5e85b00 
beautify 'ipfs ls' and 'ipfs object links'
@jbenet jbenet merged commit 5e85b00 into master Mar 1, 2015
@jbenet jbenet removed the status/in-progress In progress label Mar 1, 2015
@jbenet jbenet deleted the trailingDirSlash branch March 1, 2015 14:31
@whyrusleeping whyrusleeping mentioned this pull request Mar 1, 2015
Closed
@cryptix cryptix mentioned this pull request Mar 9, 2015
Closed
wking added a commit that referenced this pull request Jun 9, 2015
@wking
core/commands/ls: Remove trailing tabs
3b244d2 
Since 607468a (beautify 'ipfs ls' and 'ipfs object links', #833)
we've had these.  That pull request was about text/tabwriter [1]
elastic tabstops [2], but we don't need a column separator at the end
of each line.

[1]: https://golang.org/pkg/text/tabwriter/
[2]: http://nickgravgaard.com/elastic-tabstops/index.html
@wking wking mentioned this pull request Jun 9, 2015
Merged
wking added a commit that referenced this pull request Jun 9, 2015
@wking
core/commands/ls: Remove trailing tabs
97119da 
Since 607468a (beautify 'ipfs ls' and 'ipfs object links', #833)
we've had these.  That pull request was about text/tabwriter [1] and
elastic tabstops [2], but we don't need a column separator at the end
of each line.

[1]: https://golang.org/pkg/text/tabwriter/
[2]: http://nickgravgaard.com/elastic-tabstops/index.html
wking added a commit that referenced this pull request Jun 9, 2015
@wking
core/commands/ls: Remove trailing tabs
2ec0ba4 
Since 607468a (beautify 'ipfs ls' and 'ipfs object links', #833)
we've had these.  That pull request was about text/tabwriter [1] and
elastic tabstops [2], but we don't need a column separator at the end
of each line.

[1]: https://golang.org/pkg/text/tabwriter/
[2]: http://nickgravgaard.com/elastic-tabstops/index.html

Licence: MIT
Signed-off-by: W. Trevor King <[email protected]>
wking added a commit that referenced this pull request Jun 9, 2015
@wking
core/commands/ls: Remove trailing tabs
15135e1 
Since 607468a (beautify 'ipfs ls' and 'ipfs object links', #833)
we've had these.  That pull request was about text/tabwriter [1] and
elastic tabstops [2], but we don't need a column separator at the end
of each line.

[1]: https://golang.org/pkg/text/tabwriter/
[2]: http://nickgravgaard.com/elastic-tabstops/index.html

Licence: MIT
Signed-off-by: W. Trevor King <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@cryptix @whyrusleeping @jbenet @btc