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

Tracking Issue for Rustdoc Askama Migration #108868

Open
11 of 25 tasks
clubby789 opened this issue Mar 7, 2023 · 9 comments
Open
11 of 25 tasks

Tracking Issue for Rustdoc Askama Migration #108868

clubby789 opened this issue Mar 7, 2023 · 9 comments
Labels
A-rustdoc-ui Area: Rustdoc UI (generated HTML) C-tracking-issue Category: An issue tracking the progress of sth. like the implementation of an RFC E-help-wanted Call for participation: Help is requested to fix this issue. E-medium Call for participation: Medium difficulty. Experience needed to fix: Intermediate. E-mentor Call for participation: This issue has a mentor. Use #t-compiler/help on Zulip for discussion. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.

Comments

@clubby789
Copy link
Contributor

clubby789 commented Mar 7, 2023

Tracking Issue for Rustdoc Askama Migration

Zulip Discussion

This issue is to track the progress of migrating rustdoc's HTML rendering to Askama.

Suggested Workflow

If you'd like to work on a component, please leave a comment claiming it so that work doesn't conflict.
Work on translating the current series of format!() and string building calls into an HTML template. Where possible, try to keep as much logic as possible on the Rust side. Make sure to refer to STYLE.md for templating style, especially using comment blocks to minimize whitespace.

A specific example of a migration (and some style considerations) is here.

Components to Migrate

Implementation History

@rustbot label +T-rustdoc +A-rustdoc-ui

@clubby789 clubby789 added the C-tracking-issue Category: An issue tracking the progress of sth. like the implementation of an RFC label Mar 7, 2023
@rustbot rustbot added A-rustdoc-ui Area: Rustdoc UI (generated HTML) T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. labels Mar 7, 2023
@jyn514 jyn514 added E-mentor Call for participation: This issue has a mentor. Use #t-compiler/help on Zulip for discussion. E-help-wanted Call for participation: Help is requested to fix this issue. E-medium Call for participation: Medium difficulty. Experience needed to fix: Intermediate. labels Mar 7, 2023
@jsha
Copy link
Contributor

jsha commented Mar 9, 2023

I'm working on fn assoc_method and fn inner_full_print.

@clubby789
Copy link
Contributor Author

I'm working on fn print_src

@jsha
Copy link
Contributor

jsha commented Mar 15, 2023

As part of this, I'd like to get rid of struct Buffer and the f.alternate() method of emitting text instead of HTML.

Rationale: Using Formatter::alternate() to emit text instead of HTML was introduced to support line-wrapping function declarations over 80 characters. To this day, we only use f.alternate() when emitting function / method declarations, and we only care about the number of characters emitted.

Buffer was introduced as an alternative to f.alternate(): Instead of checking f.alternate() repeatedly throughout format.rs, the buffer itself could carry the information about whether we're outputting text or HTML. Removing the repeated checks of f.alternate() is a good goal - they significantly complicate format.rs. Also, Askama does not currently offer a way to access f.alternate() or format with {:#}, AFAICT. However, Buffer itself also doesn't compose well with Askama. Askama templates implement Display; ideally we would want a stack of structures, each potentially containing other structs that implement Display; then we can interpolate them directly into the parent template without excess allocations.

In the original PR adding f.alternate() support for the purpose of line wrapping, generating and then stripping HTML was considered and discarded. But I think it's the right solution here. Because we control the HTML and because we only care about counting the text output, we can get away with a very simplified HTML processor that knows how to strip tags and count entities (e.g. &) as a single character.

This is a summary of a similar comment I made on Zulip.

@jsha
Copy link
Contributor

jsha commented Mar 15, 2023

Update on the above: I missed a few places where we use {:#} and actually emit the plain output:

I still think it makes sense to filter internally-generated HTML with a limited tag stripper, but it looks like we'll need to be able to actually emit the text rather than just count it, which means processing the entities (no big deal).

@djc
Copy link
Contributor

djc commented Mar 16, 2023

FWIW, feel free to tag me in reviews if there are questions. Excited to see more usage of Askama in rustdoc!

As part of this, I'd like to get rid of struct Buffer and the f.alternate() method of emitting text instead of HTML.

As a potential point of interest, at work we have started using a very small procedural macro that effectively duplicates a template type to allow having two Templates wrapped around the same input type, one for plaintext and one for HTML. So this:

#[email(template = "contact-email")]
pub struct DomainContactEmail<'a> {
    pub domain: &'a str,
    pub sender: &'a ContactSender<'a>,
    pub message: &'a str,
    pub inbox_url: &'a str,
}

expands to this:

impl<'a: 'email, 'email> email::BodyTemplates<'email> for DomainContactEmail<'a> {
    type Text = DomainContactEmailText<'a, 'email>;
    type Html = DomainContactEmailHtml<'a, 'email>;
}
#[derive(askama::Template)]
#[template(path = "contact-email.txt")]
pub struct DomainContactEmailText<'a, 'email>(&'email DomainContactEmail<'a>);

impl<'a: 'email, 'email> From<&'email DomainContactEmail<'a>>
    for DomainContactEmailText<'a, 'email>
{
    fn from(email: &'email DomainContactEmail<'a>) -> Self {
        Self(email)
    }
}
impl<'a: 'email, 'email> std::ops::Deref for DomainContactEmailText<'a, 'email> {
    type Target = &'email DomainContactEmail<'a>;
    fn deref(&self) -> &Self::Target {
        &self.0
    }
}
#[derive(askama::Template)]
#[template(path = "contact-email.html")]
pub struct DomainContactEmailHtml<'a, 'email>(&'email DomainContactEmail<'a>);

