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

Change json::to_xxx(&Json) functions to be json::Json::to_xxx(&self) #8679

Merged
merged 1 commit into from
Aug 24, 2013

Conversation

brandonson
Copy link
Contributor

to_str, to_pretty_str, to_writer, and to_pretty_writer were at the top
level of extra::json, this moves them into an impl for Json to match
with what's been done for the rest of libextra and libstd. (or at least for vec and str)

Also meant changing some tests.

Closes #8676.

to_str, to_pretty_str, to_writer, and to_pretty_writer were at the top
level of extra::json, this moves them into an impl for Json to match
with what's been done for the rest of libextra and libstd.
@alexcrichton
Copy link
Member

While you're at it, would you mind trying to move away from io::Writer towards rt::io::Writer?

Otherwise this looks good to me, thanks!

@brandonson
Copy link
Contributor Author

I looked into it, but the new rt::io::extensions::WriterUtil doesn't seem to have a write_str method, nor does it appear to be possible to get a writer which to a string (which io::with_str_writer currently does). Which means I'd essentially have to write at least with_str_writer.

It looks like including something like write_str was discussed in #8089 but then left alone as undecided on whether to include it. I haven't seen anything regarding with_str_writer.

So I guess I need input on whether I should: A) wait for more additions to rt::io supporting strings, or B) write an equivalent to with_str_writer and include it in either json or somewhere in rt::io, then find some way to convert strings into a writable format.

Of course if I'm missing something in rt::io which supports strings, let me know. std::rt isn't included in the docs, unfortunately.

@alexcrichton
Copy link
Member

It's true that there aren't as many "nice methods", but I think that it's partially by design that way. You can get write_str with write(str.as_bytes()), and with_str_writer can be done with MemWriter and str::from_owned_bytes.

You can look at the example in the sprintf function inside of std/fmt/mod.rs of how to go from a byte-writer to a string. Ideally there'd be a helper function for doing so in rt::io, but we haven't fully finished moving towards that just yet.

@brson
Copy link
Contributor

brson commented Aug 23, 2013

Eventually there will be more string support in the new I/O library. Haven't gotten to that part yet.

@brandonson
Copy link
Contributor Author

So I dove into this today, got a fair bit done, then ran into #8579 and #6551. Using &-ptrs in the Encoder and Decoder forces lifetime constraints where they're used, and in extra::workcache providing those constraints causes ICEs. Those ICEs, in this case, can't be worked around without having unconstrained type errors.

Current status is on this branch: https://github.com/singingboyo/rust/tree/json-to-rt-io if anyone wants to take a look, but it seems to me like rt::io support isn't going to work at the moment unless we want to be using @-ptrs.

@alexcrichton
Copy link
Member

Oh well, thanks for investigating!

bors added a commit that referenced this pull request Aug 24, 2013
to_str, to_pretty_str, to_writer, and to_pretty_writer were at the top
level of extra::json, this moves them into an impl for Json to match
with what's been done for the rest of libextra and libstd.  (or at least for vec and str)

Also meant changing some tests.

Closes #8676.
@bors bors closed this Aug 24, 2013
@bors bors merged commit 3b9e2ef into rust-lang:master Aug 24, 2013
flip1995 pushed a commit to flip1995/rust that referenced this pull request Jun 30, 2023
…=Manishearth

Allow safety comment above attributes

Closes rust-lang#8679

changelog: Enhancement: [`undocumented_safety_block`]: Added `accept-comment-above-attributes` configuration.
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.

Change extra::json::to_xxx(&Json) methods to extra::json::Json::to_xxx(&self)
4 participants