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.

Sign up for GitHub

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

Jump to bottom

Improve CSS for "hide item contents, not items" #1

Merged
merged 1 commit into from
Mar 28, 2021
Merged

Improve CSS for "hide item contents, not items" #1

merged 1 commit into from
Mar 28, 2021

Conversation

jsha
Copy link

@jsha jsha commented Mar 27, 2021 •
edited
Loading

Introduce a first use of the <details> and <summary> tags as
replacements for the JS-built toggles. I think this has the potential to
replace all the JS toggles and generally clean up the JS, CSS, and HTML.

Split rendering of attributes into two cases: in the case where they are
rendered as descendents of a <pre> tag, where they use indent spaces and
newlines for formatting, matching their surrounding markup. In the case
where they are rendered as descendants of a <code> tag, they are
rendered as <div>. This let me clean up some fragile CSS that was
adjusting the margin-left of attributes depending on context.

Remove toggles for attributes. With the ALLOWED_ATTRIBUTES filter, it's
rare for an item to have more than one attribute, so hiding attributes
behind a toggle doesn't save any screen space in the common case.

Fix a couple of invocations of matches! that didn't compile on my
machine.

Restore a conditional for the JS createToggle call that was causing
"Expand description" to show up spuriously on already-expanded
descriptions.

Add JS for auto-hide settings and hide all / show all.

Remove a z-index property and some font color tweaks made unnecessary
by the <details> toggles.

Add CSS for the <details> toggles.

Examples at:

https://hoffman-andrews.com/rust/item-hide/std/iter/trait.Iterator.html
https://hoffman-andrews.com/rust/item-hide/std/convert/trait.Into.html
https://hoffman-andrews.com/rust/item-hide/std/io/trait.Read.html
https://hoffman-andrews.com/rust/item-hide/std/option/enum.Option.html

Addition to rust-lang#83337

This was referenced Mar 27, 2021
Merged
Closed
@jsha jsha force-pushed the manishearth-item-hide branch from 4e24980 to 7cd0306 Compare March 27, 2021 22:42
@jsha
Improve CSS for "hide contents, not items"
79bd6be 
Introduce a first use of the `<details>` and `<summary>` tags as
replacements for the JS-built toggles. I think this has the potential to
replace all the JS toggles and generally clean up the JS, CSS, and HTML.

Split rendering of attributes into two cases: in the case where they are
rendered as descendents of a `<pre>` tag, where they use indent spaces and
newlines for formatting, matching their surrounding markup. In the case
where they are rendered as descendants of a `<code>` tag, they are
rendered as `<div>`. This let me clean up some fragile CSS that was
adjusting the margin-left of attributes depending on context.

Remove toggles for attributes. With the ALLOWED_ATTRIBUTES filter, it's
rare for an item to have more than one attribute, so hiding attributes
behind a toggle doesn't save any screen space in the common case.

Fix a couple of invocations of `matches!` that didn't compile on my
machine.

Fix a boolean for the JS `createToggle` call that was causing
"Expand description" to show up spuriously on already-expanded
descriptions.

Add JS for auto-hide settings and hide all / show all.

Remove a z-index property and some font color tweaks made unnecessary
by the <details> toggles.

Add CSS for the <details> toggles.
@jsha jsha force-pushed the manishearth-item-hide branch from 7cd0306 to 79bd6be Compare March 27, 2021 23:13
Manishearth
Manishearth reviewed Mar 28, 2021
View reviewed changes
Copy link
Owner

@Manishearth Manishearth left a comment

Choose a reason for hiding this comment

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

Haven't had a close look but mostly looks good so far, feel free to make these changes if I don't end up doing so first

src/librustdoc/html/render/mod.rs
// a div to produce a newline after it.
fn render_attributes_in_code(w: &mut Buffer, it: &clean::Item) {
for a in attributes(it) {
write!(w, "<div>{}</div>", a);
Copy link
Owner

Choose a reason for hiding this comment

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

we should probably give this a class

src/librustdoc/html/static/rustdoc.css
/* This part is to fix the "Expand attributes" part in the type declaration. */
.type-decl > pre > .toggle-wrapper.toggle-attributes.top-attr {
margin-left: 0 !important;
/* The hideme class is used on summary tags that contain a span with
Copy link
Owner

Choose a reason for hiding this comment

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

People use <details> in doc contents as well, we absolutely should be using details.someclass here

@Manishearth Manishearth merged this pull request into Manishearth:item-hide Mar 28, 2021
Manishearth pushed a commit that referenced this pull request Aug 26, 2021
@ldm0 @nikomatsakis
select obligations after check_casts
e891044 
Otherwise, we can get into a situation where you have
a subtype obligation `#1 <: rust-lang#2` pending, #1 is constrained
by `check_casts`, but rust-lang#2` is unaffected.

Co-authored-by: Niko Matsakis <niko@alum.mit.edu>
Manishearth pushed a commit that referenced this pull request May 2, 2023
@john-h-k
Comment round #1
cffc10b
Manishearth pushed a commit that referenced this pull request May 24, 2023
@matthiaskrgr
Rollup merge of rust-lang#111039 - compiler-errors:foreign-span-rpiti…
b194b43 
…t, r=tmiasko

Encode def span for foreign return-position `impl Trait` in trait

Fixes rust-lang#111031, yet another def-span encoding issue :/

Includes a smaller repro than the issue, but I can confirm it ICEs:

```
query stack during panic:
#0 [def_span] looking up span for `rpitit::Foo::bar::{opaque#0}`
#1 [object_safety_violations] determining object safety of trait `rpitit::Foo`
rust-lang#2 [check_is_object_safe] checking if trait `rpitit::Foo` is object safe
rust-lang#3 [typeck] type-checking `main`
rust-lang#4 [used_trait_imports] finding used_trait_imports `main`
rust-lang#5 [analysis] running analysis passes on this crate
```

Luckily since this only affects nightly, this desn't need to be backported.
Manishearth pushed a commit that referenced this pull request May 24, 2023
@matthiaskrgr
Rollup merge of rust-lang#110610 - spastorino:smir-terminator, r=oli-obk
77004ea 
Add Terminator conversion from MIR to SMIR, part #1

This adds internal MIR TerminatorKind to SMIR Terminator conversion.

r? ```@oli-obk```
Manishearth pushed a commit that referenced this pull request May 24, 2023
@bors
Auto merge of rust-lang#111287 - matthiaskrgr:rollup-9lzax2c, r=matth…
905d5a3 
…iaskrgr

Rollup of 7 pull requests

Successful merges:

 - rust-lang#110577 (Use fulfillment to check `Drop` impl compatibility)
 - rust-lang#110610 (Add Terminator conversion from MIR to SMIR, part #1)
 - rust-lang#110985 (Fix spans in LLVM-generated inline asm errors)
 - rust-lang#110989 (Make the BUG_REPORT_URL configurable by tools )
 - rust-lang#111167 (debuginfo: split method declaration and definition)
 - rust-lang#111230 (add hint for =< as <=)
 - rust-lang#111279 (More robust debug assertions for `Instance::resolve` on built-in traits with non-standard trait items)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
Manishearth pushed a commit that referenced this pull request May 24, 2023
@bors
Auto merge of rust-lang#111568 - scottmcm:undo-opt, r=WaffleLapkin
e9e1bbc 
Stop turning transmutes into discriminant reads in mir-opt

Partially reverts rust-lang#109612, as after rust-lang#109993 these aren't actually equivalent any more, and I'm no longer confident this was ever an improvement in the first place.

Having this "simplification" meant that similar-looking code actually did somewhat different things.  For example,
```rust
pub unsafe fn demo1(x: std::cmp::Ordering) -> u8 {
    std::mem::transmute(x)
}
pub unsafe fn demo2(x: std::cmp::Ordering) -> i8 {
    std::mem::transmute(x)
}
```
in nightly today is generating <https://rust.godbolt.org/z/dPK58zW18>
```llvm
define noundef i8 `@_ZN7example5demo117h341ef313673d2ee6E(i8` noundef %x) unnamed_addr #0 {
  %0 = icmp uge i8 %x, -1
  %1 = icmp ule i8 %x, 1
  %2 = or i1 %0, %1
  call void `@llvm.assume(i1` %2)
  ret i8 %x
}

define noundef i8 `@_ZN7example5demo217h5ad29f361a3f5700E(i8` noundef %0) unnamed_addr #0 {
  %x = alloca i8, align 1
  store i8 %0, ptr %x, align 1
  %1 = load i8, ptr %x, align 1, !range !2, !noundef !3
  ret i8 %1
}
```

Which feels too different when the original code is essentially identical.

---

Aside: that example is different *after* optimizations too:
```llvm
define noundef i8 `@_ZN7example5demo117h341ef313673d2ee6E(i8` noundef returned %x) unnamed_addr #0 {
  %0 = add i8 %x, 1
  %1 = icmp ult i8 %0, 3
  tail call void `@llvm.assume(i1` %1)
  ret i8 %x
}

define noundef i8 `@_ZN7example5demo217h5ad29f361a3f5700E(i8` noundef returned %0) unnamed_addr #1 {
  ret i8 %0
}
```
so turning the `Transmute` into a `Discriminant` was arguably just making things worse, so leaving it alone instead -- and thus having less code in rustc -- seems clearly better.
Manishearth pushed a commit that referenced this pull request May 24, 2023
@rcvalle
CFI: Fix encode_ty: unexpected Param(B/#1)
f067935 
Fixes rust-lang#111510 and complements rust-lang#106547 by adding support for encoding
type parameters and also by transforming trait objects' traits into
their identities before emitting type checks.
Manishearth pushed a commit that referenced this pull request May 24, 2023
@bors
Auto merge of rust-lang#111697 - rcvalle:rust-cfi-fix-111510, r=bjorn3
d77014a 
CFI: Fix encode_ty: unexpected Param(B/#1)

Fixes rust-lang#111510 and complements rust-lang#106547 by adding support for encoding type parameters and also by transforming trait objects' traits into their identities before emitting type checks.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Manishearth Manishearth Manishearth left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@jsha @Manishearth