Skip to content

Conversation

@lumpov
Copy link

@lumpov lumpov commented Jul 31, 2017

  1. fix field name length bug.
  2. fix bug with unicode characters, it needs to save as is from buffer, without .toString()

…needs to save as is from buffer, without .toString()
Copy link
Contributor

@sheindel sheindel left a comment

Choose a reason for hiding this comment

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

Because this change is a little more involved than other PRs, would you be willing to add some unit tests to demonstrate this working? I know these PRs have been stale for a while. so if not, it may just take me some time to get around to this. Thanks!

field_meta.forEach(function(f, i) {
// field name
f.name.split('').slice(0, 8).forEach(function(c, x) {
f.name.split('').slice(0, 9).forEach(function(c, x) {
Copy link
Contributor

Choose a reason for hiding this comment

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

This has actually been fixed in another PR now, and the value should actually be 10.

@sheindel
Copy link
Contributor

The field name bug is fixed and because of browsers not having support for Buffer directly without a shim, I think the TextEncoder version of this in #26 would probably be a better path moving forward. But if you have another opinion on this (And if you still care, I know this is an old PR), please feel free to re-open and/or comment as such!

@sheindel sheindel closed this Feb 10, 2023
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.

2 participants