Skip to content

[Implement] Buffer.concat - #5

Open
jtenner wants to merge 10 commits into
mainfrom
concat
Open

[Implement] Buffer.concat#5
jtenner wants to merge 10 commits into
mainfrom
concat

Conversation

@jtenner

@jtenner jtenner commented Jul 18, 2019

Copy link
Copy Markdown
Contributor

Current problems:

expect<Buffer>(actual).toStrictEqual(expected); // should compare lengths and values
  • No good way to create a buffer from scratch without a naive Buffer.from<T>() function
    • Could create a helper function, but that's a todo

This pull request is not blocked by anything. Current questions:

  • Should we set the length parameter to i32.MAX_VALUE?
  • Any suggestions for optimizations?
  • Should we do a null check on the array? (in case of uninitialized field problems?)
@jtenner jtenner added the enhancement New feature or request label Jul 18, 2019
@jtenner
jtenner requested a review from dcodeIO July 18, 2019 20:10
@jtenner jtenner self-assigned this Jul 18, 2019
Comment thread assembly/buffer/index.ts Outdated
Comment thread assembly/buffer/index.ts Outdated
Comment thread assembly/buffer/index.ts Outdated
@jtenner

jtenner commented Jul 19, 2019

Copy link
Copy Markdown
Contributor Author

I think I'm going to implement a test function for utility purposes.

function bufferFrom(values: i32[]): Buffer {
  let length = values.lengrh;
  let buffer = Buffer.allocUnsafe(length);
  for (let i = 0; i < length; i++) buffer[i] = u8(unchecked(values[i]));
  return buffer;
} 

This should be relatively safe for testing.

@jtenner jtenner closed this Jul 19, 2019
@jtenner jtenner reopened this Jul 19, 2019
@jtenner jtenner changed the title [Implement] Naive Buffer.concat<T> Jul 19, 2019
@jtenner

jtenner commented Jul 31, 2019

Copy link
Copy Markdown
Contributor Author

Do you think this can be merged? It looks good to me.

@jtenner

jtenner commented Mar 12, 2020

Copy link
Copy Markdown
Contributor Author

Six months in the making! Time to revive this pull request.

@jtenner
jtenner requested review from MaxGraey and dcodeIO March 12, 2020 20:35
@jtenner jtenner changed the title [Implement] Naive Buffer.concat Mar 12, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

2 participants