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

Rename #[doc(spotlight)] to #[doc(notable_trait)] #80965

Merged
merged 1 commit into from
Apr 2, 2021

Conversation

camelid
Copy link
Member

@camelid camelid commented Jan 13, 2021

Fixes #80936.

"spotlight" is not a very specific or self-explaining name.
Additionally, the dialog that it triggers is called "Notable traits".
So, "notable trait" is a better name.

  • Rename #[doc(spotlight)] to #[doc(notable_trait)]
  • Rename #![feature(doc_spotlight)] to #![feature(doc_notable_trait)]
  • Update documentation
  • Improve documentation

r? @Manishearth

@camelid camelid added A-traits Area: Trait system C-enhancement Category: An issue proposing an enhancement or a PR with one. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. labels Jan 13, 2021
@rust-highfive

This comment has been minimized.

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jan 13, 2021
@camelid
Copy link
Member Author

camelid commented Jan 13, 2021

Dang, I wish git add src/ wouldn't add submodules.

@rust-log-analyzer

This comment has been minimized.

@Manishearth
Copy link
Member

r=me once @jyn514 and @GuillaumeGomez have a chance to approve the new name

@camelid
Copy link
Member Author

camelid commented Jan 13, 2021

Note that #[doc(spotlight)] will fail as if it never existed. However, it's probably fine because:

  1. it's a niche feature, and
  2. #![feature(doc_spotlight)] should point towards the new feature flag

@rust-log-analyzer

This comment has been minimized.

@camelid camelid added the S-waiting-on-team Status: Awaiting decision from the relevant subteam (see the T-<team> label). label Jan 13, 2021
@camelid
Copy link
Member Author

camelid commented Jan 13, 2021

Also note that this only changes the user interface; internally in rustdoc it's still called spotlight. That's because this will likely conflict with #80914 and I want to minimize the conflicts and unnecessary work :)

@camelid
Copy link
Member Author

camelid commented Jan 13, 2021

I used my own rustc-dev-guide docs for this ^^

@rust-log-analyzer

This comment has been minimized.

@jyn514
Copy link
Member

jyn514 commented Jan 13, 2021

I would strongly prefer to deprecate spotlight and have it to suggest using notable_trait instead. Otherwise people won't know where it went and be very confused.

@jyn514
Copy link
Member

jyn514 commented Jan 13, 2021

Oh, you did that, sorry. I didn't realize it from the PR description.

@jyn514
Copy link
Member

jyn514 commented Jan 13, 2021

Can you add a test that doc(spotlight) suggests using notable_trait instead?

@camelid
Copy link
Member Author

camelid commented Jan 13, 2021

Oh, you did that, sorry. I didn't realize it from the PR description.

I used the established method for renaming the feature gate, but #[doc(spotlight)] will silently do nothing. However, any real code will be using the feature flag so it's probably not a big deal. If you have a better idea, I'm happy to try it!

src/librustdoc/clean/inline.rs Outdated Show resolved Hide resolved
@camelid camelid added the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label Jan 13, 2021
@rust-log-analyzer

This comment has been minimized.

@camelid
Copy link
Member Author

camelid commented Jan 13, 2021

Huh, why did this test fail?

---- [ui] ui/panic-handler/weak-lang-item.rs stdout ----
diff of stderr:

10	LL | extern crate core as other_core;
11	   |
12	
-	error: language item required, but not found: `eh_personality`
-	
15	error: `#[panic_handler]` function required, but not found
+	
+	error: language item required, but not found: `eh_personality`
16	
17	error: aborting due to 3 previous errors
18	

Seems like a weird spurious issue...

@rust-log-analyzer

This comment has been minimized.

@rust-log-analyzer

This comment has been minimized.

@rust-log-analyzer

This comment has been minimized.

@rust-log-analyzer

This comment has been minimized.

@bors

This comment has been minimized.

"spotlight" is not a very specific or self-explaining name.
Additionally, the dialog that it triggers is called "Notable traits".
So, "notable trait" is a better name.

* Rename `#[doc(spotlight)]` to `#[doc(notable_trait)]`
* Rename `#![feature(doc_spotlight)]` to `#![feature(doc_notable_trait)]`
* Update documentation
* Improve documentation
@bstrie
Copy link
Contributor

bstrie commented Mar 31, 2021

Triage: can any of the reviewers here weigh in on the new revision?

Copy link
Member

@jyn514 jyn514 left a comment

Choose a reason for hiding this comment

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

Sorry for the delay. r=me with the nit addressed.

String::from("notable_trait"),
Applicability::MachineApplicable,
);
diag.note("`doc(spotlight)` is now a no-op");
Copy link
Member

Choose a reason for hiding this comment

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

Nah, this seems fine.

Comment on lines +93 to +112
Rustdoc keeps a list of a few traits that are believed to be "fundamental" to
types that implement them. These traits are intended to be the primary interface
for their implementers, and are often most of the API available to be documented
on their types. For this reason, Rustdoc will track when a given type implements
one of these traits and call special attention to it when a function returns one
of these types. This is the "Notable traits" dialog, accessible as a circled `i`
button next to the function, which, when clicked, shows the dialog.

In the standard library, some of the traits that are part of this list are
`Iterator`, `Future`, `io::Read`, and `io::Write`. However, rather than being
implemented as a hard-coded list, these traits have a special marker attribute
on them: `#[doc(notable_trait)]`. This means that you can apply this attribute
to your own trait to include it in the "Notable traits" dialog in documentation.

The `#[doc(notable_trait)]` attribute currently requires the `#![feature(doc_notable_trait)]`
feature gate. For more information, see [its chapter in the Unstable Book][unstable-notable_trait]
and [its tracking issue][issue-notable_trait].

[unstable-notable_trait]: ../unstable-book/language-features/doc-notable-trait.html
[issue-notable_trait]: https://github.com/rust-lang/rust/issues/45040
Copy link
Member

Choose a reason for hiding this comment

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

nit: is there a reason you rewrapped this? Did you change any of the text or just the wrapping?

Copy link
Member Author

Choose a reason for hiding this comment

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

I did change the text; I changed mentions of "spotlight" to "notable trait", and I tweaked some of the wording. Sorry for the re-wrap, I know it's hard to review. I use gqap in Vim whenever I add text so that I don't end up with super long lines.

Here's the Git word-diff output, which should hopefully be easier to review:

diff --git a/old.md b/new.md
index 02a6f7b..92322be 100644
--- a/old.md
+++ b/new.md
@@ -1,20 +1,22 @@
### Adding your trait to the [-"Important Traits"-]{+"Notable traits"+} dialog

Rustdoc keeps a list of a few traits that are believed to be "fundamental" to
[-a given type when-]
[-implemented on it.-]{+types that implement them.+} These traits are intended to be the primary interface
for their [-types,-]{+implementers,+} and are often {+most of+} the [-only thing-]{+API+} available to be documented
on their types. For this reason, Rustdoc will track when a given type implements
one of these traits and call special attention to it when a function returns one
of these types. This is the [-"Important Traits"-]{+"Notable traits"+} dialog, [-visible-]{+accessible+} as a [-circle-i-]{+circled `i`+}
button next to the function, which, when clicked, shows the dialog.

In the standard library, {+some of+} the traits that [-qualify for inclusion-]{+are part of this list+} are
`Iterator`, {+`Future`,+} `io::Read`, and `io::Write`. However, rather than being
implemented as a hard-coded list, these traits have a special marker attribute
on them: [-`#[doc(spotlight)]`.-]{+`#[doc(notable_trait)]`.+} This means that you [-could-]{+can+} apply this attribute
to your own trait to include it in the [-"Important Traits"-]{+"Notable traits"+} dialog in documentation.

The [-`#[doc(spotlight)]`-]{+`#[doc(notable_trait)]`+} attribute currently requires the [-`#![feature(doc_spotlight)]`-]{+`#![feature(doc_notable_trait)]`+}
feature gate. For more information, see [its chapter in the Unstable [-Book][unstable-spotlight]-]{+Book][unstable-notable_trait]+}
and [its tracking [-issue][issue-spotlight].-]{+issue][issue-notable_trait].+}

[-[unstable-spotlight]: ../unstable-book/language-features/doc-spotlight.html-]
[-[issue-spotlight]:-]{+[unstable-notable_trait]: ../unstable-book/language-features/doc-notable-trait.html+}
{+[issue-notable_trait]:+} https://github.com/rust-lang/rust/issues/45040

Copy link
Member

Choose a reason for hiding this comment

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

Ok cool, I am not sure all those changes are improvements but they at least don't seem strictly worse.

@jyn514
Copy link
Member

jyn514 commented Apr 1, 2021

@bors r+

@bors
Copy link
Contributor

bors commented Apr 1, 2021

📌 Commit 34c6cee has been approved by jyn514

@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 Apr 1, 2021
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request Apr 1, 2021
Rename `#[doc(spotlight)]` to `#[doc(notable_trait)]`

Fixes rust-lang#80936.

"spotlight" is not a very specific or self-explaining name.
Additionally, the dialog that it triggers is called "Notable traits".
So, "notable trait" is a better name.

* Rename `#[doc(spotlight)]` to `#[doc(notable_trait)]`
* Rename `#![feature(doc_spotlight)]` to `#![feature(doc_notable_trait)]`
* Update documentation
* Improve documentation

r? `@Manishearth`
@bors
Copy link
Contributor

bors commented Apr 2, 2021

⌛ Testing commit 34c6cee with merge 5662d93...

@bors
Copy link
Contributor

bors commented Apr 2, 2021

☀️ Test successful - checks-actions
Approved by: jyn514
Pushing 5662d93 to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Apr 2, 2021
@bors bors merged commit 5662d93 into rust-lang:master Apr 2, 2021
@rustbot rustbot added this to the 1.53.0 milestone Apr 2, 2021
@bors bors mentioned this pull request Apr 2, 2021
@camelid camelid deleted the rename-doc-spotlight branch April 2, 2021 17:43
camelid added a commit to camelid/rust that referenced this pull request Apr 2, 2021
I didn't make these renames in rust-lang#80965 because I didn't want the PR to
conflict with rust-lang#80914.
JohnTitor added a commit to JohnTitor/rust that referenced this pull request Apr 3, 2021
…ht, r=GuillaumeGomez

rustdoc: Rename internal uses of `spotlight`

I didn't make these renames in rust-lang#80965 because I didn't want the PR to
conflict with rust-lang#80914.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-traits Area: Trait system C-enhancement Category: An issue proposing an enhancement or a PR with one. merged-by-bors This PR was explicitly merged by bors. 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.

Rename doc(spotlight) to doc(notable_trait)
10 participants