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.

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

fix: files-from-path with common path prefix removed #27

Merged
merged 7 commits into from
Dec 13, 2022

Conversation

alanshaw
Copy link
Member

I think this is what we want.

This PR adds a new function filesFromPaths that returns a list of File objects with common path prefix removed. i.e. in this example test has been removed:

  1. Screenshot 2022-12-13 at 15 46 30
  2. Screenshot 2022-12-13 at 15 54 32
  3. Screenshot 2022-12-13 at 15 53 00

No commonality:

  1. Screenshot 2022-12-13 at 15 55 52

It means that you only ever need --no-wrap when there's 1 file. In the case of multiple files, they are either in different directories and so must be wrapped (1/2), or they are in the same directory, their common prefix is removed and they must be wrapped (3).

It's also consistent with the FileList you'd receive from a HTML <input type="file" multiple/>.

Added bonus is that this function returns File objects whose stream() method returns a web ReadableStream not a Node.js one, so no need to convert. It also removes globbing dependencies, util.promisify and and bunch of other functionality we're not using from files-from-path.

Copy link
Contributor

@vasco-santos vasco-santos left a comment

Choose a reason for hiding this comment

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

LGTM

Would be nice to at some point try to uniform or add this to the other lib. But we can move forward now

lib.js Outdated Show resolved Hide resolved
Co-authored-by: Oli Evans <oli@protocol.ai>
lib.js Outdated Show resolved Hide resolved
Copy link
Contributor

@olizilla olizilla left a comment

Choose a reason for hiding this comment

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

its cool i like it

@olizilla
Copy link
Contributor

(needs more tests, but can be later)

@alanshaw alanshaw merged commit c849d8e into main Dec 13, 2022
@alanshaw alanshaw deleted the fix/files-from-path branch December 13, 2022 23:02
alanshaw pushed a commit that referenced this pull request Dec 14, 2022
🤖 I have created a release *beep* *boop*
---


## 1.0.0 (2022-12-14)


### Features

* add `list` command
([#10](#10))
([b6cb1f0](b6cb1f0))
* add `proof add` command
([#24](#24))
([eb32d28](eb32d28))
* add `space` command with `create` and `register`
([#3](#3))
([9c25a2d](9c25a2d))
* add `w3 can store add` and `w3 can upload add` commands
([#26](#26))
([07fa1b0](07fa1b0))
* add `whoami` to print agent DID
([#11](#11))
([e3f2497](e3f2497))
* add delegation create command
([#5](#5))
([272c53a](272c53a))
* add old CLI bin reference
([#4](#4))
([9a0716c](9a0716c))
* delegation ls and proof ls commands
([#22](#22))
([04a7d31](04a7d31))
* sade cli skeleton
([#1](#1))
([3104b9f](3104b9f))
* up command ([#7](#7))
([283b938](283b938))


### Bug Fixes

* add expiration parameter
([#21](#21))
([9457841](9457841))
* better error reporting
([#19](#19))
([0f6a2a6](0f6a2a6)),
closes [#15](#15)
[#16](#16)
* create space on register
([#13](#13))
([f4a1a0f](f4a1a0f))
* dont emit warnings for fetch api
([#9](#9))
([cf52922](cf52922))
* files-from-path with common path prefix removed
([#27](#27))
([c849d8e](c849d8e))
* no build
([72b0bfb](72b0bfb))
* no-wrap parameter
([#25](#25))
([38aa353](38aa353)),
closes [#17](#17)
* space create and register improvements
([#8](#8))
([8617b49](8617b49))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
alanshaw pushed a commit to storacha/files-from-path that referenced this pull request Mar 13, 2023
This PR propagates the change here storacha/w3cli#27 so we can start using this module in `w3cli` as well as in the new `ipfs-car`.

* Switches to web streams by default, which is what the new web3.storage libraries expect to be passed and use internally
* Strips common prefix from paths automatically for reasons given in storacha/w3cli#27

BREAKING CHANGE: API, behavior and return types have changed. The module now only exports one function `filesFromPaths` which returns an array of file-like objects. The stream returned by `stream()` in these objects is now a web stream ([`ReadableStream`](https://developer.mozilla.org/en-US/docs/Web/API/ReadableStream)). File names now have their common prefix stripped automatically. Finally, the minimum Node.js version has increased from 14 to 18 (the current LTS) because of we use `Readable.toWeb()`.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants