-
Notifications
You must be signed in to change notification settings - Fork 716
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
Clarify the JSON data referenced in patterns sample is deserialized #6422
base: main
Are you sure you want to change the base?
Conversation
1. Renamed the variable json to data throughout the section. 2. Updated the text to clarify that the example works with deserialized data (e.g., parsed from JSON) rather than raw JSON strings. 3. Ensured consistency in variable naming and explanations. This update makes the documentation more accurate and avoids misleading readers into thinking that the variable holds a JSON string. If you’re contributing to the Dart documentation, you can submit this change as a pull request to the [site-www repository](https://github.com/dart-lang/site-www)
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
Hi @maintainers, could you please approve the workflows for this PR? Thank you! |
@dcharkes can you please review this PR? |
src/content/language/patterns.md
Outdated
if (data is Map<String, Object?> && | ||
data.length == 1 && | ||
data.containsKey('user')) { | ||
var user = data['user']; | ||
if (user is List<Object> && | ||
user.length == 2 && | ||
user[0] is String && | ||
user[1] is int) { | ||
var name = user[0] as String; | ||
var age = user[1] as int; | ||
print('User $name is $age years old.'); | ||
} |
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.
Looks like spacing was erroneously removed (here and in the next hunk).
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.
Can you kindly check it again
i fixed the spacing issue
thank you
src/content/language/patterns.md~
Outdated
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.
Looks like this file was not intended to be part of the commit.
Kindly check
Fixed the spacing issue :
7a12801
________________________________
From: Ilya Sher ***@***.***>
Sent: 17 February 2025 10:37
To: dart-lang/site-www ***@***.***>
Cc: MiniPiku ***@***.***>; Author ***@***.***>
Subject: Re: [dart-lang/site-www] Summary of Changes : (PR #6422)
@ilyash-b commented on this pull request.
________________________________
In src/content/language/patterns.md<#6422 (comment)>:
+if (data is Map<String, Object?> &&
+data.length == 1 &&
+data.containsKey('user')) {
+var user = data['user'];
+if (user is List<Object> &&
+user.length == 2 &&
+user[0] is String &&
+user[1] is int) {
+var name = user[0] as String;
+var age = user[1] as int;
+print('User $name is $age years old.');
+}
Looks like spacing was erroneously removed (here and in the next hunk).
—
Reply to this email directly, view it on GitHub<#6422 (review)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/A754NWW42VQOXGKILZWAUC32QFU7RAVCNFSM6AAAAABXGZINAWVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDMMRQGAZTKNZTHA>.
You are receiving this because you authored the thread.Message ID: ***@***.***>
|
Fixed the spacing issue
7a12801
________________________________
From: Ilya Sher ***@***.***>
Sent: 17 February 2025 10:37
To: dart-lang/site-www ***@***.***>
Cc: MiniPiku ***@***.***>; Author ***@***.***>
Subject: Re: [dart-lang/site-www] Summary of Changes : (PR #6422)
@ilyash-b commented on this pull request.
________________________________
In src/content/language/patterns.md<#6422 (comment)>:
+if (data is Map<String, Object?> &&
+data.length == 1 &&
+data.containsKey('user')) {
+var user = data['user'];
+if (user is List<Object> &&
+user.length == 2 &&
+user[0] is String &&
+user[1] is int) {
+var name = user[0] as String;
+var age = user[1] as int;
+print('User $name is $age years old.');
+}
Looks like spacing was erroneously removed (here and in the next hunk).
—
Reply to this email directly, view it on GitHub<#6422 (review)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/A754NWW42VQOXGKILZWAUC32QFU7RAVCNFSM6AAAAABXGZINAWVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDMMRQGAZTKNZTHA>.
You are receiving this because you authored the thread.
|
@ilyash-b |
@ilyash-b |
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 for thinking to clarify this @MiniPiku, it makes a lot of sense to do so.
In general these changes look good to me, but there is a failing test due to needing to update the source file as well. The snippets will need proper formatting as well.
Thanks again for the PR and let me know if you have any issues or questions. If you'd prefer, I'd be happy to make the necessary updates as well, just let me know :)
Also as a note, you don't need to ping/tag anyone to review your PRs. Generally different repositories have triage processes that will make sure your work gets reviewed. For us, it could take a few days or up to a week. We appreciate your patience :)
@@ -360,14 +360,14 @@ double calculateArea(Shape shape) => switch (shape) { | |||
### Validating incoming JSON | |||
|
|||
[Map][] and [list][] patterns work well for destructuring key-value pairs in | |||
JSON data: | |||
deserialized data, such as data parsed from JSON: |
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.
Nice clarification!
@@ -360,14 +360,14 @@ double calculateArea(Shape shape) => switch (shape) { | |||
### Validating incoming JSON | |||
|
|||
[Map][] and [list][] patterns work well for destructuring key-value pairs in | |||
JSON data: | |||
deserialized data, such as data parsed from JSON: | |||
|
|||
<?code-excerpt "language/lib/patterns/json.dart (json-1)"?> |
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.
Each of these code snippets are extracted from https://github.com/dart-lang/site-www/blob/main/examples/language/lib/patterns/json.dart (as specified by the <?code-excerpt
instructions).
Can you also update the source in that file to account for these changes?
If you followed the repository setup instructions, you can then use dart run dart_site refresh-excerpts
to update these snippets in the Markdown to match the source files. Or you can manually update them to match.
src/content/language/patterns.md
Outdated
var name = user[0] as String; | ||
var age = user[1] as int; | ||
print('User $name is $age years old.'); |
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.
Looks like some indentation was lost here:
var name = user[0] as String; | |
var age = user[1] as int; | |
print('User $name is $age years old.'); | |
var name = user[0] as String; | |
var age = user[1] as int; | |
print('User $name is $age years old.'); |
src/content/language/patterns.md~
Outdated
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 looks like this file was included on accident as a duplicate of the original. Did you intend to commit it?
This update makes the documentation more accurate and avoids misleading readers into thinking that the variable holds a JSON string.