This repository has been archived by the owner on Mar 25, 2024. It is now read-only.
-
-
Notifications
You must be signed in to change notification settings - Fork 175
more string quoting #398
Open
thedavidmeister
wants to merge
7
commits into
dtolnay:master
Choose a base branch
from
thedavidmeister:2023-11-28-always-quote-strings
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
more string quoting #398
Changes from all commits
Commits
Show all changes
7 commits
Select commit
Hold shift + click to select a range
1967b05
always quote strings
thedavidmeister c4b9ef5
fix tests
thedavidmeister c3fb5bc
tests and fix for long hexadecimal quoting
thedavidmeister e923a53
handling for NO on serialization
thedavidmeister f0d9ff4
more tests
thedavidmeister 629dbd5
more test cases
thedavidmeister 64afa73
more test cases
thedavidmeister 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
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
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -195,7 +195,7 @@ fn test_map() { | |
thing.insert("y".to_owned(), 2); | ||
let yaml = indoc! {" | ||
x: 1 | ||
y: 2 | ||
'y': 2 | ||
"}; | ||
test_serde(&thing, yaml); | ||
} | ||
|
@@ -238,7 +238,7 @@ fn test_basic_struct() { | |
}; | ||
let yaml = indoc! {r#" | ||
x: -4 | ||
y: "hi\tquoted" | ||
'y': "hi\tquoted" | ||
z: true | ||
"#}; | ||
test_serde(&thing, yaml); | ||
|
@@ -316,6 +316,78 @@ fn test_strings_needing_quote() { | |
test_serde(&thing, yaml); | ||
} | ||
|
||
#[test] | ||
fn test_moar_strings_needing_quote() { | ||
#[derive(Serialize, Deserialize, PartialEq, Debug)] | ||
struct Struct { | ||
s: String, | ||
} | ||
|
||
for s in &[ | ||
// Short hex values. | ||
"0x0", | ||
"0x1", | ||
// Long hex values that don't fit in a u64 need to be quoted. | ||
"0xffaed20B7B67e498A3bEEf97386ec1849EFeE6Ac", | ||
// "empty" strings. | ||
"", | ||
" ", | ||
// The norway problem https://hitchdev.com/strictyaml/why/implicit-typing-removed/ | ||
"NO", | ||
"no", | ||
"No", | ||
"Yes", | ||
"YES", | ||
"yes", | ||
"True", | ||
"TRUE", | ||
"true", | ||
"False", | ||
"FALSE", | ||
"false", | ||
"y", | ||
"Y", | ||
"n", | ||
"N", | ||
"on", | ||
"On", | ||
"ON", | ||
"off", | ||
"Off", | ||
"OFF", | ||
"0", | ||
"1", | ||
"null", | ||
"Null", | ||
"NULL", | ||
"nil", | ||
"Nil", | ||
"NIL", | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. A test string for There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @fzgregor i added this There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. thanks |
||
// https://hitchdev.com/strictyaml/why/implicit-typing-removed/#string-or-float | ||
"9.3", | ||
// https://github.com/dtolnay/serde-yaml/pull/398#discussion_r1432944356 | ||
"2E234567", | ||
// https://yaml.org/spec/1.2.2/#1022-tag-resolution | ||
"0o7", | ||
"0x3A", | ||
"+12.3", | ||
"0.", | ||
"-0.0", | ||
"12e3", | ||
"-2E+05", | ||
"0", | ||
"-0", | ||
"3", | ||
"-19", | ||
] { | ||
let thing = Struct { | ||
s: s.to_string(), | ||
}; | ||
let yaml = format!("s: '{}'\n", s); | ||
test_serde(&thing, &yaml); | ||
} | ||
} | ||
|
||
#[test] | ||
fn test_nested_vec() { | ||
let thing = vec![vec![1, 2, 3], vec![4, 5, 6]]; | ||
|
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.
It would be great if this would also cover the
float
orstring
issue: https://hitchdev.com/strictyaml/why/implicit-typing-removed/#string-or-floatThere 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.
See also https://yaml.org/spec/1.2.2/#1022-tag-resolution
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.
@fzgregor tests added
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.
thanks