-
-
Notifications
You must be signed in to change notification settings - Fork 494
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
Rev yaml-test-suite submodule and fix Unix build #631
Closed
Closed
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,15 @@ | ||
# Set default behavior to automatically normalize line endings. | ||
* text=auto | ||
|
||
# Force bash scripts to always use lf line endings so that if a repo is accessed | ||
# in Unix via a file share from Windows, the scripts will work. | ||
*.sh text eol=lf | ||
|
||
# Likewise, force cmd and batch scripts to always use crlf | ||
*.cmd text eol=crlf | ||
*.bat text eol=crlf | ||
|
||
*.cs text=auto diff=csharp | ||
|
||
*.csproj text=auto | ||
*.sln text=auto eol=crlf |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Submodule yaml-test-suite
updated
304 files
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@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 methodFetchFlowScalar(bool isSingleQuoted)
, but for plain style the control flow either resorts toFetchValue
, or (like in case of 5T43's fix) toFetchPlainScalar
. IMHO, we should try to refactor flow scalar's plain style by changing its method's signature toFetchFlowScalar(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.. 🤣