impl<'a: 'email, 'email> std::ops::Deref for DomainContactEmailHtml<'a, 'email> {
    type Target = &'email DomainContactEmail<'a>;
    fn deref(&self) -> &Self::Target {
        &self.0
    }
}
impl<'a: 'email, 'email> From<&'email DomainContactEmail<'a>>
    for DomainContactEmailHtml<'a, 'email>
{
    fn from(email: &'email DomainContactEmail<'a>) -> Self {
        Self(email)
    }
}

Maybe this is used too much/duplicating the templates would be too much work for Rustdoc, just thought I'd mention it here as a point in the design space.

@jsha
Copy link
Contributor

jsha commented Mar 17, 2023

I'm working on changing primitive_link_fragment, href_relative_parts, and href_with_root_path to use a template to construct the hrefs with fewer allocations.

And thanks for the proc macro advice, and the kind offer of assistance, @djc!

Dylan-DPC added a commit to Dylan-DPC/rust that referenced this issue Mar 22, 2023
…meGomez

Render source page layout with Askama

~~I was looking at making `code_html` render into the buffer instead of in advance, but it turned out to need a pretty big refactor, so starting with rearranging the high-level layout.~~
Found another approach which required much less changes

cc rust-lang#108868
Dylan-DPC added a commit to Dylan-DPC/rust that referenced this issue Mar 22, 2023
…meGomez

Render source page layout with Askama

~~I was looking at making `code_html` render into the buffer instead of in advance, but it turned out to need a pretty big refactor, so starting with rearranging the high-level layout.~~
Found another approach which required much less changes

cc rust-lang#108868
aliemjay added a commit to aliemjay/rust that referenced this issue Apr 15, 2023
rustdoc: use a template to generate Hrefs

Previously, href always returned a string, so each link incurred at least one allocation. We were also using `"../".repeat()`, and `.collect()` into strings to generate part of the paths. This abstracts away those repetitions into a couple of helper structs, ParentDirectories and PathComponents, and puts them together into an `Href` template, which should save some allocations. As a side benefit, I think it makes the logic of constructing links a little clearer.

This removes the only call sites for certain methods of UrlPartsBuilder, so it also removes those methods.

Also, remove the ItemType return value from `href` and friends; it was basically unused.

Part of rust-lang#108868
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue May 26, 2023
…omez

Migrate `item_static` to Askama

This pull request addresses the type signature of the item_static function in our codebase. Previously, this function accepted a mutable reference to a Buffer for writing output. The current changes modify this to instead accept a mutable reference to any type that implements the Write trait.

Referes rust-lang#108868
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue May 27, 2023
…it, r=GuillaumeGomez

rustdoc: Add `ItemTemplate` trait and related functions to avoid repetitively wrapping existing functions

Context: rust-lang#111430 (comment)

This trait will be used extensively in performing migrations to Askama templates (tracking issue: rust-lang#108868)
fee1-dead added a commit to fee1-dead-contrib/rust that referenced this issue May 28, 2023
…llaumeGomez

Migrate `item_foreign_type` to Askama

This PR continues the migration of `print_item.rs` functions to Askama. This piece of work migrates the function `item_foreign_type`

Refers rust-lang#108868
Dylan-DPC added a commit to Dylan-DPC/rust that referenced this issue May 30, 2023
…llaumeGomez

Migrate  `item_proc_macro` to Askama

This PR migrates `item_proc_macro` to Askama

Refers rust-lang#108868
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue May 31, 2023
…llaumeGomez

Migrate  `item_proc_macro` to Askama

This PR migrates `item_proc_macro` to Askama

Refers rust-lang#108868
compiler-errors added a commit to compiler-errors/rust that referenced this issue Jun 2, 2023
…aumeGomez

Migrate `item_trait_alias` to Askama

This PR migrates `item_trait_alias` to Askama

Refers rust-lang#108868
bors added a commit to rust-lang-ci/rust that referenced this issue Jun 3, 2023
…uillaumeGomez

Migrate  `item_primitive` to Askama

This PR migrates `item_primitive` to Askama

Refers rust-lang#108868
GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this issue Jun 8, 2023
…laumeGomez

Migrate `item_opaque_ty` to Askama

This PR migrates `item_opaque_ty` to Askama

Refers: rust-lang#108868
RalfJung pushed a commit to RalfJung/miri that referenced this issue Jun 11, 2023
Migrate `item_trait_alias` to Askama

This PR migrates `item_trait_alias` to Askama

Refers rust-lang/rust#108868
RalfJung pushed a commit to RalfJung/miri that referenced this issue Jun 11, 2023
…omez

Migrate  `item_primitive` to Askama

This PR migrates `item_primitive` to Askama

Refers rust-lang/rust#108868
GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this issue Aug 16, 2023
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Aug 16, 2023
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Aug 16, 2023
RalfJung pushed a commit to RalfJung/rust-analyzer that referenced this issue Apr 20, 2024
…omez

Migrate  `item_primitive` to Askama

This PR migrates `item_primitive` to Askama

Refers rust-lang/rust#108868
RalfJung pushed a commit to RalfJung/rust-analyzer that referenced this issue Apr 27, 2024
…omez

Migrate  `item_primitive` to Askama

This PR migrates `item_primitive` to Askama

Refers rust-lang/rust#108868
@TmLev
Copy link
Contributor

TmLev commented Jun 10, 2024

@clubby789 can I please ask you to update the current status of components?
Judging by PRs merged, these components have been migrated: static items, trait aliases, proc macros, primitives, foreign types, opaque types.

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) C-tracking-issue Category: An issue tracking the progress of sth. like the implementation of an RFC E-help-wanted Call for participation: Help is requested to fix this issue. E-medium Call for participation: Medium difficulty. Experience needed to fix: Intermediate. E-mentor Call for participation: This issue has a mentor. Use #t-compiler/help on Zulip for discussion. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

7 participants