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 teeny tiny amount of trace logging #17

Merged
merged 2 commits into from
Jun 30, 2019

Conversation

killercup
Copy link
Contributor

This is a first step towards #9.

I had waaaay more logs in there but those were just for me to understand
how this works. (Turns out, I just wrote inner where it should've said
outer.)

I had waaaay more logs in there but those were just for me to understand
how this works. (Turns out, I just wrote `inner` where it should've said
`outer`.)
@CAD97
Copy link
Collaborator

CAD97 commented Jun 20, 2019

To avoid the need to add log to the user's crate, log should probably be re-exported from from-pest conditionally. Honestly, I'm fine just adding log support unconditionally, it's good to have and it's not like log is going to significantly change the compile time of this procedural macro.

The problem with adding a "peer dependency" like this is that it'll break upstream crates using pest-ast as well as the crate enabling the feature, so it's not purely additive.

@killercup
Copy link
Contributor Author

Good idea about re-exporting log. It might be a bit tricky to get it right, though, as a single log::trace!(..) expands to this:

{
    let lvl = ::log::Level::Trace;
    if lvl <= ::log::STATIC_MAX_LEVEL && lvl <= ::log::max_level() {
        ::log::__private_api_log( fmt_args!(OMITTED) );
    }
};

As you can see, it refers to a few items from log itself. I'll try to adapt the function I added to directly emit this code while replacing ::log:: with ::from_pest::.

I'll still keep it behind a feature flag, just because it's no additional work; but I'll enable it by default.

@killercup
Copy link
Contributor Author

Ah, nope, not necessary thanks to #[macro_export(local_inner_macros)]!

@CAD97 CAD97 merged commit 4bfef36 into pest-parser:master Jun 30, 2019
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