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

@jtennerjtenner 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?)

@jtennerjtenner added the enhancement New feature or request label Jul 18, 2019
@jtenner
jtenner requested a review from dcodeIOJuly 18, 2019 20:10
@jtennerjtenner self-assigned this Jul 18, 2019
Comment threadassembly/buffer/index.ts Outdated
Comment threadassembly/buffer/index.ts Outdated
Comment threadassembly/buffer/index.ts Outdated
@jtenner

jtenner commented Jul 19, 2019

Copy link
Copy Markdown
ContributorAuthor

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

functionbufferFrom(values: i32[]): Buffer{letlength=values.lengrh;letbuffer=Buffer.allocUnsafe(length);for(leti=0;i<length;i++)buffer[i]=u8(unchecked(values[i]));returnbuffer;}

This should be relatively safe for testing.

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

Copy link
Copy Markdown
ContributorAuthor

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

@jtenner

Copy link
Copy Markdown
ContributorAuthor

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

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

Labels

enhancementNew feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@jtenner@dcodeIO