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

Document Token{Stream,Tree}::Display more thoroughly. #120220

Merged
merged 1 commit into from
Jan 23, 2024

Conversation

nnethercote
Copy link
Contributor

To expressly warn against the kind of proc macro implementation that was broken in #119875.

r? @petrochenkov

To expressly warn against the kind of proc macro implementation that was
broken in rust-lang#119875.
@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jan 22, 2024
@nnethercote
Copy link
Contributor Author

I'm not totally set on the wording, and I am happy to hear any suggestions for improvements.

@@ -191,6 +191,14 @@ impl ToString for TokenStream {
/// Prints the token stream as a string that is supposed to be losslessly convertible back
/// into the same token stream (modulo spans), except for possibly `TokenTree::Group`s
/// with `Delimiter::None` delimiters and negative numeric literals.
///
/// Note: the exact form of the output is subject to change, e.g. there might
Copy link
Member

@fmease fmease Jan 22, 2024

Choose a reason for hiding this comment

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

You could consider placing the paragraphs inside <div class="warning"></div> (with an empty line between the tags and the content) and remove the leading Note: to make rustdoc render a warning block. This might be too much though.

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 was wondering if there was a better way to mark this paragraph, but I don't see any other use of this class in the library docs. But there are plenty of existing "Note:" examples. So I think I will leave it unchanged.

Copy link
Member

Choose a reason for hiding this comment

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

Right, that's because warning blocks were added pretty recently and they haven't found much adoption yet. There's one use of them in the rustc docs since I encouraged Nils to make use of them once.

However, I see your point, do however you prefer.

@petrochenkov
Copy link
Contributor

@bors r+ rollup

@bors
Copy link
Contributor

bors commented Jan 22, 2024

📌 Commit c4fc9ff has been approved by petrochenkov

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jan 22, 2024
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Jan 22, 2024
…cs, r=petrochenkov

Document `Token{Stream,Tree}::Display` more thoroughly.

To expressly warn against the kind of proc macro implementation that was broken in rust-lang#119875.

r? `@petrochenkov`
bors added a commit to rust-lang-ci/rust that referenced this pull request Jan 22, 2024
…iaskrgr

Rollup of 9 pull requests

Successful merges:

 - rust-lang#119664 (Fix tty detection for msys2's `/dev/ptmx`)
 - rust-lang#120104 (never_patterns: Count `!` bindings as diverging)
 - rust-lang#120109 (Move cmath into `sys`)
 - rust-lang#120143 (Consolidate logic around resolving built-in coroutine trait impls)
 - rust-lang#120159 (Track `verbose` and `verbose_internals`)
 - rust-lang#120188 (compiler: update freebsd and netbsd base specs.)
 - rust-lang#120216 (Fix a `trimmed_def_paths` assertion failure.)
 - rust-lang#120220 (Document `Token{Stream,Tree}::Display` more thoroughly.)
 - rust-lang#120233 (Revert stabilization of trait_upcasting feature)

r? `@ghost`
`@rustbot` modify labels: rollup
bors added a commit to rust-lang-ci/rust that referenced this pull request Jan 22, 2024
…iaskrgr

Rollup of 8 pull requests

Successful merges:

 - rust-lang#119664 (Fix tty detection for msys2's `/dev/ptmx`)
 - rust-lang#120104 (never_patterns: Count `!` bindings as diverging)
 - rust-lang#120109 (Move cmath into `sys`)
 - rust-lang#120143 (Consolidate logic around resolving built-in coroutine trait impls)
 - rust-lang#120159 (Track `verbose` and `verbose_internals`)
 - rust-lang#120216 (Fix a `trimmed_def_paths` assertion failure.)
 - rust-lang#120220 (Document `Token{Stream,Tree}::Display` more thoroughly.)
 - rust-lang#120233 (Revert stabilization of trait_upcasting feature)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit a430718 into rust-lang:master Jan 23, 2024
11 checks passed
@rustbot rustbot added this to the 1.77.0 milestone Jan 23, 2024
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Jan 23, 2024
Rollup merge of rust-lang#120220 - nnethercote:TokenStream-Display-docs, r=petrochenkov

Document `Token{Stream,Tree}::Display` more thoroughly.

To expressly warn against the kind of proc macro implementation that was broken in rust-lang#119875.

r? ``@petrochenkov``
DianQK pushed a commit to DianQK/rust that referenced this pull request Jan 23, 2024
…iaskrgr

Rollup of 8 pull requests

Successful merges:

 - rust-lang#119664 (Fix tty detection for msys2's `/dev/ptmx`)
 - rust-lang#120104 (never_patterns: Count `!` bindings as diverging)
 - rust-lang#120109 (Move cmath into `sys`)
 - rust-lang#120143 (Consolidate logic around resolving built-in coroutine trait impls)
 - rust-lang#120159 (Track `verbose` and `verbose_internals`)
 - rust-lang#120216 (Fix a `trimmed_def_paths` assertion failure.)
 - rust-lang#120220 (Document `Token{Stream,Tree}::Display` more thoroughly.)
 - rust-lang#120233 (Revert stabilization of trait_upcasting feature)

r? `@ghost`
`@rustbot` modify labels: rollup
@nnethercote nnethercote deleted the TokenStream-Display-docs branch January 23, 2024 03:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants