-
Notifications
You must be signed in to change notification settings - Fork 13.4k
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
Conversation
r? @notriddle rustbot has assigned @notriddle. Use |
Some changes occurred in HTML/CSS/JS. cc @GuillaumeGomez, @jsha, @lolbinarycat Some changes occurred in GUI tests. |
This comment has been minimized.
This comment has been minimized.
80d65ae
to
1f4435a
Compare
Seems good to me, thanks! Please remove the left-over debugging and then I'll approve. |
tests/rustdoc-gui/globals.goml
Outdated
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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)
@GuillaumeGomez debug print has been removed. |
Thanks! Considering the second commit is the removal of a forgotten debug log, please squash it. Then I'll r+ the PR. |
6cecd76
to
4236f4b
Compare
This comment has been minimized.
This comment has been minimized.
4236f4b
to
f62e2de
Compare
@GuillaumeGomez squashed and rebased |
This comment has been minimized.
This comment has been minimized.
sigh rebased this onto a broken master, gotta fix... |
f62e2de
to
a07ddd9
Compare
@GuillaumeGomez ok, rebased properly this time. |
Perfect, thanks! r=me once CI pass. @bors delegate |
@bors delegate+ |
✌️ @lolbinarycat, you can now approve this pull request! If @GuillaumeGomez told you to " |
* @param {string} srcIndexStr - strinified json map from crate name to dir structure | ||
*/ | ||
function createSrcSidebar(srcIndexStr) { | ||
console.log(srcIndexStr); |
There was a problem hiding this comment.
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.log
s... might be an easy and helpful change to enable that lint.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
a07ddd9
to
00c1042
Compare
…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
@bors r- |
Double-checked the @bors r+ rollup |
…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`
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
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`
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
…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`
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
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