Rev yaml-test-suite submodule and fix Unix build #631
Conversation
am11
force-pushed
the
feature/rev-spec-suite-submodule
branch
from
6e51317
to
78a876c
Compare
am11
force-pushed
the
feature/rev-spec-suite-submodule
branch
from
78a876c
to
d8ea651
Compare
| @@ -39,71 +39,71 @@ private sealed class ParserSpecTestsData : SpecTestsData | |||
|
|
|||
| private static readonly List<string> ignoredSuites = new List<string> | |||
| { | |||
| // no spec test is ignored as of https://github.com/yaml/yaml-test-suite/releases/tag/data-2020-02-11 | |||
| "5T43", "JR7V", "NKF9", "CFD4", "U99R" | |||
There was a problem hiding this comment.
@aaubry, I have started working on fixing these new cases; got the first one fixed here am11@5943247.
Plain style scanning
From parsing architecture perspective, one significant issue is that we have FetchPlainScalar in Scanner class taking up a lot of responsibility and glues up a lot of mixed concerns (from all kinds of plain styles) in one place; which makes it harder to comprehend, if not impossible.
In particular, YAML 1.2 §7.3.3 talks about plain style of flow scalars, that is more restrictive than the $other plain styles of other scalars, and other styles of flow scalars. For the other two flow scalar styles (single and double quoted), we have a dedicated method FetchFlowScalar(bool isSingleQuoted), but for plain style the control flow either resorts to FetchValue, or (like in case of 5T43's fix) to FetchPlainScalar. IMHO, we should try to refactor flow scalar's plain style by changing its method's signature to FetchFlowScalar(bool isQuoted, bool isSingleQuoted) and implement its rules there. It might result in a little code duplication but much cleaner and implementation would be easily relatable with the spec, 1:1.
Similarly, for other kinds of plain styles, they could be structured in their corresponding parent concern and hence made relatable to the spec.
However, I believe priority 1 is correctness. Refactorings / optimizations can wait. I just wanted to record this message as the pain of finding the problem and my wounds therefore, are fresh at the moment. 8-)
Maybe someone has time to undergo this (very targetted) refactoring. My hands are full with stuff like real life, work and tracking down why JR7V and other are failing.. 🤣
|
I have updated the yaml tests cases to the latest available and fixed all broken tests. Thanks for your work. |
yaml-test-suitesubmodule to latest release