Skip to content

ls: Report real file size#5906

Merged
Stebalien merged 4 commits intomasterfrom
feat/ls-filesize
Jan 17, 2019
Merged

ls: Report real file size#5906
Stebalien merged 4 commits intomasterfrom
feat/ls-filesize

Conversation

@magik6k
Copy link
Copy Markdown
Member

@magik6k magik6k commented Jan 8, 2019

Need this for ipfs/go-ipfs-http-client#1 to implement fast api.Unixfs().Get().

@magik6k magik6k requested a review from Kubuxu as a code owner January 8, 2019 16:41
@ghost ghost assigned magik6k Jan 8, 2019
@ghost ghost added the status/in-progress In progress label Jan 8, 2019
@magik6k magik6k force-pushed the feat/ls-filesize branch 2 times, most recently from 329a35b to 34ab008 Compare January 8, 2019 16:42
@magik6k magik6k mentioned this pull request Jan 8, 2019
Merged
1 task
@Stebalien
Copy link
Copy Markdown
Member

Stebalien commented Jan 8, 2019 •
edited
Loading

So, what if we made this the default? That is:

  1. Replace --resolve-size with just --size.
  2. Default this to true.
  3. Always use the bytes size for files.
  4. Consider adding a --disk-usage (or similar) flag. Not sure if we really care about this as, IIRC, this will be going away with UnixFSv2.

(issue for discussion: ipfs-inactive/interface-js-ipfs-core#427)

@Stebalien Stebalien mentioned this pull request Jan 8, 2019
Closed
@magik6k magik6k changed the title ls: option to report real file size ls: Report real file size Jan 11, 2019
@magik6k magik6k force-pushed the feat/ls-filesize branch 3 times, most recently from 7e6b597 to c4d041a Compare January 11, 2019 16:14
Stebalien
Stebalien requested changes Jan 11, 2019
View reviewed changes
Copy link
Copy Markdown
Member

@Stebalien Stebalien left a comment

Choose a reason for hiding this comment

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

Mostly looks good but I'm concerned with backwards compatibility. I expect that using actual file sizes will reduce confusion and actually fix bugs however, we should probably print something for directory sizes to avoid breaking column-based parsing.

@magik6k magik6k mentioned this pull request Jan 12, 2019
Open
51 tasks
@magik6k magik6k added the need/review Needs a review label Jan 14, 2019
@magik6k magik6k requested a review from Stebalien January 15, 2019 12:12
@magik6k @Stebalien
ls: option to report real file size
6bb02a5 
License: MIT
Signed-off-by: Łukasz Magiera <magik6k@gmail.com>
@magik6k @Stebalien
ls: report real size by default
e63c06c 
License: MIT
Signed-off-by: Łukasz Magiera <magik6k@gmail.com>
@magik6k @Stebalien
ls: skip size for directories
39c2e01 
License: MIT
Signed-off-by: Łukasz Magiera <magik6k@gmail.com>
@magik6k @Stebalien
ls: dummy value in place of size for dirs
3b5c071 
License: MIT
Signed-off-by: Łukasz Magiera <magik6k@gmail.com>
@ghost ghost assigned Stebalien Jan 17, 2019
@Stebalien Stebalien merged commit ac26948 into master Jan 17, 2019
@ghost ghost removed the status/in-progress In progress label Jan 17, 2019
@ghost ghost deleted the feat/ls-filesize branch January 20, 2019 18:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Stebalien Stebalien Stebalien approved these changes

@Kubuxu Kubuxu Awaiting requested review from Kubuxu

Assignees

@Stebalien Stebalien

@magik6k magik6k

Labels

need/review Needs a review

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@magik6k @Stebalien @eingenito