Revert removing of open_ended after top level plain scalar by perlpunk · Pull Request #122 · yaml/libyaml

perlpunk · 2018-06-29T19:51:25Z

See also issue #60

This reverts commit 56400d9.

Currently tests will fail because we have the yaml-test-suite with very strict
tests that check for explicit document start/end markers.

So the following tests need to be skipped for now: 27NA 35KP 4V8U 9KAX P76L

The problematic case was that it created the following YAML:

--- foo
%YAML 1.1
--- bar

But you would need ... after `foo' here. Only when quoted you can leave out the end marker:

--- "foo"
%YAML 1.1
--- bar

In YAML 1.2 you always have to add the end marker, so I think it's a good thing.

But I know that the end marker also gets added to cases where it is not needed. Maybe I can have a look into that.

See also issue #60 Revert "This code is not needed and breaks tests" This reverts commit 56400d9.

perlpunk · 2018-06-29T19:54:44Z

@ingydotnet Could you add 27NA 35KP 4V8U 9KAX P76L to the emitter test skip list? (Or remove them from the test whitelist)
And when doing that, maybe you can also change the test-suite to fetch the yaml-test-suite data release instead of a commit id?

LocutusOfBorg · 2018-06-29T20:11:24Z

can you please also apply the second part of the patch on #60?

perlpunk · 2018-06-29T20:15:18Z

@LocutusOfBorg yeah, I was looking at it. Funnily I read this issue only after I searched for the cause of the test failures myself =)

I have to first run a whole lotta tests on it, with pyyaml, with perl's YAML::LibYAML, and yaml-test-suite, to make sure it isn't breaking something else.
Not sure how far I can get today...

perlpunk · 2018-06-29T21:25:59Z

I added the patch, thanks @jrtc27

@ingydotnet the tests are passing again, no need to skip them.
Also YAML::LibYAML tests are all passing again.
pyyaml tests are also passing, although they were already passing for me without the patch from @jrtc27

As a side note:
I can actually think of cases where I would want to add the trailing ..., but these have different conditions.
For example if you have this stream:

foo: |+
  a
  b

...

Then the document-end marker should be emitted, even if not requested, because the exact number of trailing lines needs to be preserved.
I should create a new issue for that.

jrtc27 · 2018-06-29T21:29:48Z

You missed the 7 on my username for the second commit :)

perlpunk · 2018-06-29T21:31:11Z

@jrtc27 oops, will fix it ;-)
I'll let appveyor finish its tests first though

@jrtc27

(only when explicitly requested) @jrtc27++ for the patch. See #60

perlpunk · 2018-06-29T22:17:12Z

fixed typo and force pushed

ingydotnet · 2018-06-30T22:20:03Z

Applied. Pushed. Thanks!

Changes ======= * yaml/libyaml#95 -- build: do not install config.h * yaml/libyaml#97 -- appveyor.yml: fix Release build * yaml/libyaml#103 -- Remove unused code in yaml_document_delete * yaml/libyaml#104 -- Allow colons in plain scalars inside flow collections * yaml/libyaml#109 -- Fix comparison in tests/run-emitter.c * yaml/libyaml#117 -- Fix typo error * yaml/libyaml#119 -- The closing single quote needs to be indented... * yaml/libyaml#121 -- fix token name typos in comments * yaml/libyaml#122 -- Revert removing of open_ended after top level plain scalar * yaml/libyaml#125 -- Cherry-picks from PR 27 * yaml/libyaml#135 -- Windows/C89 compatibility * yaml/libyaml#136 -- allow override of Windows static lib name

Fix portlint errors in Makefile Changes in 0.2.2: * yaml/libyaml#95 -- build: do not install config.h * yaml/libyaml#97 -- appveyor.yml: fix Release build * yaml/libyaml#103 -- Remove unused code in yaml_document_delete * yaml/libyaml#104 -- Allow colons in plain scalars inside flow collections * yaml/libyaml#109 -- Fix comparison in tests/run-emitter.c * yaml/libyaml#117 -- Fix typo error * yaml/libyaml#119 -- The closing single quote needs to be indented... * yaml/libyaml#121 -- fix token name typos in comments * yaml/libyaml#122 -- Revert removing of open_ended after top level plain scalar * yaml/libyaml#125 -- Cherry-picks from PR 27 * yaml/libyaml#135 -- Windows/C89 compatibility * yaml/libyaml#136 -- allow override of Windows static lib name git-svn-id: svn+ssh://svn.freebsd.org/ports/head@498674 35697150-7ecd-e111-bb59-0022644237b5

Fix portlint errors in Makefile Changes in 0.2.2: * yaml/libyaml#95 -- build: do not install config.h * yaml/libyaml#97 -- appveyor.yml: fix Release build * yaml/libyaml#103 -- Remove unused code in yaml_document_delete * yaml/libyaml#104 -- Allow colons in plain scalars inside flow collections * yaml/libyaml#109 -- Fix comparison in tests/run-emitter.c * yaml/libyaml#117 -- Fix typo error * yaml/libyaml#119 -- The closing single quote needs to be indented... * yaml/libyaml#121 -- fix token name typos in comments * yaml/libyaml#122 -- Revert removing of open_ended after top level plain scalar * yaml/libyaml#125 -- Cherry-picks from PR 27 * yaml/libyaml#135 -- Windows/C89 compatibility * yaml/libyaml#136 -- allow override of Windows static lib name

Fix portlint errors in Makefile Changes in 0.2.2: * yaml/libyaml#95 -- build: do not install config.h * yaml/libyaml#97 -- appveyor.yml: fix Release build * yaml/libyaml#103 -- Remove unused code in yaml_document_delete * yaml/libyaml#104 -- Allow colons in plain scalars inside flow collections * yaml/libyaml#109 -- Fix comparison in tests/run-emitter.c * yaml/libyaml#117 -- Fix typo error * yaml/libyaml#119 -- The closing single quote needs to be indented... * yaml/libyaml#121 -- fix token name typos in comments * yaml/libyaml#122 -- Revert removing of open_ended after top level plain scalar * yaml/libyaml#125 -- Cherry-picks from PR 27 * yaml/libyaml#135 -- Windows/C89 compatibility * yaml/libyaml#136 -- allow override of Windows static lib name git-svn-id: svn+ssh://svn.freebsd.org/ports/head@498674 35697150-7ecd-e111-bb59-0022644237b5

Changes ======= * yaml/libyaml#95 -- build: do not install config.h * yaml/libyaml#97 -- appveyor.yml: fix Release build * yaml/libyaml#103 -- Remove unused code in yaml_document_delete * yaml/libyaml#104 -- Allow colons in plain scalars inside flow collections * yaml/libyaml#109 -- Fix comparison in tests/run-emitter.c * yaml/libyaml#117 -- Fix typo error * yaml/libyaml#119 -- The closing single quote needs to be indented... * yaml/libyaml#121 -- fix token name typos in comments * yaml/libyaml#122 -- Revert removing of open_ended after top level plain scalar * yaml/libyaml#125 -- Cherry-picks from PR 27 * yaml/libyaml#135 -- Windows/C89 compatibility * yaml/libyaml#136 -- allow override of Windows static lib name

In YAML 1.1, the document end marker `...` is optional even if the next document starts with a directive: https://github.com/yaml/pyyaml/blob/master/tests/data/spec-07-09.canonical ``` %YAML 1.1 --- !!str "foo" %YAML 1.1 --- !!str "bar" %YAML 1.1 --- !!str "baz" ``` It is only required if the scalar is "open ended", for example for plain scalars. In YAML 1.2 the `...` marker is always required before a directive. My suggestion would be to make the output 1.2 compatible. It will still be 1.1 compatible, so that shouldn't be a problem. I believe this will also make it easier to fix #123 which was introduced with the last fixes regarding `open_ended`. I think I can make a fix for this soon after this issue is fixed. Fixing #123 without this would be a bit more complicated. If we do this, we also need to adjust PyYAML to behave the same. Related issues/commits: - #60 - #122 - 56400d9, 8ee83c0, 56f4b17

Revert removing of open_ended after top level plain scalar

8ee83c0

See also issue #60 Revert "This code is not needed and breaks tests" This reverts commit 56400d9.

perlpunk mentioned this pull request Jun 29, 2018

testsuite failure on i386 after applying test fix from master #60

Closed

Don't emit document-end marker at the end of stream

56f4b17

(only when explicitly requested) @jrtc27++ for the patch. See #60

perlpunk force-pushed the revert-open-ended branch from 8e33199 to 56f4b17 Compare June 29, 2018 22:16

This was referenced Jun 30, 2018

Build libyaml on travis yaml/pyyaml#195

Closed

Fails spec-07-09, spec-07-10, spec-07-13, spec-08-08 when built with LibYAML 0.2.1 yaml/pyyaml#184

Closed

ingydotnet merged commit 56f4b17 into master Jun 30, 2018

perlpunk mentioned this pull request Jul 17, 2018

test_emitter.test_emitter_styles failing when using C extensions on python2 only yaml/pyyaml#206

Open

dmacks mentioned this pull request Sep 15, 2018

libyaml &yaml-py: update to newer upstream version fink/fink-distributions#241

Merged

perlpunk deleted the revert-open-ended branch March 11, 2019 23:13

perlpunk mentioned this pull request Dec 26, 2019

Always output document end before directive (YAML 1.2 compatibility) #160

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Revert removing of open_ended after top level plain scalar#122

Revert removing of open_ended after top level plain scalar#122
ingydotnet merged 2 commits intomasterfrom
revert-open-ended

perlpunk commented Jun 29, 2018 •

edited

Loading

Uh oh!

perlpunk commented Jun 29, 2018

Uh oh!

LocutusOfBorg commented Jun 29, 2018

Uh oh!

perlpunk commented Jun 29, 2018

Uh oh!

perlpunk commented Jun 29, 2018 •

edited

Loading

Uh oh!

jrtc27 commented Jun 29, 2018

Uh oh!

perlpunk commented Jun 29, 2018

Uh oh!

perlpunk commented Jun 29, 2018

Uh oh!

ingydotnet commented Jun 30, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

perlpunk commented Jun 29, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

perlpunk commented Jun 29, 2018

Uh oh!

LocutusOfBorg commented Jun 29, 2018

Uh oh!

perlpunk commented Jun 29, 2018

Uh oh!

perlpunk commented Jun 29, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jrtc27 commented Jun 29, 2018

Uh oh!

perlpunk commented Jun 29, 2018

Uh oh!

perlpunk commented Jun 29, 2018

Uh oh!

ingydotnet commented Jun 30, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

perlpunk commented Jun 29, 2018 •

edited

Loading

perlpunk commented Jun 29, 2018 •

edited

Loading