Skip to content

rustdoc: make srcIndex no longer a global variable #142100

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

Merged
merged 1 commit into from
Jun 18, 2025

Conversation

lolbinarycat
Copy link
Contributor

@lolbinarycat lolbinarycat commented Jun 5, 2025

this is one-time initialization data, it can just
be a function parameter.

while we're doing that, we can more the json parsing into the function and save a few extra bytes of storage for free, at least in the case of multiple crates in a doc bundle.

fixes #138467

@rustbot
Copy link
Collaborator

rustbot commented Jun 5, 2025

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
Copy link
Collaborator

rustbot commented Jun 5, 2025

Some changes occurred in HTML/CSS/JS.

cc @GuillaumeGomez, @jsha, @lolbinarycat

Some changes occurred in GUI tests.

cc @GuillaumeGomez

@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. T-rustdoc-frontend Relevant to the rustdoc-frontend team, which will review and decide on the web UI/UX output. labels Jun 5, 2025
@rust-log-analyzer

This comment has been minimized.

@lolbinarycat lolbinarycat force-pushed the rustdoc-srcIndex-138467 branch from 80d65ae to 1f4435a Compare June 5, 2025 23:57
@GuillaumeGomez
Copy link
Member

Seems good to me, thanks! Please remove the left-over debugging and then I'll approve.

Copy link
Contributor

Choose a reason for hiding this comment

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

Why does this exist? searchIndex isn't part of the intended external interface, either.

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 mean, at least the search index is something we actually want to persist, but I suppose it could just live in a closure upvar instead of a global.

The results table needs to be public for integration testing, although i'm not sure if the same is true of searchIndex (it's definitely not true for srcIndex, I checked)

@lolbinarycat
Copy link
Contributor Author

@GuillaumeGomez debug print has been removed.

@GuillaumeGomez
Copy link
Member

Thanks!

Considering the second commit is the removal of a forgotten debug log, please squash it. Then I'll r+ the PR.

@lolbinarycat lolbinarycat force-pushed the rustdoc-srcIndex-138467 branch from 6cecd76 to 4236f4b Compare June 16, 2025 20:36
@rustbot

This comment has been minimized.

@lolbinarycat lolbinarycat force-pushed the rustdoc-srcIndex-138467 branch from 4236f4b to f62e2de Compare June 16, 2025 20:37
@lolbinarycat
Copy link
Contributor Author

@GuillaumeGomez squashed and rebased

@rust-log-analyzer

This comment has been minimized.

@lolbinarycat
Copy link
Contributor Author

sigh rebased this onto a broken master, gotta fix...

@lolbinarycat lolbinarycat force-pushed the rustdoc-srcIndex-138467 branch from f62e2de to a07ddd9 Compare June 16, 2025 20:50
@lolbinarycat
Copy link
Contributor Author

@GuillaumeGomez ok, rebased properly this time.

@GuillaumeGomez
Copy link
Member

Perfect, thanks! r=me once CI pass.

@bors delegate

@GuillaumeGomez
Copy link
Member

@bors delegate+

@bors
Copy link
Collaborator

bors commented Jun 16, 2025

✌️ @lolbinarycat, you can now approve this pull request!

If @GuillaumeGomez told you to "r=me" after making some further change, please make that change, then do @bors r=@GuillaumeGomez

