Skip to content

Extend doc keyword feature by allowing any ident - #79464

Merged
bors merged 3 commits into
rust-lang:masterfrom
GuillaumeGomez:doc-keyword-ident
Nov 29, 2020
Merged

Extend doc keyword feature by allowing any ident#79464
bors merged 3 commits into
rust-lang:masterfrom
GuillaumeGomez:doc-keyword-ident

Conversation

@GuillaumeGomez

@GuillaumeGomezGuillaumeGomez commented Nov 27, 2020•
edited
Loading

Copy link
Copy Markdown
Member

Part of #51315.

As suggested by @danielhenrymantilla in this comment, this PR extends #[doc(keyword = "...")] to allow any ident to be used as keyword. The final goal is to allow (proc-)macro crates' owners to write documentation of the keywords they might introduce.

r? @jyn514

@rust-highfiverust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Nov 27, 2020
@GuillaumeGomezGuillaumeGomez changed the title Doc keyword identExtend doc keyword feature by allowing any identNov 27, 2020
@GuillaumeGomezGuillaumeGomez added the T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. label Nov 27, 2020
petrochenkov
petrochenkov reviewed Nov 27, 2020
View reviewed changes
Comment threadsrc/librustdoc/clean/mod.rs Outdated
danielhenrymantilla
danielhenrymantilla reviewed Nov 27, 2020
View reviewed changes

@danielhenrymantilladanielhenrymantilla left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Great! Having this will be awesome indeed for proc-macros 😍

  • Just a minor nit about the span of the error in the non-identifier case.

Comment threadsrc/test/rustdoc-ui/invalid-keyword.stderr Outdated
Comment threadsrc/test/rustdoc-ui/invalid-keyword.stderr Outdated
Comment threadsrc/librustdoc/clean/mod.rs Outdated
@jyn514

jyn514 commented Nov 27, 2020

Copy link
Copy Markdown
Member

Rather than removing is_doc_keyword, can you add an internal lint that fires if you try to use this on something that's not a keyword? I know there are internal lints for the compiler, not sure if they run on the standard library.

@GuillaumeGomez

GuillaumeGomez commented Nov 27, 2020

Copy link
Copy Markdown
MemberAuthor

You mean for the std directly or as a whole?

@petrochenkov

petrochenkov commented Nov 27, 2020

Copy link
Copy Markdown
Contributor

Rather than removing is_doc_keyword, can you add an internal lint that fires if you try to use this on something that's not a keyword? I know there are internal lints for the compiler, not sure if they run on the standard library.

The benefit/cost ratio for this is abysmal, IMO.

@GuillaumeGomez

GuillaumeGomez commented Nov 27, 2020

Copy link
Copy Markdown
MemberAuthor

Maybe @jyn514 means that we want to add a check for that specifically for the std? If so, I think it's worth it doing it in another PR (but still removing the function here). What do you think?

@jyn514

jyn514 commented Nov 27, 2020

Copy link
Copy Markdown
Member

Yes, I meant for the standard library.

@petrochenkov if you think that's not worth maintaining, I can make a PR doing this once, making sure the lint doesn't trigger, and then just close the PR.

@GuillaumeGomez

GuillaumeGomez commented Nov 28, 2020

Copy link
Copy Markdown
MemberAuthor

if you think that's not worth maintaining, I can make a PR doing this once, making sure the lint doesn't trigger, and then just close the PR.

@jyn514 Just realized that it's unneeded since it was only allowing official keywords before this PR. Therefore, the std literally could not add doc for something else.

@jyn514

jyn514 commented Nov 28, 2020

Copy link
Copy Markdown
Member

@GuillaumeGomez it could, it just wouldn't show up in the documentation. Anyway, I agree it's not worth blocking this PR.

@bors r+

@bors

bors commented Nov 28, 2020

Copy link
Copy Markdown
Collaborator

📌 Commit 482b3ac has been approved by jyn514

@borsbors 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 Nov 28, 2020
@GuillaumeGomezGuillaumeGomez mentioned this pull request Nov 28, 2020
Merged
GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this pull request Nov 28, 2020
@GuillaumeGomez
Rollup merge ofrust-lang#79464- GuillaumeGomez:doc-keyword-ident, r…
ffc5032
…=jyn514
Extend doc keyword feature by allowing any ident
Part of rust-lang#51315.
As suggested by `@danielhenrymantilla` in [this comment](rust-lang#51315 (comment)), this PR extends `#[doc(keyword = "...")]` to allow any ident to be used as keyword. The final goal is to allow (proc-)macro crates' owners to write documentation of the keywords they might introduce.
r? `@jyn514`
@GuillaumeGomezGuillaumeGomez mentioned this pull request Nov 28, 2020
Closed
@Dylan-DPC-zzDylan-DPC-zz mentioned this pull request Nov 29, 2020
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Nov 29, 2020
@bors
Auto merge ofrust-lang#79529- Dylan-DPC:rollup-6k20msr, r=Dylan-DPC
40d7efa
Rollup of 11 pull requests
Successful merges:
- rust-lang#79327 (Require allocator to be static for boxed `Pin`-API)
- rust-lang#79340 (Rename "stability" CSS class to "item-info" and combine `document_stability` with `document_short`)
- rust-lang#79363 (BTreeMap: try to enhance various comments)
- rust-lang#79395 (Move ui if tests from top-level into `expr/if`)
- rust-lang#79443 (Improve rustdoc JS tests error output)
- rust-lang#79464 (Extend doc keyword feature by allowing any ident)
- rust-lang#79484 (add enable-full-tools to freebsd builds to prevent occasional link er…)
- rust-lang#79505 (Cleanup: shorter and faster code)
- rust-lang#79514 (Add test for issue rust-lang#54121: order dependent trait bounds)
- rust-lang#79516 (Remove unnecessary `mut` binding)
- rust-lang#79528 (Fix a bootstrap comment)
Failed merges:
r? `@ghost`
`@rustbot` modify labels: rollup
@bors
bors merged commit ca8a1b0 into rust-lang:masterNov 29, 2020
@rustbotrustbot added this to the 1.50.0 milestone Nov 29, 2020
@GuillaumeGomez
GuillaumeGomez deleted the doc-keyword-ident branch November 29, 2020 14:27
GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this pull request Dec 1, 2020
@GuillaumeGomez
Rollup merge ofrust-lang#79509- GuillaumeGomez:improve-attr-spans, …
55c5127
…r=oli-obk
Improve attribute message error spans
I got the idea while working on rust-lang#79464
GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this pull request Dec 1, 2020
@GuillaumeGomez
Rollup merge ofrust-lang#79509- GuillaumeGomez:improve-attr-spans, …
3d631b0
…r=oli-obk
Improve attribute message error spans
I got the idea while working on rust-lang#79464
@jyn514jyn514 mentioned this pull request Jan 1, 2021
Merged
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@petrochenkovpetrochenkovpetrochenkov left review comments
+1 more reviewer
@danielhenrymantilladanielhenrymantilladanielhenrymantilla approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

@jyn514jyn514

Labels

S-waiting-on-borsStatus: Waiting on bors to run and complete tests. Bors will change the label on completion.T-rustdocRelevant to the rustdoc team, which will review and decide on the PR/issue.

Projects

None yet

Milestone

1.50.0

Development

Successfully merging this pull request may close these issues.

7 participants

@GuillaumeGomez@jyn514@petrochenkov@bors@danielhenrymantilla@rust-highfive@rustbot