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

feat: add getUintBE, findSequence, and encoding for uint8ArrayToString #12

Merged

Conversation

bjornstar
Copy link
Contributor

There is some functionality from Buffer that was being used by file-type that seems to be generally useful, specifically getUintBE and the behavior of indexOf to take a buffer instead of a single byte's worth of an integer.

It would be great to get this in and released so it can be used in sindresorhus/file-type#633

@bjornstar
Copy link
Contributor Author

bjornstar commented Jun 28, 2024

I believe this one is ready to go and once we get a release we can move forward with:

In that order.

index.d.ts Outdated Show resolved Hide resolved
index.d.ts Outdated Show resolved Hide resolved
index.d.ts Outdated Show resolved Hide resolved
index.d.ts Outdated Show resolved Hide resolved
index.d.ts Outdated Show resolved Hide resolved
index.d.ts Outdated Show resolved Hide resolved
index.js Outdated Show resolved Hide resolved
@sindresorhus
Copy link
Owner

You need to update the readme too.

@bjornstar bjornstar requested a review from sindresorhus June 29, 2024 02:36
@Borewit
Copy link

Borewit commented Jun 29, 2024

I believe this one is ready to go and once we get a release we can move forward with:

In that order.

Explained in more detail here, I propose this steps:

  1. token-types
  2. strtok3
  3. music-metadata: Ensure compatibility of strtok3 & token-types, ensure extensive unit tests are working
  4. file-type
  5. music-metadata: Replace Buffer dependency, release music-metadata

@bjornstar
Copy link
Contributor Author

I believe this one is ready to go and once we get a release we can move forward with:

In that order.

Explained in more detail here, I propose this steps:

  1. token-types
  2. strtok3
  3. music-metadata: Ensure compatibility of strtok3 & token-types, ensure extensive unit tests are working
  4. file-type
  5. music-metadata: Replace Buffer dependency, release music-metadata

Since music-metadata has file-type as a dependency I put it before music-metadata. I suppose it's not required to be in that order, but it should work either way.

@sindresorhus sindresorhus merged commit 64a362b into sindresorhus:main Jun 29, 2024
2 checks passed
@bjornstar bjornstar deleted the feat/add-get-uint-be-find-sequence branch June 29, 2024 12:30
@sindresorhus
Copy link
Owner

@bjornstar I noticed two issues in the indexOf implementation after merging. Can you review the fix? 79401bc

  1. The inner loop resets index2 to 0 before breaking, which looks incorrect.
  2. The outer loop's termination condition should use <= to include the last valid offset.

@bjornstar
Copy link
Contributor Author

@bjornstar I noticed two issues in the indexOf implementation after merging. Can you review the fix? 79401bc

  1. The inner loop resets index2 to 0 before breaking, which looks incorrect.
  2. The outer loop's termination condition should use <= to include the last valid offset.
  1. Sorry about that, the reset to 0 was leftover from a change that I did to satisfy a linting rule that I ended up not using.
  2. Absolutely correct, good catch!

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