Skip to content

Circular data & cross references #110#148

Merged
dervus merged 1 commit intonodeca:masterfrom
loamhoof:master
Oct 8, 2014
Merged

Circular data & cross references #110#148
dervus merged 1 commit intonodeca:masterfrom
loamhoof:master

Conversation

@loamhoof
Copy link
Copy Markdown

@loamhoof loamhoof commented Oct 7, 2014

Adding an option (checkDuplicates) to catch circular and cross
references, disabled by default so as not to slow down any existing code (only a few bits of code are still executed when the option is disabled).

Implemented following @puzrin suggestion that is: do a first pass to get the duplicates, and then put the aliases inside the algorithm. Slower than @larkox 's implementation, but perhaps easier to maintain.

Also added a related test.

NB: There may be some unused aliases if a duplicate is found inside a custom type, which must be a pretty rare case.

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 do you duplicate this code across all of the writers? Can it be placed in writeNode function?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

I'll study the possibility.

@dervus
Copy link
Copy Markdown
Collaborator

dervus commented Oct 8, 2014

In conclusion, please merge all these commits together.

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.

Oh, by the way, if you swap the operands here, it'll be great.

@loamhoof
Copy link
Copy Markdown
Author

loamhoof commented Oct 8, 2014

Sorry about the small remainders, should have seen them. Anyway, here you go.

@puzrin
Copy link
Copy Markdown
Member

puzrin commented Oct 8, 2014

https://github.com/nodeca/js-yaml/pull/148/commits

@loamhoof squash to single commit please. See 'git rebase --interactive' in google

Catch circular and cross references.
The object is first scanned to get these references before starting the writing algorithm. Aliases are then added during it.
@loamhoof
Copy link
Copy Markdown
Author

loamhoof commented Oct 8, 2014

@puzrin sorry about that, it's done.

dervus added a commit that referenced this pull request Oct 8, 2014
Circular data & cross references #110
@dervus dervus merged commit 2c23e7b into nodeca:master Oct 8, 2014
@dervus
Copy link
Copy Markdown
Collaborator

dervus commented Oct 8, 2014

Perfect! Thank you for contribution!

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