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

Migrate item_foreign_type to Askama #112033

Closed
wants to merge 2 commits into from

Conversation

sladyn98
Copy link
Contributor

This PR migrates item_foreign_type to Askama
Refers : #108868

@rustbot
Copy link
Collaborator

rustbot commented May 28, 2023

r? @GuillaumeGomez

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

@rustbot 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 May 28, 2023
render_attributes_in_code(w, it, cx.tcx());
fn item_foreign_type(w: &mut impl fmt::Write, cx: &mut Context<'_>, it: &clean::Item) {
let mut buffer = Buffer::new();
wrap_item(&mut buffer, |buffer| {
Copy link
Member

Choose a reason for hiding this comment

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

Let's wait for #112030 (comment) to be solved first.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Here render attributes expects a buffer so we might have to create one here or change the implementation of render_attributes in code to take both a buffer and an impl write

render_attributes_in_code(w, it, cx.tcx());
     |         ------------------------- ^ expected `&mut Buffer`, found `&mut impl fmt::Write`
     |         |
     |         arguments to this function are incorrect
     |

How we could modify render_attributess_in_code

enum WriteWrapper<'a> {
    Buffer(&'a mut html::format::Buffer),
    Writer(&'a mut dyn fmt::Write),
}

impl fmt::Write for WriteWrapper<'_> {
    fn write_str(&mut self, s: &str) -> fmt::Result {
        match self {
            WriteWrapper::Buffer(buffer) => buffer.write_str(s),
            WriteWrapper::Writer(writer) => writer.write_str(s),
        }
    }
}

fn render_attributes_in_code(w: &mut WriteWrapper<'_>, it: &clean::Item, tcx: TyCtxt<'_>) {
    for a in it.attributes(tcx, false) {
        write!(w, "<div class=\"code-attribute\">{}</div>", a);
    }
}

Copy link
Member

Choose a reason for hiding this comment

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

Or you can update render_attributes_in_code to make it take a impl Write instead of Buffer. ;)

fn render_attributes_in_code(w: &mut Buffer, it: &clean::Item, tcx: TyCtxt<'_>) {
    for a in it.attributes(tcx, false) {
        write!(w, "<div class=\"code-attribute\">{}</div>", a);
    }
}

Copy link
Member

Choose a reason for hiding this comment

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

(Unless I missed something obvious, Buffer implements Write so it should be the only change required?)

Copy link
Contributor

Choose a reason for hiding this comment

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

We can actually remove the use of Buffer here and only rely on write!().

Recently, I made a change (to be merged soon) on render_attributes_in_code() that doesn't have Buffer nor impl Write in its input. Instead, it returns impl Display, which then can be used for write!().

See the change here

Co-authored-by: Nicky Lim <nickylim.p@gmail.com>
@GuillaumeGomez
Copy link
Member

Please fix merge conflicts.

@sladyn98
Copy link
Contributor Author

I will reopen this PR in a different commit, currently was just experimenting with my config.toml, which cause codgen to break.

@sladyn98 sladyn98 closed this Jun 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants