-
Notifications
You must be signed in to change notification settings - Fork 45
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.
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
Display cids with associated filename #124
Display cids with associated filename #124
Conversation
This lists the cid first and the path next to it.
This allows you to run ipfs-car --list-full /path/to/my.car. It will list the files with their CIDs which can be really practical for people.
During packing, we will later implement the ability to be verbose so that it prints which file is being processed.
The verbose option allows to print which file is being processed when packing a car file.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for adding this, and all the fixes in docs
src/pack/stream.ts
Outdated
@@ -40,7 +40,8 @@ export async function packToStream ({ input, writable, blockstore: userBlockstor | |||
maxChildrenPerNode: maxChildrenPerNode || unixfsImporterOptionsDefault.maxChildrenPerNode, | |||
wrapWithDirectory: wrapWithDirectory === false ? false : unixfsImporterOptionsDefault.wrapWithDirectory, | |||
rawLeaves: rawLeaves == null ? unixfsImporterOptionsDefault.rawLeaves : rawLeaves | |||
}) | |||
}), | |||
(source: any) => printCarContent(source, verbose) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we call this printUnixFsContent instead? It is more what it is than car content
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also thinking this should not be part of the lib, but part of the CLI. So, what do you think on receiving an optional handler as parameter to packToStream
that would be an async iterator of verbose option?
Than CLI can actually provide the printUnixFsContent, and users can also provide any other function they care, being cautious that in the end it must yield what it received.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we call this printUnixFsContent instead? It is more what it is than car content
The function can totally be renamed, no problem.
Also thinking this should not be part of the lib, but part of the CLI. So, what do you think on receiving an optional handler as parameter to
packToStream
that would be an async iterator of verbose option?Than CLI can actually provide the printUnixFsContent, and users can also provide any other function they care, being cautious that in the end it must yield what it received.
I see what you mean and I understand the point, I'll look into making that change since it makes more sense as you state.
This allows to keep the cli and lib part on their own
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It comes late but here are the edits asked.
LMK if anything else needs to be done here
The function printing what's being added to the car file has been renamed as discussed in storacha#124. The function has been moved to the cli part instead of the lib so that it is only tied to the cli and no to the lib to print stuff.
src/pack/index.ts
Outdated
@@ -19,6 +19,7 @@ export interface PackProperties { | |||
maxChildrenPerNode?: number, | |||
wrapWithDirectory?: boolean, | |||
hasher?: MultihashHasher, | |||
customHandler?: (sources: AsyncGenerator<ImportResult, void, unknown>) => AsyncGenerator<any, void, unknown> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's call this customStreamSink instead to better illustrate what it does
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No problem I'll change that on Monday
This new name is more in line with what it does rather than the old one
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM thanks @Breigner01
In order for people to have a much easier time with associating CIDs with the filename, there is now the option
--list-full
that displays on the same line the CID and the file path.A verbose option is available with the packing function.
It allows to print which file is being processed during packing.