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

fixed broken serialization of duplicated entries in sequences #205

Merged
merged 1 commit into from Sep 9, 2015

Conversation

vogelsgesang
Copy link
Contributor

This pull request fixes a bug in the serialization of duplicated values within arrays. For example, with the following code

var obj = { test: 'canary' };
var array = [ 0, 1 ];
var arrayWithRefs = [ obj, obj, array, array ];

arrayWithRefs was serialized as

- &ref_0test: canary
- *ref0
- &ref_1- 0
  - 1
- *ref1

which is incorrect. Line breaks were missing between the reference names (ref_0, ref_1) and the consecutive characters.

In addition, this PR contains a regression test.

compact = false;
}


Copy link
Collaborator

Choose a reason for hiding this comment

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

Why extra empty line?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

A mistake. I will fix it. Just a moment...

For example, with the following code

```
var obj = { test: 'canary' };
var array = [ 0, 1 ];
var arrayWithRefs = [ obj, obj, array, array ];
```

`arrayWithRefs` was serialized as

```
- &ref_0test: canary
- *ref0
- &ref_1- 0
  - 1
- *ref1
```

which is incorrect.
@vogelsgesang
Copy link
Contributor Author

Rebased in order to remove the duplicated empty line I introduced.

@puzrin
Copy link
Member

puzrin commented Sep 9, 2015

please remove "make browserify" from PR.

@vogelsgesang
Copy link
Contributor Author

Ok, I removed the "make browserify" commit.

Would be perfect if you could browserify the library then, so that I have a commit to which I can point bower.

@puzrin
Copy link
Member

puzrin commented Sep 9, 2015

Browserification is done for every tagged release. I'll release as soon as @dervus confirm that PR is ok.

dervus added a commit that referenced this pull request Sep 9, 2015
fixed broken serialization of duplicated entries in sequences
@dervus dervus merged commit 3cf7be8 into nodeca:master Sep 9, 2015
@dervus
Copy link
Collaborator

dervus commented Sep 9, 2015

@vogelsgesang Thanks for contribution!

@vogelsgesang
Copy link
Contributor Author

Thanks for your quick response and for maintaining this library :)

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.

None yet

3 participants