* @param {string} srcIndexStr - strinified json map from crate name to dir structure
*/
function createSrcSidebar(srcIndexStr) {
console.log(srcIndexStr);
Copy link
Contributor

Choose a reason for hiding this comment

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

Another one? 😅 https://github.com/rust-lang/rust/pull/142100/files#r2131358481
I think we have eslint run on tidy, and it can catch these console.logs... might be an easy and helpful change to enable that lint.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

there's a few that have just been sitting in the repo for a while, i think.. unless they are there intentionally, which might be why the lint is disabled.

Copy link
Member

Choose a reason for hiding this comment

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

I'll add this check. We can handle each remaining console.log by telling eslint to ignore them.

Thanks @yotamofek!

this is one-time initialization data, it can just
be a function parameter.

we also move the json parsing into createSrcSidebar
to save a few bytes.
@lolbinarycat lolbinarycat force-pushed the rustdoc-srcIndex-138467 branch from a07ddd9 to 00c1042 Compare June 17, 2025 01:22
@lolbinarycat
Copy link
Contributor Author

@bors r=@GuillaumeGomez

@bors
Copy link
Collaborator

bors commented Jun 17, 2025

📌 Commit 00c1042 has been approved by GuillaumeGomez

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 Jun 17, 2025
workingjubilee added a commit to workingjubilee/rustc that referenced this pull request Jun 17, 2025
…67, r=GuillaumeGomez

rustdoc: make srcIndex no longer a global variable

this is one-time initialization data, it can just
be a function parameter.

while we're doing that, we can more the json parsing into the function and save a few extra bytes of storage for free, at least in the case of multiple crates in a doc bundle.

fixes rust-lang#138467
@GuillaumeGomez
Copy link
Member

@bors r-

@bors bors 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-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Jun 17, 2025
@GuillaumeGomez
Copy link
Member

Double-checked the console.log and nothing remaining so all good.

@bors r+ rollup

@bors
Copy link
Collaborator

bors commented Jun 17, 2025

📌 Commit 00c1042 has been approved by GuillaumeGomez

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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jun 17, 2025
jhpratt added a commit to jhpratt/rust that referenced this pull request Jun 17, 2025
…r=lolbinarycat

Lint about `console` calls in rustdoc JS

As discussed [here](rust-lang#142100 (comment)), this PR enforces that `console` is not used in rustdoc JS by default.

cc `@lolbinarycat`
bors added a commit that referenced this pull request Jun 18, 2025
Rollup of 14 pull requests

Successful merges:

 - #141574 (impl `Default` for `array::IntoIter`)
 - #141608 (Add support for repetition to `proc_macro::quote`)
 - #142100 (rustdoc: make srcIndex no longer a global variable)
 - #142371 (avoid `&mut P<T>` in `visit_expr` etc methods)
 - #142517 (Windows: Use anonymous pipes in Command)
 - #142520 (alloc: less static mut + some cleanup)
 - #142588 (Generic ctx imprv)
 - #142605 (Don't unwrap in enzyme builds in case of missing llvm-config)
 - #142608 (Refresh module-level docs for `rustc_target::spec`)
 - #142618 (Lint about `console` calls in rustdoc JS)
 - #142620 (Remove a panicking branch in `BorrowedCursor::advance`)
 - #142631 (Dont suggest remove semi inside macro expansion for redundant semi lint)
 - #142632 (Update cargo)
 - #142635 (Temporarily add back -Zwasm-c-abi=spec)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 0772ee7 into rust-lang:master Jun 18, 2025
10 checks passed
@rustbot rustbot added this to the 1.89.0 milestone Jun 18, 2025
rust-timer added a commit that referenced this pull request Jun 18, 2025
Rollup merge of #142618 - GuillaumeGomez:eslint-no-console, r=lolbinarycat

Lint about `console` calls in rustdoc JS

As discussed [here](#142100 (comment)), this PR enforces that `console` is not used in rustdoc JS by default.

cc `@lolbinarycat`
rust-timer added a commit that referenced this pull request Jun 18, 2025
Rollup merge of #142100 - lolbinarycat:rustdoc-srcIndex-138467, r=GuillaumeGomez

rustdoc: make srcIndex no longer a global variable

this is one-time initialization data, it can just
be a function parameter.

while we're doing that, we can more the json parsing into the function and save a few extra bytes of storage for free, at least in the case of multiple crates in a doc bundle.

fixes #138467
lnicola pushed a commit to lnicola/rust-analyzer that referenced this pull request Jun 18, 2025
…rycat

Lint about `console` calls in rustdoc JS

As discussed [here](rust-lang/rust#142100 (comment)), this PR enforces that `console` is not used in rustdoc JS by default.

cc `@lolbinarycat`
lnicola pushed a commit to lnicola/rust-analyzer that referenced this pull request Jun 18, 2025
Rollup of 14 pull requests

Successful merges:

 - rust-lang/rust#141574 (impl `Default` for `array::IntoIter`)
 - rust-lang/rust#141608 (Add support for repetition to `proc_macro::quote`)
 - rust-lang/rust#142100 (rustdoc: make srcIndex no longer a global variable)
 - rust-lang/rust#142371 (avoid `&mut P<T>` in `visit_expr` etc methods)
 - rust-lang/rust#142517 (Windows: Use anonymous pipes in Command)
 - rust-lang/rust#142520 (alloc: less static mut + some cleanup)
 - rust-lang/rust#142588 (Generic ctx imprv)
 - rust-lang/rust#142605 (Don't unwrap in enzyme builds in case of missing llvm-config)
 - rust-lang/rust#142608 (Refresh module-level docs for `rustc_target::spec`)
 - rust-lang/rust#142618 (Lint about `console` calls in rustdoc JS)
 - rust-lang/rust#142620 (Remove a panicking branch in `BorrowedCursor::advance`)
 - rust-lang/rust#142631 (Dont suggest remove semi inside macro expansion for redundant semi lint)
 - rust-lang/rust#142632 (Update cargo)
 - rust-lang/rust#142635 (Temporarily add back -Zwasm-c-abi=spec)

r? `@ghost`
`@rustbot` modify labels: rollup
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. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. T-rustdoc-frontend Relevant to the rustdoc-frontend team, which will review and decide on the web UI/UX output.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

src-script.js: make srcIndex into a parameter instead of a global variable
8 participants