Skip to content

fixed broken serialization of duplicated entries in sequences#205

Merged
dervus merged 1 commit intonodeca:masterfrom
vogelsgesang:fix_duplicate_serialization
Sep 9, 2015
Merged

fixed broken serialization of duplicated entries in sequences#205
dervus merged 1 commit intonodeca:masterfrom
vogelsgesang:fix_duplicate_serialization

Conversation

@vogelsgesang
Copy link
Copy Markdown
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.

Comment thread lib/js-yaml/dumper.js Outdated
Copy link
Copy Markdown
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
Copy Markdown
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 vogelsgesang force-pushed the fix_duplicate_serialization branch from b83e4f6 to b157c24 Compare September 8, 2015 23:06
@vogelsgesang
Copy link
Copy Markdown
Contributor Author

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

@puzrin
Copy link
Copy Markdown
Member

puzrin commented Sep 9, 2015

please remove "make browserify" from PR.

@vogelsgesang vogelsgesang force-pushed the fix_duplicate_serialization branch from b157c24 to f25843d Compare September 9, 2015 07:33
@vogelsgesang
Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
Collaborator

dervus commented Sep 9, 2015

@vogelsgesang Thanks for contribution!

@vogelsgesang
Copy link
Copy Markdown
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.

3 participants