rustdoc: Treat declarative macros more like other item kinds #132302

fmease · 2024-10-29T02:38:47Z

Apparently at some time in the past we were unable to generate an href for the module path inside the visibility of decl macros 2.0 (pub(in ...)). As a result of this, a whole separate function was introduced specifically for printing the visibility of decl macros that didn't attempt to generate any links. The description of PR #84074 states:

This fixes the overly-complex invariant mentioned in #83237 (comment), where the macro source can't have any links in it only because the cache hasn't been populated yet.

I can no longer reproduce the original issue. Reusing the existing visibility rendering logic seems to work just fine (I couldn't come up with any counterexamples, though I invite you to prove me wrong).

Fixes Paths in macro visibilities are not linkified #83000
Fixes the visibility showing up "twice" in rustdoc-JSON output: Once as the visibility field, once baked into the source¹
Fixes #[doc(hidden)] not getting rendered on doc(hidden) decl macros 2.0 under --document-hidden-items (for decl macros 1.2 the issue remains; I will address this separately when fixing rustdoc: Appearance of #[doc(hidden)] differs from all the other attributes #132304).

Outdated Section

NOTE: The current version of this PR is committing a UI crime, I'd like to receive feedback on that. Maybe you have a satisfactory solution for how to remedy it. Namely, as you know we have two different ways of / modes for highlighting code with color:

Only highlighting links / item paths and avoiding to highlight tokens by kind like keywords (to reduce visual noise and maybe also artifact size). Used for item declarations(*).
Highlighting tokens by kind. Used for code blocks written by the user.

(*): With the notable exception being macro declarations! Well, since this PR reuses the same function for rendering the item visibility (which only makes sense), we have a clash of modes: We now use both ways of highlighting code for decl macros: №1 for the visibility, №2 for the rest. This awkward. See for yourself:

On master:
On this branch:

Furthermore, we now no longer syntax-highlight declarative macros (be it macro_rules! or macro) since that was inconsistent with the way we render all other item kinds. See (collapsed) Outdated Section above. See also #132302 (comment).

On master	On this branch

E.g., "visibility":{"restricted":{"parent":1,"path":"::a"}},/*OMITTED*/,"inner":{"macro":"pub(in a) macro by_example_vis_named($foo:expr) {\n ...\n}"} ↩

rustbot · 2024-10-29T02:38:55Z

r? @notriddle

rustbot has assigned @notriddle.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

rustbot · 2024-10-29T02:38:57Z

Some changes occurred in src/librustdoc/clean/types.rs

cc @camelid

fmease · 2024-10-29T02:39:40Z

src/librustdoc/html/render/print_item.rs

@@ -1745,7 +1745,13 @@ fn item_variants(
 }

 fn item_macro(w: &mut Buffer, cx: &mut Context<'_>, it: &clean::Item, t: &clean::Macro) {
-    highlight::render_item_decl_with_highlighting(&t.source, w);
+    wrap_item(w, |w| {
+        // FIXME: Also print `#[doc(hidden)]` for `macro_rules!` if it `is_doc_hidden`.


Preexisting, will fix separately bc. there's another bug related to this that concerns all item kinds: #132304. Context: visibility_print_with_space not only renders an item's vis but also its #[doc(hidden)] if applicable.

fmease · 2024-10-29T02:45:32Z

src/librustdoc/html/render/print_item.rs

-    highlight::render_item_decl_with_highlighting(&t.source, w);
+    wrap_item(w, |w| {
+        // FIXME: Also print `#[doc(hidden)]` for `macro_rules!` if it `is_doc_hidden`.
+        if !t.macro_rules {


(pub... macro_rules! was once proposed as a middle ground between decl macros 1.2 and 2.0; however, this proposal was rejected if I remember correctly)

notriddle · 2024-10-29T17:55:30Z

src/librustdoc/html/render/print_item.rs

+        if !t.macro_rules {
+            write!(w, "{}", visibility_print_with_space(it, cx));
+        }
+        highlight::write_code(w, &t.source, None, None);


I do think we should just stop highlighting macros.

Here's the original PR that added it: rustdoc: syntax highlight macro definitions, colour $... substitutions. #12647

I notice that, in the original implementation, the highlighter accomplished more because the body source code was shown.

A later pull request made it only show the arm, making the highlighter less useful, but not removing it, and no reason for keeping the highlighter was given AFAICT: Rustdoc: Only show the interface provided by macros #30074

notriddle · 2024-11-13T18:55:09Z

@bors r+

bors · 2024-11-13T18:55:11Z

📌 Commit 9016711 has been approved by notriddle

It is now in the queue for this repository.

…iaskrgr Rollup of 8 pull requests Successful merges: - rust-lang#126046 (Implement `mixed_integer_ops_unsigned_sub`) - rust-lang#132302 (rustdoc: Treat declarative macros more like other item kinds) - rust-lang#132842 (ABI checks: add support for tier2 arches) - rust-lang#132995 (compiletest: Add ``exact-llvm-major-version`` directive) - rust-lang#132996 (Trim extra space when suggesting removing bad `let`) - rust-lang#132998 (Unvacation myself) - rust-lang#133000 ([rustdoc] Fix duplicated footnote IDs) - rust-lang#133001 (actually test next solver) r? `@ghost` `@rustbot` modify labels: rollup

Rollup merge of rust-lang#132302 - fmease:rustdoc-better-vis-for-macro-decl, r=notriddle rustdoc: Treat declarative macros more like other item kinds Apparently at some time in the past we were unable to generate an href for the module path inside the visibility of decl macros 2.0 (`pub(in ...)`). As a result of this, a whole separate function was introduced specifically for printing the visibility of decl macros that didn't attempt to generate any links. The description of PR rust-lang#84074 states: > This fixes the overly-complex invariant mentioned in rust-lang#83237 (comment), where the macro source can't have any links in it only because the cache hasn't been populated yet. I can no longer reproduce the original issue. Reusing the existing visibility rendering logic *seems* to work just fine (I couldn't come up with any counterexamples, though I invite you to prove me wrong). * Fixes rust-lang#83000 * Fixes the visibility showing up "twice" in rustdoc-JSON output: Once as the `visibility` field, once baked into the source[^1] * Fixes `#[doc(hidden)]` not getting rendered on doc(hidden) decl macros 2.0 under `--document-hiden-items` (for decl macros 1.2 the issue remains; I will address this separately when fixing rust-lang#132304). --- <details><summary>Outdated Section</summary> NOTE: The current version of this PR is committing a UI crime, I'd like to receive feedback on that. Maybe you have a satisfactory solution for how to remedy it. Namely, as you know we have two different ways of / modes for highlighting code with color: 1. Only highlighting links / item paths and avoiding to highlight tokens by kind like keywords (to reduce visual noise and maybe also artifact size). Used for item declarations(\*). 2. Highlighting tokens by kind. Used for code blocks written by the user. (\*): With the notable exception being macro declarations! Well, since this PR reuses the same function for rendering the item visibility (which only makes sense), we have a clash of modes: We now use both ways of highlighting code for decl macros: №1 for the visibility, №2 for the rest. This awkward. See for yourself: * On master: ![Screenshot 2024-10-29 at 03-37-48 by_example_vis_named in decl_macro a b c - Rust](https://github.com/user-attachments/assets/22f0ab6e-9ba9-4c4e-8fb0-0741c91d360b) * On this branch: ![Screenshot 2024-10-29 at 03-36-41 by_example_vis_named in decl_macro a b c - Rust](https://github.com/user-attachments/assets/b11d81a3-3e2e-43cb-a5b8-6773a3048732) </details> Furthermore, we now no longer syntax-highlight declarative macros (be it `macro_rules!` or `macro`) since that was inconsistent with the way we render all other item kinds. See (collapsed) *Outdated Section* above. See also rust-lang#132302 (comment). | On master | On this branch | |---|---| | ![Screenshot 2024-11-13 at 16-12-46 by_example_vis_named in decl_macro a b c - Rust](https://github.com/user-attachments/assets/cb3aeb42-a56d-4ced-80d9-f2694f369af1) | ![Screenshot 2024-11-13 at 16-13-22 by_example_vis_named in decl_macro a b c - Rust](https://github.com/user-attachments/assets/b73bee50-1b85-4862-afba-5ad471443ccc) | [^1]: E.g., `"visibility":{"restricted":{"parent":1,"path":"::a"}},/*OMITTED*/,"inner":{"macro":"pub(in a) macro by_example_vis_named($foo:expr) {\n ...\n}"}`

rustbot assigned notriddle Oct 29, 2024

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 Oct 29, 2024

fmease commented Oct 29, 2024

View reviewed changes

fmease changed the title ~~rustdoc: Treat decl macros like other items when it comes to visibility~~ rustdoc: Treat decl macros like other item kinds when it comes to visibility Oct 29, 2024

notriddle reviewed Oct 29, 2024

View reviewed changes

fmease added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Nov 5, 2024

rustdoc: Treat decl macros like other items

9016711

fmease force-pushed the rustdoc-better-vis-for-macro-decl branch from 179cba4 to 9016711 Compare November 13, 2024 15:05

fmease changed the title ~~rustdoc: Treat decl macros like other item kinds when it comes to visibility~~ rustdoc: Treat decl macros like other item kinds Nov 13, 2024

fmease changed the title ~~rustdoc: Treat decl macros like other item kinds~~ rustdoc: Treat declarative macros like other item kinds Nov 13, 2024

fmease added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Nov 13, 2024

fmease changed the title ~~rustdoc: Treat declarative macros like other item kinds~~ rustdoc: Treat declarative macros more like other item kinds Nov 13, 2024

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 Nov 13, 2024

matthiaskrgr mentioned this pull request Nov 13, 2024

Rollup of 8 pull requests #133006

Merged

bors merged commit 3376eda into rust-lang:master Nov 14, 2024
6 checks passed

rustbot added this to the 1.84.0 milestone Nov 14, 2024

fmease deleted the rustdoc-better-vis-for-macro-decl branch November 14, 2024 04:17

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

rustdoc: Treat declarative macros more like other item kinds #132302

rustdoc: Treat declarative macros more like other item kinds #132302

fmease commented Oct 29, 2024 •

edited

Loading

rustbot commented Oct 29, 2024

rustbot commented Oct 29, 2024

fmease Oct 29, 2024 •

edited

Loading

fmease Oct 29, 2024

notriddle Oct 29, 2024

fmease Nov 13, 2024

notriddle commented Nov 13, 2024

bors commented Nov 13, 2024

rustdoc: Treat declarative macros more like other item kinds #132302

rustdoc: Treat declarative macros more like other item kinds #132302

Conversation

fmease commented Oct 29, 2024 • edited Loading

Footnotes

rustbot commented Oct 29, 2024

rustbot commented Oct 29, 2024

fmease Oct 29, 2024 • edited Loading

Choose a reason for hiding this comment

fmease Oct 29, 2024

Choose a reason for hiding this comment

notriddle Oct 29, 2024

Choose a reason for hiding this comment

fmease Nov 13, 2024

Choose a reason for hiding this comment

notriddle commented Nov 13, 2024

bors commented Nov 13, 2024

fmease commented Oct 29, 2024 •

edited

Loading

fmease Oct 29, 2024 •

edited

Loading