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

Fix index-page generation #60134

Merged
merged 3 commits into from
Apr 26, 2019
Merged

Conversation

GuillaumeGomez
Copy link
Member

Fixes #60096.

The minifier was minifying crates name in searchIndex key position, which was a bit problematic for multiple reasons.

r? @rust-lang/rustdoc

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Apr 20, 2019
@bors
Copy link
Contributor

bors commented Apr 23, 2019

☔ The latest upstream changes (presumably #60140) made this pull request unmergeable. Please resolve the merge conflicts.

@GuillaumeGomez
Copy link
Member Author

This one is quite a huge issue so it'd be nice to have it reviewed as soon as possible. :-/

r? @Manishearth

f,
"R",
Token::Char(ReservedChar::Backline),
|tokens, pos| {
pos < 2 ||
!tokens[pos - 1].is_char(ReservedChar::OpenBracket) ||
Copy link
Member

Choose a reason for hiding this comment

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

please leave a comment here explaining why this is necessary

@ollie27
Copy link
Member

ollie27 commented Apr 23, 2019

At the very least this needs a test.

@GuillaumeGomez
Copy link
Member Author

@ollie27 To test it, we need two things:

  1. A test to check output JS (which must work, whatever the minification used)
  2. To compile multiple crates at once since this bug only occurs at this time.

I can do it, but considering the amount of code necessary, this promise to be complicated to maintain...

@ollie27
Copy link
Member

ollie27 commented Apr 24, 2019

Changing src/test/rustdoc/index-page.rs to:

// aux-build:all-item-types.rs
// build-aux-docs
// compile-flags: -Z unstable-options --enable-index-page

#![crate_name = "foo"]

// @has foo/../index.html
// @has - '//span[@class="in-band"]' 'List of all crates'
// @has - '//ul[@class="mod"]//a[@href="foo/index.html"]' 'foo'
// @has - '//ul[@class="mod"]//a[@href="all_item_types/index.html"]' 'all_item_types'
pub struct Foo;

is enough to test for #60096.

Also now that crate names are no longer minified this code should be removed:

// We need to check if the crate name has been put into a variable as well.
let tokens: js::Tokens<'_> = js::simple_minify(&line)
.into_iter()
.filter(js::clean_token)
.collect::<Vec<_>>()
.into();
let mut pos = 0;
while pos < tokens.len() {
if let Some((var_pos, Some(value_pos))) =
js::get_variable_name_and_value_positions(&tokens, pos) {
if let Some(s) = tokens.0[value_pos].get_string() {
if &s[1..s.len() - 1] == krate {
if let Some(var) = tokens[var_pos].get_other() {
krate = var.to_owned();
break
}
}
}
}
pos += 1;
}

@GuillaumeGomez
Copy link
Member Author

@ollie27: Good catch! Completely forgot about this part...

@GuillaumeGomez
Copy link
Member Author

Updated!

f,
"R",
Token::Char(ReservedChar::Backline),
// This closure prevents crates' name to be aggregated. It allows to not
Copy link
Member

Choose a reason for hiding this comment

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

"This closure prevents crates' names from being aggregated."

I'm not sure what the second sentence means. And this doesn't explain how the OpenBracket or searchIndex check work.

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'm not sure what the second sentence means. And this doesn't explain how the OpenBracket or searchIndex check work.

Ah? I'll reword it then...

@GuillaumeGomez
Copy link
Member Author

@Manishearth Are the explanations more clear this way or is there something else I need to clarify?

@Manishearth
Copy link
Member

@bors r+

thanks!

@bors
Copy link
Contributor

bors commented Apr 25, 2019

📌 Commit 6aa5a5d has been approved by Manishearth

@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 25, 2019
Centril added a commit to Centril/rust that referenced this pull request Apr 25, 2019
…nishearth

Fix index-page generation

Fixes rust-lang#60096.

The minifier was minifying crates name in `searchIndex` key position, which was a bit problematic for multiple reasons.

r? @rust-lang/rustdoc
Centril added a commit to Centril/rust that referenced this pull request Apr 25, 2019
…nishearth

Fix index-page generation

Fixes rust-lang#60096.

The minifier was minifying crates name in `searchIndex` key position, which was a bit problematic for multiple reasons.

r? @rust-lang/rustdoc
Centril added a commit to Centril/rust that referenced this pull request Apr 26, 2019
…nishearth

Fix index-page generation

Fixes rust-lang#60096.

The minifier was minifying crates name in `searchIndex` key position, which was a bit problematic for multiple reasons.

r? @rust-lang/rustdoc
bors added a commit that referenced this pull request Apr 26, 2019
Rollup of 12 pull requests

Successful merges:

 - #59734 (Prevent failure in case no space left on device in rustdoc)
 - #59940 (Set cfg(test) when rustdoc is running with --test option)
 - #60134 (Fix index-page generation)
 - #60165 (Add Pin::{into_inner,into_inner_unchecked})
 - #60183 (Chalkify: Add builtin Copy/Clone)
 - #60225 (Introduce hir::ExprKind::Use and employ in for loop desugaring.)
 - #60247 (Implement Debug for Place using Place::iterate)
 - #60259 (Derive Default instead of new in applicable lint)
 - #60267 (Add feature-gate for f16c target feature)
 - #60284 (Do not allow const generics to depend on type parameters)
 - #60285 (Do not ICE when checking types against foreign fn)
 - #60289 (Make `-Z allow-features` work for stdlib features)

Failed merges:

r? @ghost
@bors
Copy link
Contributor

bors commented Apr 26, 2019

⌛ Testing commit 6aa5a5d with merge 180edc2...

@bors bors merged commit 6aa5a5d into rust-lang:master Apr 26, 2019
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.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

rustdoc --enable-index-page broken
5 participants