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 print macros #7

Closed
wants to merge 8 commits into from
Closed

add print macros #7

wants to merge 8 commits into from

Conversation

yoshuawuyts
Copy link
Collaborator

Adds the print macros from async-std here so we can re-export them from submodules. Thanks!

Signed-off-by: Yoshua Wuyts <[email protected]>
src/print.rs Outdated Show resolved Hide resolved
Signed-off-by: Yoshua Wuyts <[email protected]>
Signed-off-by: Yoshua Wuyts <[email protected]>
@taiki-e
Copy link
Contributor

taiki-e commented Oct 14, 2019

@yoshuawuyts Could you add tests? As the document uses async-std's macro, it does not work as this crate's test.

Signed-off-by: Yoshua Wuyts <[email protected]>
@yoshuawuyts
Copy link
Collaborator Author

@taiki-e I'm not sure how to go about the testing stdout output here. We could do some process spawning + capture, but it doesn't feel great. Do you have any suggestions?

@yoshuawuyts
Copy link
Collaborator Author

Or well, we could check if things compile which may be good enough. I'll add those tests (:

@yoshuawuyts
Copy link
Collaborator Author

I tried adding:

use std::error::Error;

#[test]
fn print_works() {
    futures::executor::block_on(async {
        async_macros::print!("hello {}", "world").await;
        async_macros::println!("hello {}", "world").await;
        async_macros::eprint!("hello {}", "world").await;
        async_macros::eprintln!("hello {}", "world").await;
    });
}

But it fails because write_fmt isn't part of an async-std release yet. I don't think we can add tests right now, and should instead test this within async-std.

Signed-off-by: Yoshua Wuyts <[email protected]>
Signed-off-by: Yoshua Wuyts <[email protected]>
@taiki-e
Copy link
Contributor

taiki-e commented Oct 14, 2019

But it fails because write_fmt isn't part of an async-std release yet. I don't think we can add tests right now, and should instead test this within async-std.

Is it possible to use git dependencies in dev-dependencies? It blocks cargo publish, but for now I'd like to make sure this PR is working.
I'm worried about the possibility of releasing macros like async-rs/async-attributes#9 that don't actually compile.

macro_rules! print {
($($arg:tt)*) => (
async {
use ::async_std::prelude::*;
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure if this PR is good. This will make it difficult to reexport these macros.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Maybe my understanding of scoping is off here; but doesn't this include the prelude only within the async block?

The goal of this crate is to provide re-exports for async-std's submodules, because we can't define the macros inside the crate itself. I think as long as we cover that use case we should be okay?

Copy link
Contributor

Choose a reason for hiding this comment

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

I think this is enough for that purpose.

(I was thinking about the restriction that $crate cannot be used. But given proc-macro always has this limitation, we probably don't need to worry.)

Signed-off-by: Yoshua Wuyts <[email protected]>
@taiki-e
Copy link
Contributor

taiki-e commented Oct 14, 2019

@taiki-e I'm not sure how to go about the testing stdout output here. We could do some process spawning + capture, but it doesn't feel great. Do you have any suggestions?

Or well, we could check if things compile which may be good enough. I'll add those tests (:

Oh, sorry. I said tests, but in fact I just want to see if it can be compiled.

@taiki-e
Copy link
Contributor

taiki-e commented Oct 31, 2019

since async-rs/async-std#328 is closed, does this also close?

@yoshuawuyts
Copy link
Collaborator Author

Yeah, I think so!

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.

2 participants