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

Fix logic for document end before directive #174

Merged
merged 2 commits into from
Apr 19, 2020

Conversation

perlpunk
Copy link
Member

@perlpunk perlpunk commented Apr 15, 2020

See yaml/pyyaml#396

open_ended can have three states now:

0: The previous document was ended explicitly with '...'
1: The previous document wasn't ended with '...'
2: The last scalar event was a block scalar with trailing empty lines |+, and
       last document wasn't ended with '...'.
       Important at stream end.

This was broken in the past, and fixed in fa1293a.
With my last PR #162 I added the faulty behaviour again.

The problematic behaviour showed only when

  • Writing YAML directives and
  • writing unquoted top level scalars and
  • writing more than one document

See the commit message for the second commit for examples how to show the behaviour.
Check out the commit before the fix and running the first example.

Additionally this PR contains:

  • Allow writing %YAML 1.2 directive. Before it was valid to pass 1.2 but the output was hardcoded to 1.1
  • Add --directive (1.1|1.2) to ./tests/run-emitter-test-suite for easy debugging

@perlpunk perlpunk marked this pull request as ready for review April 17, 2020 20:20
@perlpunk perlpunk added this to Backlog in Release 0.2.4 Apr 17, 2020
@perlpunk perlpunk moved this from Backlog to In progress/Review in Release 0.2.4 Apr 17, 2020
@perlpunk perlpunk force-pushed the perlpunk/fix-open-ended branch 2 times, most recently from fbb4fec to 59b4bc8 Compare April 17, 2020 20:47
Before `1.1` was hardcoded in the emitter.

* Also add --directive option to run-emitter-test-suite
  Allows to easily test how output looks like if %YAML directives
  are output.
open_ended can have three states now:
0: The previous document was ended explicitly with '...'
1: The previous document wasn't ended with '...'
2: The last scalar event was a block scalar with trailing empty lines |+, and
   last document wasn't ended with '...'.
   Important at stream end.

This was broken in the past, and fixed in fa1293a.
With my last PR #162 I added the faulty behaviour again.

The problematic behaviour showed only when all of the following conditions were
true:
* writing YAML directives
* writing unquoted top level scalars
* writing more than one document

================== BROKEN ==============================

The first example shows that the document end marker is not emitted before
the next document. This would be valid in YAML 1.1 if the scalar was quoted,
but not if it is plain.

This commit fixes this.

echo '--- foo
--- bar
' | ./tests/run-parser-test-suite  | ./tests/run-emitter-test-suite  --directive 1.1

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

================== FIXED ==============================

echo '--- foo
--- bar
' | ./tests/run-parser-test-suite  | ./tests/run-emitter-test-suite  --directive 1.1
%YAML 1.1
--- foo
...
%YAML 1.1
--- bar

=======================================================

Other examples which should look like this (and were correct already before
this fix):

Open ended scalars like |+ need '...' at the end of the stream:

echo '--- |+
  a

--- |+
  a

' | ./tests/run-parser-test-suite  | ./tests/run-emitter-test-suite
--- |+
  a

--- |+
  a

...

=======================================================

If a document is ended with an explicit '...', the code should not
print '...' twice:

echo '--- foo
...
--- bar
' | ./tests/run-parser-test-suite  | ./tests/run-emitter-test-suite --directive 1.1
%YAML 1.1
--- foo
...
%YAML 1.1
--- bar

==========================================================
@perlpunk
Copy link
Member Author

Rebased and squashed first two commits.

@perlpunk perlpunk changed the base branch from master to release/0.2.4 April 19, 2020 10:45
@perlpunk perlpunk merged commit 3694a4a into release/0.2.4 Apr 19, 2020
@perlpunk perlpunk moved this from In progress/Review to Merged in Release 0.2.4 Apr 19, 2020
@perlpunk perlpunk deleted the perlpunk/fix-open-ended branch April 19, 2020 11:36
uqs pushed a commit to freebsd/freebsd-ports that referenced this pull request Apr 22, 2020
libyaml release changes:

  - yaml/libyaml#143
    Add packaging/docker-dist to Makefile.am

  - yaml/libyaml#174
    Fix logic for document end before directive

FreeBSD port improvement:

  - Enable make test target

  - Use correct license file

  - Switch download mirror to HTTPS

PR:	245813
Submitted by:	daniel.engberg.lists@pyret.net


git-svn-id: svn+ssh://svn.freebsd.org/ports/head@532485 35697150-7ecd-e111-bb59-0022644237b5
uqs pushed a commit to freebsd/freebsd-ports that referenced this pull request Apr 22, 2020
libyaml release changes:

  - yaml/libyaml#143
    Add packaging/docker-dist to Makefile.am

  - yaml/libyaml#174
    Fix logic for document end before directive

FreeBSD port improvement:

  - Enable make test target

  - Use correct license file

  - Switch download mirror to HTTPS

PR:	245813
Submitted by:	daniel.engberg.lists@pyret.net
Jehops pushed a commit to Jehops/freebsd-ports-legacy that referenced this pull request Apr 22, 2020
libyaml release changes:

  - yaml/libyaml#143
    Add packaging/docker-dist to Makefile.am

  - yaml/libyaml#174
    Fix logic for document end before directive

FreeBSD port improvement:

  - Enable make test target

  - Use correct license file

  - Switch download mirror to HTTPS

PR:	245813
Submitted by:	daniel.engberg.lists@pyret.net


git-svn-id: svn+ssh://svn.freebsd.org/ports/head@532485 35697150-7ecd-e111-bb59-0022644237b5
ericbsd pushed a commit to ghostbsd/ghostbsd-ports that referenced this pull request Apr 22, 2020
libyaml release changes:

  - yaml/libyaml#143
    Add packaging/docker-dist to Makefile.am

  - yaml/libyaml#174
    Fix logic for document end before directive

FreeBSD port improvement:

  - Enable make test target

  - Use correct license file

  - Switch download mirror to HTTPS

PR:	245813
Submitted by:	daniel.engberg.lists@pyret.net
bungle added a commit to Kong/kong that referenced this pull request May 22, 2020
### Summary

- yaml/libyaml#143
  Add packaging/docker-dist to Makefile.am

- yaml/libyaml#174
  Fix logic for document end before directive
bungle added a commit to Kong/kong that referenced this pull request May 22, 2020
### Summary

- yaml/libyaml#143
  Add packaging/docker-dist to Makefile.am

- yaml/libyaml#174
  Fix logic for document end before directive
svmhdvn pushed a commit to svmhdvn/freebsd-ports that referenced this pull request Jan 10, 2024
libyaml release changes:

  - yaml/libyaml#143
    Add packaging/docker-dist to Makefile.am

  - yaml/libyaml#174
    Fix logic for document end before directive

FreeBSD port improvement:

  - Enable make test target

  - Use correct license file

  - Switch download mirror to HTTPS

PR:	245813
Submitted by:	daniel.engberg.lists@pyret.net
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Release 0.2.4
  
Merged
Development

Successfully merging this pull request may close these issues.

None yet

2 participants