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

[rustdoc] Add highlighting for comments in items declaration #117869

Merged

Conversation

GuillaumeGomez
Copy link
Member

@GuillaumeGomez GuillaumeGomez commented Nov 13, 2023

Fixes #117555.

So after the discussion in #117643, the outcome was that having the comments in the item declaration at the same level (in term of color) as the rest of the code was actually a bit distracting and could be improved.

The current highlighting color for comments is "lighter" than the rest and I think it fits perfectly to improve the current situation. With this, we now have different "levels" which makes it easier to read and filter out what we want when reading the items declaration.

Here's a screenshot:

image

r? @notriddle

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. labels Nov 13, 2023
@GuillaumeGomez GuillaumeGomez changed the title Comment highlighting item decl [rustdoc] Add highlighting for comments in items declaration Nov 13, 2023
@rust-log-analyzer

This comment has been minimized.

@GuillaumeGomez
Copy link
Member Author

Forgot to update some rustdoc snapshots. Updated.

@rust-log-analyzer

This comment has been minimized.

@GuillaumeGomez
Copy link
Member Author

And the rebase added a new test that I updated. This time it finally passed CI.

T: <a class="trait" href="trait.MyTrait.html" title="trait foo::MyTrait">MyTrait</a></span>;</code></pre>
T: <a class="trait" href="trait.MyTrait.html" title="trait foo::MyTrait">MyTrait</a></span>;</code></pre>
Copy link
Contributor

Choose a reason for hiding this comment

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

Does anybody know what's going on with trailing newlines here? This seems like something that should be fixable, though presumably it should go in a separate PR.

@notriddle
Copy link
Contributor

User-visible change -> FCP.

@rfcbot fcp merge

@rfcbot
Copy link

rfcbot commented Nov 13, 2023

Team member @notriddle has proposed to merge this. The next step is review by the rest of the tagged team members:

No concerns currently listed.

Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

See this document for info about what commands tagged team members can give me.

@rfcbot rfcbot added proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. disposition-merge This issue / PR is in PFCP or FCP with a disposition to merge it. labels Nov 13, 2023
@rfcbot rfcbot added final-comment-period In the final comment period and will be merged soon unless new substantive objections are raised. and removed proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. labels Nov 23, 2023
@rfcbot
Copy link

rfcbot commented Nov 23, 2023

🔔 This is now entering its final comment period, as per the review above. 🔔

@bors
Copy link
Contributor

bors commented Nov 23, 2023

