Skip to content
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

Add example of json use in json.rs #10801

Merged
merged 3 commits into from
Jan 20, 2014
Merged

Add example of json use in json.rs #10801

merged 3 commits into from
Jan 20, 2014

Conversation

musitdev
Copy link
Contributor

@musitdev musitdev commented Dec 4, 2013

I update the example of json use to the last update of the json.rs code. I delete the old branch.
From my last request, I remove the example3 because it doesn't compile. I don't understand why and I don't have the time now to investigate.

@musitdev
Copy link
Contributor Author

musitdev commented Dec 4, 2013

Id add two function too
pub fn buffer_encode<T:Encodable<Encoder<'self>>>(to_encode_object: &T) -> ~[u8]
and
pub fn str_encode<T:Encodable<Encoder<'self>>>(to_encode_object: &T) -> ~str

@brson
Copy link
Contributor

brson commented Dec 4, 2013

Those are some beautiful docs! (^0^)/

@musitdev
Copy link
Contributor Author

musitdev commented Dec 5, 2013

I put ```rust isn't it right? Where are they missing? No problem to change but I thought it was ok.

@huonw
Copy link
Member

huonw commented Dec 5, 2013

@musitdev for inline code it is better if it is serialize::Encodable and #[deriving(Decodable, Encodable)], because then it will be rendered as serialize::Encodable rather than 'serialize::Encodable'.


Json data are encoded in a form of "key":"value".
Data type that can by encoded are javascript type :
bool (string true or false), double, string, array, object, null.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

boolean (`true` or `false`), number (`f64`), string, array, object, null.

@musitdev
Copy link
Contributor Author

any chance that this request will be closed?

@emberian
Copy link
Member

@musitdev you have yet to address @huonw's comment

@emberian
Copy link
Member

See also the doctest changes at #11120; code examples in documentation should be executable.

@musitdev
Copy link
Contributor Author

I've done the corrections but there are some code that is provided for the example and I don't put all the necessary use. I use the \rust,notest` tag to define the code. My problem is that the compiler still try to compile it and I have a lot of unresolved name error. Do I have to put the use which make the example less clear or is there any other tag.

@huonw
Copy link
Member

huonw commented Dec 27, 2013

Do I have to put the use which make the example less clear or is there any other tag.

You can prefix lines with a # to have those lines removed from the final output, but still passed to the test runner, e.g.:

# use extra::json::Json;

let j: Json = from_str("{}").unwrap();

Note that we will not want any code examples that aren't tested automatically (that is, no use of notest or equivalent at all), since they just bitrot and go out-of-date.

Also, we normally want the imports to be listed, to make the code examples self-contained (someone reading the docs for the first time won't necessarily know which functions to import and where to import them from), although if the imports are exactly the same in each one, it is possibly ok to only show the first one.

@musitdev
Copy link
Contributor Author

It seems that I have a problem with the # directive that is use for struct annotation and hide comment.
the code
use extra::serialize::Decodable; #[deriving(Decodable)] pub struct MyStruct { attr1: u8, attr2: ~str, }
generate an error : expected one of;,}but foundpub`

If I add a space before the # I have the error :
failed to find an implementation of trait extra::serialize::Decodable<extra::json::Decoder> for main::MyStruct

Have you any idea?

@huonw
Copy link
Member

huonw commented Dec 28, 2013

That appears to be a bug in rustdoc; I'm just testing a patch for it now, I'll open a PR soon.

@huonw
Copy link
Member

huonw commented Dec 29, 2013

(Opened #11185)

@musitdev
Copy link
Contributor Author

I've done a git fetch and git rebase and try make check and I still have the same problem. I push my change perhaps you can help me.

@huonw
Copy link
Member

huonw commented Dec 31, 2013

Oh, this is #4913. It should be fixed if you replace

#[deriving(...)]
struct Foo { ... }

bar();
baz();

with

#[deriving(...)]
struct Foo { ... }

fn main() {
   bar();
   baz();
}

(that is, give an explicit main, which stops rustdoc wrapping the entire example in its own main, which hits that issue).

@@ -15,6 +15,211 @@
#[allow(missing_doc)];

//! json parsing and serialization
/*!
#What is JSON?
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be good to draw the json parsing and serialization comment into the start of this one, i.e.:

/*!
JSON parsing and serialization

# What is JSON?

JSON (JavaScript Object Notation) is ...

(Also, notice I have a space after the #, all the headings should have this.)

}
```

##second example
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I personally don't like this example: I don't think we should necessarily be encouraging the use of ToJson, since serializing (using Encodable) directly to a string or a byte vector or any other writer will be much, much faster than going to JSON and then to a string.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I put the ToJson example to show another possibility of serailization. And I said I think for the special case when you develope your own serialization code it's simplier.

@huonw
Copy link
Member

huonw commented Jan 2, 2014

Since this is documentation, I've been fairly picky about grammar and phrasing and also about code style in the examples.

@musitdev
Copy link
Contributor Author

musitdev commented Jan 2, 2014

