rustdoc: migrate `document_type_layout` to askama by notriddle · Pull Request #109949 · rust-lang/rust

notriddle · 2023-04-04T21:22:46Z

No description provided.

rustbot · 2023-04-04T21:22:51Z

(rustbot has picked a reviewer for you, use r? to override)

camelid

Since I implemented the original version of this function, I thought reviewing this PR could be a good way to start getting familiar with Askama. :)

camelid · 2023-04-04T23:34:36Z

src/librustdoc/html/templates/type_layout.html

What's the meaning of the {# #} here and below? According to Askama's docs, it's a block comment?

It's a block comment, yes.

The point of it is to eat the whitespace between the end of this tag and the start of the one on the next line (it would normally be preserved).

camelid · 2023-04-04T23:35:56Z

src/librustdoc/html/templates/type_layout.html

According to Askama's docs, the {#+ #} keeps whitespace, but don't we want to suppress it to reduce page size?

If it was {# #}, I'd wind up with <strong>completelyunstable</strong>. If it was completely absent, I'd wind up with the newline and several spaces in a row between the two words.

src/librustdoc/html/render/print_item.rs

bors · 2023-04-09T01:19:08Z

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

jsha

Thanks for working on this! Sorry I was slow to get back to you.

src/librustdoc/html/render/print_item.rs

src/librustdoc/html/templates/type_layout.html

notriddle · 2023-04-19T02:24:59Z

@jsha These all seem like good ideas, and I've pushed a new version of the commit that makes these changes.

jsha

Generally looks great! Just a style question about nesting lets in scopes. And, relatedly: why nest the struct definitions inside document_type_layout? It winds up making the function body harder to navigate. If we're concerned about polluting scopes, maybe it would be better to use a module? That is, have render/type_layout.rs, with document_type_layout as the only public item, and with struct TypeLayout and struct TypeLayoutSize at the top level of that module.

src/librustdoc/html/render/print_item.rs

notriddle · 2023-04-21T16:04:28Z

The nested let bindings aren't really necessary. I've pushed a new commit to get rid of them.

The nested struct is following the pattern set in

rust/src/librustdoc/html/sources.rs

Lines 294 to 312 in 4212c1b

    
           /// Wrapper struct to render the source code of a file. This will do things like 
        
           /// adding line numbers to the left-hand side. 
        
           pub(crate) fn print_src( 
        
               buf: &mut Buffer, 
        
               s: &str, 
        
               file_span: rustc_span::Span, 
        
               context: &Context<'_>, 
        
               root_path: &str, 
        
               decoration_info: highlight::DecorationInfo, 
        
               source_context: SourceContext, 
        
           ) { 
        
               #[derive(Template)] 
        
               #[template(path = "source.html")] 
        
               struct Source<Code: std::fmt::Display> { 
        
                   embedded: bool, 
        
                   needs_expansion: bool, 
        
                   lines: RangeInclusive<usize>, 
        
                   code_html: Code, 
        
               }

src/librustdoc/html/render/print_item.rs

jsha

Looks great. r=me when tests pass. Thanks!

notriddle · 2023-04-21T18:42:20Z

@bors r=jsha rollup

bors · 2023-04-21T18:42:22Z

📌 Commit e6664c0 has been approved by jsha

It is now in the queue for this repository.

Rollup of 7 pull requests Successful merges: - rust-lang#109949 (rustdoc: migrate `document_type_layout` to askama) - rust-lang#110622 (Stable hash tag (discriminant) of `GenericArg`) - rust-lang#110635 (More `IS_ZST` in `library`) - rust-lang#110640 (compiler/rustc_target: Raise m68k-linux-gnu baseline to 68020) - rust-lang#110657 (nit: consistent naming for SimplifyConstCondition) - rust-lang#110659 (rustdoc: clean up JS) - rust-lang#110660 (Print ty placeholders pretty) Failed merges: r? `@ghost` `@rustbot` modify labels: rollup

rustbot assigned jsha Apr 4, 2023

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. labels Apr 4, 2023

camelid reviewed Apr 5, 2023

View reviewed changes

camelid assigned camelid and unassigned jsha Apr 5, 2023

camelid 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-review Status: Awaiting review from the assignee but also interested parties. labels Apr 5, 2023

notriddle added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Apr 5, 2023

rustdoc: migrate document_type_layout to askama

08f204e

notriddle force-pushed the notriddle/type-layout branch from ae57609 to 08f204e Compare April 9, 2023 02:24

jsha suggested changes Apr 18, 2023

View reviewed changes

notriddle added 3 commits April 18, 2023 10:21

rustdoc: use a separate template for type layout size

5f9746b

rustdoc: create variants list outside of template

3a16db1

rustdoc: format type layout template with newline after <p>

e26ae95

notriddle force-pushed the notriddle/type-layout branch from ab8ea2b to e26ae95 Compare April 18, 2023 18:14

notriddle assigned jsha Apr 18, 2023

jsha reviewed Apr 21, 2023

View reviewed changes

src/librustdoc/html/render/print_item.rs Outdated Show resolved Hide resolved

rustdoc: get rid of redundant, nested let lines

99e1cdb

This comment has been minimized.

Sign in to view

jsha suggested changes Apr 21, 2023

View reviewed changes

src/librustdoc/html/render/print_item.rs Outdated Show resolved Hide resolved

src/librustdoc/html/render/print_item.rs Outdated Show resolved Hide resolved

notriddle added 2 commits April 21, 2023 11:04

rustdoc: factor document_type_layout into its own module

2b728c1

rustdoc: remove unnecessary binding

e6664c0

notriddle force-pushed the notriddle/type-layout branch from 097a299 to e6664c0 Compare April 21, 2023 18:06

jsha approved these changes Apr 21, 2023

View reviewed changes

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 21, 2023

JohnTitor mentioned this pull request Apr 22, 2023

Rollup of 7 pull requests #110666

Merged

bors merged commit 684bdf8 into rust-lang:master Apr 22, 2023

rustbot added this to the 1.71.0 milestone Apr 22, 2023

notriddle deleted the notriddle/type-layout branch April 22, 2023 17:49

Uh oh!

Conversation

notriddle commented Apr 4, 2023

Uh oh!

rustbot commented Apr 4, 2023

Uh oh!

camelid left a comment

Choose a reason for hiding this comment

Uh oh!

camelid Apr 4, 2023

Choose a reason for hiding this comment

Uh oh!

notriddle Apr 5, 2023

Choose a reason for hiding this comment

Uh oh!

camelid Apr 4, 2023

Choose a reason for hiding this comment

Uh oh!

notriddle Apr 5, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

bors commented Apr 9, 2023

Uh oh!

jsha left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

notriddle commented Apr 19, 2023

Uh oh!

jsha left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

notriddle commented Apr 21, 2023

Uh oh!

This comment has been minimized.

Uh oh!

Uh oh!

jsha left a comment

Choose a reason for hiding this comment

Uh oh!

notriddle commented Apr 21, 2023

Uh oh!

bors commented Apr 21, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

notriddle Apr 5, 2023 •

edited

Loading