-
Notifications
You must be signed in to change notification settings - Fork 124
fix: files ls tests #282
base: master
Are you sure you want to change the base?
fix: files ls tests #282
Conversation
{ name: 'b', type: 0, size: 0, hash: '' }, | ||
{ name: 'lv1', type: 0, size: 0, hash: '' } | ||
{ name: 'b', type: 'file', size: 0, hash: '' }, | ||
{ name: 'lv1', type: 'file', size: 0, hash: '' } |
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.
type: 0, size: 0, hash: ''
sent by Go regardless - it's unfortunate that type
is a valid "type" for the item.
Basically you have to specify the long
option to ls
in order get these values otherwise you get type: 0, size: 0, hash: ''
independent of whether the thing is a file or directory.
You've added 'file' here for the item that is actually a directory and I'd argue that's even more confusing than 0
! 🤣
@hacdias - if you're still up for the challenge (I know this PR is old now) I think that we can usefully do is:
- Have these properties removed from the core MFS API when you do not specify the
long
option - Ensure they have the correct
type
value (string of "directory" or "file") when you do specifylong
- Keep HTTP API compatibility with Go
- still reply
type: 0, size: 0, hash: ''
if nolong
option is set - reply with
type
as a number in the response iflong
option IS set - have js-ipfs-api do the right thing and normalise the response, removing
type: 0, size: 0, hash: ''
or convertingtype
to the correct string
- still reply
BTW, it might be worth closing this PR and opening a new one as things have moved around in this repo significantly!
@achingbrain does that sound ok by you?
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.
I'm starting to think we should make the APIs do the right thing and submit a PR to go-ipfs
to make it do the right thing instead of trying to maintain compatibility with an API that is something of a footgun.
In this case long
should not even be an option since when you are listing the files internally you have all the information you need to fill those fields in the response. If the CLI wants to throw some of that information away it's essentially UI concern.
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.
So, @alanshaw @achingbrain how should I proceed then? I can remove the properties from the core MFS and ensure they have the correct value for type
, keeping the HTTP API compatibility. Or I can always return everything independently of the long
option.
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.
@alanshaw please advise :)
Related to ipfs-inactive/js-ipfs-http-client#776