I understand your picky side for the docs. It's just you and I spend a lot of time to make this short toturial. At the beginning I thought it was simplier. Now I'am aware, I'll try to do it much better the next time. I'll update the code with your remarks.

@brson
Copy link
Contributor

brson commented Jan 2, 2014

Thanks for your patience @musitdev

@musitdev
Copy link
Contributor Author

musitdev commented Jan 2, 2014

The remark about ToJson still need to be settle.

@adrientetar
Copy link
Contributor

Once you are done, it would be nice to have your commits squashed into one, something like:
git reset --hard HEAD~9 && git merge --squash HEAD@{1} && git commit (9 being the number of commits you want to merge)

Its native compatibility with JavaScript and its simple syntax make it used widely.

Json data are encoded in a form of "key":"value".
Data types that can by encoded are JavaScript types :
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

s/by/be ?

@emberian
Copy link
Member

@musitdev can you squash these commits? make sure to comment when you're done: github doesn't notify when a PR is changed.

@musitdev
Copy link
Contributor Author

Done I squashed the last 10 commit. Tell me if it's ok.

@musitdev
Copy link
Contributor Author

There was a change in the master. I update the file to compile. Does I have to redo a squash?

@huonw
Copy link
Member

huonw commented Jan 15, 2014

Yes please. Also, it'd be good if the squashed commit message was just something like "extra::json: add documentation and examples", rather than the list of the individual commits that were squashed.

commit d00623d60afd460755b749ad5f94935f756f29d2
Author: musitdev <philippe.delrieu@free.fr>
Date:   Sat Jan 4 22:31:40 2014 +0100

    correct last comments.

commit ef09d6b6d1eebbd7c713c9dad96ed7bfc19dd884
Author: musitdev <philippe.delrieu@free.fr>
Date:   Thu Jan 2 20:28:53 2014 +0100

    update with the last remarks.

commit 46a028fe1fcdc2a7dcdd78a63001793eff614349
Author: musitdev <philippe.delrieu@free.fr>
Date:   Thu Jan 2 10:17:18 2014 +0100

    wrap example code in main function.

commit 2472901929bef09786b7aef8ca7c89fbe67d8e3e
Author: musitdev <philippe.delrieu@free.fr>
Date:   Mon Dec 30 19:32:46 2013 +0100

    Correct code to compile.

commit ed96b2223176781743e984af0e19abcb82150f1f
Author: musitdev <philippe.delrieu@free.fr>
Date:   Thu Dec 5 11:32:28 2013 +0100

    Correct the comment based on the PR comment.
    Change init call to new to reflect last change.

commit 38b0390c3533a16f822a6df5f90b907bd8ed6edc
Author: musitdev <philippe.delrieu@free.fr>
Date:   Wed Dec 4 22:34:25 2013 +0100

    correct from_utf8_owned call.

commit 08bed4c5f4fadf93ec457b605a1a1354323d2f5c
Author: musitdev <philippe.delrieu@free.fr>
Date:   Wed Dec 4 22:12:41 2013 +0100

    correct code '''

commit 02fddcbe2ab37fe842872691105bc4c5cff5abb5
Author: musitdev <philippe.delrieu@free.fr>
Date:   Wed Dec 4 13:25:54 2013 +0100

    correct typing error

commit b26830b8ddb49f551699e791832ed20640a0fafc
Author: musitdev <philippe.delrieu@free.fr>
Date:   Wed Dec 4 13:18:39 2013 +0100

    pass make check

commit e87c4f53286122efd0d2364ea45600d4fa4d5744
Author: musitdev <philippe.delrieu@free.fr>
Date:   Wed Dec 4 10:47:24 2013 +0100

    Add Json example and documentation.
@huonw
Copy link
Member

huonw commented Jan 19, 2014

The tests failed because MemWriter has moved to be just std::io::MemWriter rather than std::io::mem::MemWriter.

Comment when you've fixed it, for a new r+.

@musitdev
Copy link
Contributor Author

I see it too late. I'll do the change, commit and squash

bors added a commit that referenced this pull request Jan 19, 2014
I update the example of json use to the last update of the json.rs code. I delete the old branch.
From my last request, I remove the example3 because it doesn't compile. I don't understand why and I don't have the time now to investigate.
bors added a commit that referenced this pull request Jan 20, 2014
I update the example of json use to the last update of the json.rs code. I delete the old branch.
From my last request, I remove the example3 because it doesn't compile. I don't understand why and I don't have the time now to investigate.
@bors bors merged commit aeb5416 into rust-lang:master Jan 20, 2014
@emberian
Copy link
Member

Yay! Congratulations :)

On Sun, Jan 19, 2014 at 10:41 PM, bors notifications@github.com wrote:

Merged #10801 #10801.


Reply to this email directly or view it on GitHubhttps://github.com//pull/10801
.

@olsonjeffery
Copy link
Contributor

\o/ thanks so much for this @musitdev , this is great work

@musitdev
Copy link
Contributor Author

Thanks it was a pleasure. Everybody has done a great job to make things better.

@musitdev musitdev deleted the jsondoc2 branch January 21, 2014 06:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants