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 "run" button position in error index #82979

Merged
merged 1 commit into from
Mar 12, 2021

Conversation

GuillaumeGomez
Copy link
Member

This isn't really a rustdoc issue but I still made the same fix in the rustdoc.css file (doesn't hurt).

Before:

Screenshot from 2021-03-10 16-35-49

After:

Screenshot from 2021-03-10 16-40-08

cc @jyn514 (considering this is quite a big bug and an easy fix)
r? @Nemo157

@GuillaumeGomez GuillaumeGomez added T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. A-rustdoc-ui Area: Rustdoc UI (generated HTML) labels Mar 10, 2021
@rust-highfive
Copy link
Collaborator

Some changes occurred in HTML/CSS/JS.

cc @GuillaumeGomez

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Mar 10, 2021
@Nemo157
Copy link
Member

Nemo157 commented Mar 11, 2021

@bors r+

@bors
Copy link
Contributor

bors commented Mar 11, 2021

📌 Commit a4d7046 has been approved by Nemo157

@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 Mar 11, 2021
@GuillaumeGomez
Copy link
Member Author

@rust-lang/infra Once this fix is merged, can we update the current file for the error index for the current stable, beta and nightly please? (We can even update further back, the issue remains)

@jyn514
Copy link
Member

jyn514 commented Mar 11, 2021

This isn't really a rustdoc issue but I still made the same fix in the rustdoc.css file (doesn't hurt).

If it's not a rustdoc issue, then what is it?

@GuillaumeGomez
Copy link
Member Author

As you can see, they don't use the rustdoc CSS on https://doc.rust-lang.org/nightly/error-index.html

@jyn514
Copy link
Member

jyn514 commented Mar 11, 2021

I don't understand. What bug is the rustdoc change fixing? Does it not use the rust.css file?

@GuillaumeGomez
Copy link
Member Author

run.css is a short version of rustdoc.css, but it hasn't been updated in a long time. As for the change in rustdoc.css, it's mostly "just in case".

Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request Mar 11, 2021
…mo157

Fix "run" button position in error index

This isn't really a rustdoc issue but I still made the same fix in the `rustdoc.css` file (doesn't hurt).

Before:

![Screenshot from 2021-03-10 16-35-49](https://user-images.githubusercontent.com/3050060/110655807-aa402800-81bf-11eb-8a88-bc979efd1697.png)

After:

![Screenshot from 2021-03-10 16-40-08](https://user-images.githubusercontent.com/3050060/110655843-b4622680-81bf-11eb-8670-42975d92b4eb.png)

cc `@jyn514` (considering this is quite a big bug and an easy fix)
r? `@Nemo157`
JohnTitor added a commit to JohnTitor/rust that referenced this pull request Mar 11, 2021
…mo157

Fix "run" button position in error index

This isn't really a rustdoc issue but I still made the same fix in the `rustdoc.css` file (doesn't hurt).

Before:

![Screenshot from 2021-03-10 16-35-49](https://user-images.githubusercontent.com/3050060/110655807-aa402800-81bf-11eb-8a88-bc979efd1697.png)

After:

![Screenshot from 2021-03-10 16-40-08](https://user-images.githubusercontent.com/3050060/110655843-b4622680-81bf-11eb-8670-42975d92b4eb.png)

cc ``@jyn514`` (considering this is quite a big bug and an easy fix)
r? ``@Nemo157``
JohnTitor added a commit to JohnTitor/rust that referenced this pull request Mar 11, 2021
…mo157

Fix "run" button position in error index

This isn't really a rustdoc issue but I still made the same fix in the `rustdoc.css` file (doesn't hurt).

Before:

![Screenshot from 2021-03-10 16-35-49](https://user-images.githubusercontent.com/3050060/110655807-aa402800-81bf-11eb-8a88-bc979efd1697.png)

After:

![Screenshot from 2021-03-10 16-40-08](https://user-images.githubusercontent.com/3050060/110655843-b4622680-81bf-11eb-8670-42975d92b4eb.png)

cc ```@jyn514``` (considering this is quite a big bug and an easy fix)
r? ```@Nemo157```
bors added a commit to rust-lang-ci/rust that referenced this pull request Mar 12, 2021
Rollup of 11 pull requests

Successful merges:

 - rust-lang#80385 (Clarify what `Cell::replace` returns)
 - rust-lang#82571 (Rustdoc Json: Add tests for Reexports, and improve jsondocck)
 - rust-lang#82860 (Add `-Z unpretty` flag for the THIR)
 - rust-lang#82950 (convert slice doc link to intra-doc links)
 - rust-lang#82965 (Add spirv extension handling in compiletest)
 - rust-lang#82966 (update MSYS2 link in README)
 - rust-lang#82979 (Fix "run" button position in error index)
 - rust-lang#83001 (Ignore Vim swap files)
 - rust-lang#83003 (rustdoc: tweak the search index format)
 - rust-lang#83013 (Adjust some `#[cfg]`s to take non-Unix non-Windows operating systems into account)
 - rust-lang#83018 (Reintroduce accidentally deleted assertions.)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 6ea1685 into rust-lang:master Mar 12, 2021
@rustbot rustbot added this to the 1.52.0 milestone Mar 12, 2021
@GuillaumeGomez GuillaumeGomez deleted the run-button-pos branch March 12, 2021 08:54
@GuillaumeGomez
Copy link
Member Author

ping @rust-lang/infra Can update the rust.css on https://doc.rust-lang.org/error-index.html (for both stable and beta) please?

@Mark-Simulacrum
Copy link
Member

I admit to being a bit confused by that - why would we do so out of band? If you want this on beta then a beta backport seems like the right strategy? (Likewise for stable though we're unlikely to have a point release this late in the cycle)

@GuillaumeGomez
Copy link
Member Author

This is a weird situation considering that the css file I fixed is kinda "on its own".

@Mark-Simulacrum
Copy link
Member

I don't understand. Is it not shipped with Rust? If it's not, then we need to update nightly too, right? I didn't think we had files not shipped with Rust - how does e.g. the error index work locally then?

@GuillaumeGomez
Copy link
Member Author

I mean: the rust.css file used only for the error index doesn't follow the rustdoc update, so it might have been broken for quite a lot of time. So I guess that when we make a new release, this file is part of it, but considering how special it is, we need to "backport" this fix to stable and beta (of the error index only).

@Mark-Simulacrum
Copy link
Member

So I guess that when we make a new release, this file is part of it

This tells me that... we need to beta-nominate this PR? (And potentially stable-nominate, but as I said, it's unlikely that there will be a stable point release this cycle).

If we're in agreement (as far as my reading of your comments goes) that the file would get updated with a beta backport, which seems definitively true - otherwise the error index wouldn't find it when viewed locally, which it does -- then the correct way to update it is to let this PR roll out as part of our release train. If we consider it a critical fix, then a beta backport seems like the right step, not an ad-hoc process.

You say "how special it is" but I admit to not seeing anything special about it - can you clarify? Why is this file special compared to all the other files? How does this impede using the standard process for beta and stable backports, but apparently not nightly?

@GuillaumeGomez
Copy link
Member Author

I would have hoped to at least fix the current stable but beta is already that...

You say "how special it is" but I admit to not seeing anything special about it - can you clarify? Why is this file special compared to all the other files? How does this impede using the standard process for beta and stable backports, but apparently not nightly?

This file is special in the way that it's a shortened version of the rustdoc.css file. Meaning that even though it is generated using rustdoc, it doesn't fully use the rustdoc CSS changes because it uses its own CSS file (which is, in my opinion, not great). Like I said, the file is released the same as all the others, but I didn't expect that we didn't use the rustdoc.css file here, which is problematic because any change in rustdoc DOM might break the error index without us knowing about it. It's only by "luck" that I discovered this issue...

Anyway, I let you put the tags?

@Mark-Simulacrum
Copy link
Member

So the file (rust.css) is not at all special with regards to distribution? Why did you ping the infra team specially then? If there's something that should be done in an ad-hoc way, then we should do it, but my current understanding is that even if I was to replace the files manually in the S3 bucket, it wouldn't actually help -- it would get replace by the standard deploy anyway.

@Mark-Simulacrum Mark-Simulacrum added the beta-nominated Nominated for backporting to the compiler in the beta channel. label Mar 12, 2021
@GuillaumeGomez
Copy link
Member Author

It might be interesting to understand why the error index is using a special file though. This was for this reason that I pinged the infra team originally.

@Mark-Simulacrum
Copy link
Member

Well, I don't think the infra team knows the answer to that question. Maybe git commit history can yield something if you looked; I'd guess it's just historical artifact.

@pnkfelix
Copy link
Member

pnkfelix commented Mar 18, 2021

discussed at compiler team meeting. since this seems like its been broken like this for ages, and there is no ticket filed discussing the breakage, then it seems bad to beta backport a week before the release.

(Yes change seems clearly low risk, but approval sets a bad precedent given the (lack of) context.)

Declining to beta backport. (likewise for stable etc)

@pnkfelix pnkfelix removed the beta-nominated Nominated for backporting to the compiler in the beta channel. label Mar 18, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-rustdoc-ui Area: Rustdoc UI (generated HTML) 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.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants