docker / app

Make your Docker Compose applications reusable, and share them on Docker Hub
Apache License 2.0
1.57k stars 177 forks source link

Fix: A single-file app can have CRLF and random YAML documents order #561

Closed silvin-lubecki closed 5 years ago

silvin-lubecki commented 5 years ago

- What I did

Fix CRLF detection on single-file app (https://github.com/docker/app/issues/553) Fix YAML documents order detection for single-file apps (https://github.com/docker/app/issues/249)

- How to verify it Add e2e tests:

- Description for the changelog

- A picture of a cute animal (not mandatory but encouraged) image

codecov[bot] commented 5 years ago

Codecov Report

Merging #561 into master will not change coverage. The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #561   +/-   ##
=======================================
  Coverage   71.23%   71.23%           
=======================================
  Files          55       55           
  Lines        3306     3306           
=======================================
  Hits         2355     2355           
  Misses        650      650           
  Partials      301      301

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update ea61c10...cfc5966. Read the comment docs.

thaJeztah commented 5 years ago

Did a quick check what the YAML spec says about newlines; https://yaml.org/spec/1.0/#id2565848

Inside all scalar nodes, a compliant YAML processor must translate the two-character combination CR LF, any CR that is not followed by an LF, and any NEL into a single LF (this does not apply to escaped characters). LS and PS characters are preserved. These rules are compatible with Unicode's newline guidelines.

silvin-lubecki commented 5 years ago

Yes but I used the 1.2 version of the specs https://yaml.org/spec/1.2/spec.html#id2774608 says CRLF is a valid line break:

YAML recognizes the following ASCII line break characters. [24] b-line-feed ::= #xA / LF */
[25] b-carriage-return ::= #xD /
CR */
[26] b-char ::= b-line-feed | b-carriage-return

silvin-lubecki commented 5 years ago

@jcsirot @rumpl PTAL

silvin-lubecki commented 5 years ago

PTAL @chris-crone