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

Exposes the formatting macros. #883

Merged
merged 1 commit into from
Jan 19, 2020
Merged

Conversation

ctm
Copy link
Contributor

@ctm ctm commented Jan 19, 2020

@jstarry This turned out to be a little trickier than I thought. I've verified that exposing the macros allows people to write their own extensions, but I couldn't really come up with any examples that weren't from real-life, so I just refer to the source filenames, which I think is a bad cop-out.

If you'd like, I can pull in the actual source from json.rs into macros.rs and rework it so it's an actual compiling documentation example. Replicated code feels bad to me though, although perhaps as an example it wouldn't be horrible if it got out of phase with the actual implementation of json.rs. I definitely didn't want to include the code I'm using to get the more compact representation for CBOR. However, I thought that mentioning the form of the macro that I used to get what I wanted might be useful, although I'm not a good judge.

So, I fully expect to do more work on this PR, but with a little additional guidance. However, I'm in no rush to get it merged.

@jstarry
Copy link
Member

jstarry commented Jan 19, 2020

@ctm I think real-life examples like bincode and json would be great. I think you can add dev-dependencies on those serde crates for doc tests to work properly.

This allows users to create their own encoding wrappers.

The provided documentation uses real-life examples from within yew.
This commit also fixes a minor oversight, where use of the
text_format_is_an_error macro required the end-user to explicitly use
the text_format macro instead of simply bringing it in itself.
@ctm ctm force-pushed the expose_format_macros branch from f7fa139 to 053fcad Compare January 19, 2020 17:49
@ctm
Copy link
Contributor Author

ctm commented Jan 19, 2020

@jstarry Thank you for the quick advice. I like the end result much better.

I reused some examples between macros, because of the way the macros work together.

The only thing I feel bad about is the name to_make_rustdoc_happy I used for the mod I needed to enclose some of the sample code in. Perhaps there's a way to get rid of that mod that I'm overlooking. If not, I'm all for a better name.

@jstarry
Copy link
Member

jstarry commented Jan 19, 2020

The only thing I feel bad about is the name to_make_rustdoc_happy I used for the mod I needed to enclose some of the sample code in.

Oh that's totally fine! Looks great

@jstarry jstarry merged commit 287f0d6 into yewstack:master Jan 19, 2020
llebout pushed a commit to llebout/yew that referenced this pull request Jan 20, 2020
This allows users to create their own encoding wrappers.

The provided documentation uses real-life examples from within yew.
This commit also fixes a minor oversight, where use of the
text_format_is_an_error macro required the end-user to explicitly use
the text_format macro instead of simply bringing it in itself.
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