☔ The latest upstream changes (presumably #118192) made this pull request unmergeable. Please resolve the merge conflicts.

@bors
Copy link
Contributor

bors commented Dec 1, 2023

☔ The latest upstream changes (presumably #118493) made this pull request unmergeable. Please resolve the merge conflicts.

@rfcbot rfcbot added finished-final-comment-period The final comment period is finished for this PR / Issue. to-announce Announce this issue on triage meeting and removed final-comment-period In the final comment period and will be merged soon unless new substantive objections are raised. labels Dec 3, 2023
@rfcbot
Copy link

rfcbot commented Dec 3, 2023

The final comment period, with a disposition to merge, as per the review above, is now complete.

As the automated representative of the governance process, I would like to thank the author for their work and everyone else who contributed.

This will be merged soon.

@GuillaumeGomez
Copy link
Member Author

@bors r=notriddle rollup

@bors
Copy link
Contributor

bors commented Dec 3, 2023

📌 Commit 06695ea has been approved by notriddle

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 Dec 3, 2023
bors added a commit to rust-lang-ci/rust that referenced this pull request Dec 3, 2023
…iaskrgr

Rollup of 3 pull requests

Successful merges:

 - rust-lang#117869 ([rustdoc] Add highlighting for comments in items declaration)
 - rust-lang#118525 (coverage: Skip spans that can't be un-expanded back to the function body)
 - rust-lang#118574 (rustc_session: Address all `rustc::potential_query_instability` lints)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 591b845 into rust-lang:master Dec 3, 2023
11 checks passed
@rustbot rustbot added this to the 1.76.0 milestone Dec 3, 2023
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Dec 3, 2023
Rollup merge of rust-lang#117869 - GuillaumeGomez:comment-highlighting-item-decl, r=notriddle

[rustdoc] Add highlighting for comments in items declaration

Fixes rust-lang#117555.

So after the discussion in rust-lang#117643, the outcome was that having the comments in the item declaration at the same level (in term of color) as the rest of the code was actually a bit distracting and could be improved.

The current highlighting color for comments is "lighter" than the rest and I think it fits perfectly to improve the current situation. With this, we now have different "levels" which makes it easier to read and filter out what we want when reading the items declaration.

Here's a screenshot:

![image](https://github.com/rust-lang/rust/assets/3050060/dbd98029-e98b-4997-9a89-6b823eaac9a4)

r? `@notriddle`
@GuillaumeGomez GuillaumeGomez deleted the comment-highlighting-item-decl branch December 4, 2023 10:26
@apiraino apiraino removed the to-announce Announce this issue on triage meeting label Dec 14, 2023
wip-sync pushed a commit to NetBSD/pkgsrc-wip that referenced this pull request Feb 18, 2024
Pkgsrc changes:
 * Adapt checksums and patches.

Upstream chnages:

Version 1.76.0 (2024-02-08)
==========================

Language
--------
- [Document Rust ABI compatibility between various types]
  (rust-lang/rust#115476)
- [Also: guarantee that char and u32 are ABI-compatible]
  (rust-lang/rust#118032)
- [Warn against ambiguous wide pointer comparisons]
  (rust-lang/rust#117758)

Compiler
--------
- [Lint pinned `#[must_use]` pointers (in particular, `Box<T>`
  where `T` is `#[must_use]`) in `unused_must_use`.]
  (rust-lang/rust#118054)
- [Soundness fix: fix computing the offset of an unsized field in
  a packed struct]
  (rust-lang/rust#118540)
- [Soundness fix: fix dynamic size/align computation logic for
  packed types with dyn Trait tail]
  (rust-lang/rust#118538)
- [Add `$message_type` field to distinguish json diagnostic outputs]
  (rust-lang/rust#115691)
- [Enable Rust to use the EHCont security feature of Windows]
  (rust-lang/rust#118013)
- [Add tier 3 {x86_64,i686}-win7-windows-msvc targets]
  (rust-lang/rust#118150)
- [Add tier 3 aarch64-apple-watchos target]
  (rust-lang/rust#119074)
- [Add tier 3 arm64e-apple-ios & arm64e-apple-darwin targets]
  (rust-lang/rust#115526)

Refer to Rust's [platform support page][platform-support-doc]
for more information on Rust's tiered platform support.

Libraries
---------
- [Add a column number to `dbg!()`]
  (rust-lang/rust#114962)
- [Add `std::hash::{DefaultHasher, RandomState}` exports]
  (rust-lang/rust#115694)
- [Fix rounding issue with exponents in fmt]
  (rust-lang/rust#116301)
- [Add T: ?Sized to `RwLockReadGuard` and `RwLockWriteGuard`'s Debug impls.]
  (rust-lang/rust#117138)
- [Windows: Allow `File::create` to work on hidden files]
  (rust-lang/rust#116438)

Stabilized APIs
---------------
- [`Arc::unwrap_or_clone`]
  (https://doc.rust-lang.org/stable/std/sync/struct.Arc.html#method.unwrap_or_clone)
- [`Rc::unwrap_or_clone`]
  (https://doc.rust-lang.org/stable/std/rc/struct.Rc.html#method.unwrap_or_clone)
- [`Result::inspect`]
  (https://doc.rust-lang.org/stable/std/result/enum.Result.html#method.inspect)
- [`Result::inspect_err`]
  (https://doc.rust-lang.org/stable/std/result/enum.Result.html#method.inspect_err)
- [`Option::inspect`]
  (https://doc.rust-lang.org/stable/std/option/enum.Option.html#method.inspect)
- [`type_name_of_val`]
  (https://doc.rust-lang.org/stable/std/any/fn.type_name_of_val.html)
- [`std::hash::{DefaultHasher, RandomState}`]
  (https://doc.rust-lang.org/stable/std/hash/index.html#structs)
  These were previously available only through `std::collections::hash_map`.
- [`ptr::{from_ref, from_mut}`]
  (https://doc.rust-lang.org/stable/std/ptr/fn.from_ref.html)
- [`ptr::addr_eq`](https://doc.rust-lang.org/stable/std/ptr/fn.addr_eq.html)

Cargo
-----

See [Cargo release notes]
(https://github.com/rust-lang/cargo/blob/master/CHANGELOG.md#cargo-176-2024-02-08).

Rustdoc
-------
- [Don't merge cfg and doc(cfg) attributes for re-exports]
  (rust-lang/rust#113091)
- [rustdoc: allow resizing the sidebar / hiding the top bar]
  (rust-lang/rust#115660)
- [rustdoc-search: add support for traits and associated types]
  (rust-lang/rust#116085)
- [rustdoc: Add highlighting for comments in items declaration]
  (rust-lang/rust#117869)

Compatibility Notes
-------------------
- [Add allow-by-default lint for unit bindings]
  (rust-lang/rust#112380)
  This is expected to be upgraded to a warning by default in a future Rust
  release. Some macros emit bindings with type `()` with user-provided spans,
  which means that this lint will warn for user code.
- [Remove x86_64-sun-solaris target.]
  (rust-lang/rust#118091)
- [Remove asmjs-unknown-emscripten target]
  (rust-lang/rust#117338)
- [Report errors in jobserver inherited through environment variables]
  (rust-lang/rust#113730)
  This [may warn](rust-lang/rust#120515)
  on benign problems too.
- [Update the minimum external LLVM to 16.]
  (rust-lang/rust#117947)
- [Improve `print_tts`](rust-lang/rust#114571)
  This change can break some naive manual parsing of token trees
  in proc macro code which expect a particular structure after
  `.to_string()`, rather than just arbitrary Rust code.
- [Make `IMPLIED_BOUNDS_ENTAILMENT` into a hard error from a lint]
  (rust-lang/rust#117984)
- [Vec's allocation behavior was changed when collecting some iterators]
  (rust-lang/rust#110353)
  Allocation behavior is currently not specified, nevertheless
  changes can be surprising.
  See [`impl FromIterator for Vec`]
  (https://doc.rust-lang.org/nightly/std/vec/struct.Vec.html#impl-FromIterator%3CT%3E-for-Vec%3CT%3E)
  for more details.
- [Properly reject `default` on free const items]
  (rust-lang/rust#117818)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
disposition-merge This issue / PR is in PFCP or FCP with a disposition to merge it. finished-final-comment-period The final comment period is finished for this PR / Issue. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

rustdoc does not consistently apply syntax highlighting to the item declaration
7 participants