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 chomp indicator emitting logic #4

Merged
merged 1 commit into from
Oct 17, 2012
Merged

Conversation

roji
Copy link
Contributor

@roji roji commented Oct 17, 2012

Unit test RoundtripExample17 was failing because the correct chomp indicator wasn't being emitted

Hi Antoine, I think I found the bug causing the failing unit test. Please check what I've done since I don't know the YAML specs that well and especially not your source code.

The logic I implemented was simple:

  • If a scalar does not end with a newline, a strip indicator "-" must be emitted
  • If a scalar ends with two newlines, a keep indicator "+" must be emitted
  • Otherwise, no indicator should be emitted (one newline).

The relevant section is http://www.yaml.org/spec/1.2/spec.html#id2794534.

The was a boolean isOpenEnded which I know nothing about, I left the previous behavior of setting it to true iff the chomp indicator is keep "+".

Let me know what you think...

Unit test RoundtripExample17 was failing because the correct chomp indicator wasn't being emitted
@aaubry
Copy link
Owner

aaubry commented Oct 17, 2012

I checked the output and all seems well. I don't know either what isOpenEnded means. Most of the code from the parser and the emitter was converted from c++ (libyaml) a long time ago and I no longer remember the details. I would like to get the latest version of libyaml and integrate changes and fixes that were made since I performed the conversion, but I lost the source code of the version that I converted, which makes it harder to compare with the current version.

aaubry added a commit that referenced this pull request Oct 17, 2012
Fixed chomp indicator emitting logic
@aaubry aaubry merged commit 696edcd into aaubry:master Oct 17, 2012
@roji
Copy link
Contributor Author

roji commented Oct 18, 2012

Great.

Is this http://pyyaml.org/wiki/LibYAML the libyaml you ported? Maybe we
can compare your first version date with their revision
loghttp://pyyaml.org/log/and figure out which version you took? I'd
be happy to help out a little
bit...

On Wed, Oct 17, 2012 at 10:47 PM, Antoine Aubry notifications@github.comwrote:

I checked the output and all seems well. I don't know either what
isOpenEnded means. Most of the code from the parser and the emitter was
converted from c++ (libyaml) a long time ago and I no longer remember the
details. I would like to get the latest version of libyaml and integrate
changes and fixes that were made since I performed the conversion, but I
lost the source code of the version that I converted, which makes it harder
to compare with the current version.


Reply to this email directly or view it on GitHubhttps://github.com//pull/4#issuecomment-9543173.

@aaubry
Copy link
Owner

aaubry commented Oct 22, 2012

That's the one. I looked for the date of the initial commit of YamlDotNet, which is 31 July 2008. Since that date, there have been no commits on the LibYAML trunk, so I assume that there have been no new developments.

@roji
Copy link
Contributor Author

roji commented Oct 23, 2012

Hmm... Looking here: http://pyyaml.org/log/libyaml?rev=385, it seems like they've had some updates since 2008, it definitely might worth to take a look at their bugfixes...

@aaubry
Copy link
Owner

aaubry commented Oct 24, 2012

Humm, I think that's the log for PyYAML, the library for Python. I'm not
sure that it would be very useful.

On Tue, Oct 23, 2012 at 2:00 AM, Shay Rojansky notifications@github.comwrote:

Hmm... Looking here: http://pyyaml.org/log/, it seems like they've had
some updates since 2008, it definitely might worth to take a look at their
bugfixes...


Reply to this email directly or view it on GitHubhttps://github.com//pull/4#issuecomment-9686508.

@roji
Copy link
Contributor Author

roji commented Oct 26, 2012

It's weird, but the source for the C++ libyaml appears to be hosted on pyyaml.org along with the Python implementation (http://pyyaml.org/browser). the Python source code is under pyyaml, while the C++ code is under libyaml, and was changed 17 months ago, might be worth a look...

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

2 participants