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

Function signature of resource_equals differs from implementation in docs #8898

Open
RobWalt opened this issue Jun 20, 2023 · 6 comments · Fixed by rust-lang/rust#113013
Open
Labels
A-ECS Entities, components, systems, and events C-Docs An addition or correction to our documentation S-Blocked This cannot move forward until something else changes

Comments

@RobWalt
Copy link
Contributor

RobWalt commented Jun 20, 2023

How can Bevy's documentation be improved?

In the latest version of bevys docs, the signature of resource_equals seems to be buggy.

I was looking into how the common run conditions were implemented and was a bit confused when I saw this

image

I needed to check the bevy repos code to verify that the returned closure returns a bool.

image

I don't know how to fix it yet though, but I'll see what I can do!

@RobWalt RobWalt added C-Docs An addition or correction to our documentation S-Needs-Triage This issue needs to be labelled labels Jun 20, 2023
@Selene-Amanita Selene-Amanita added A-ECS Entities, components, systems, and events and removed S-Needs-Triage This issue needs to be labelled labels Jun 20, 2023
@Selene-Amanita
Copy link
Member

Selene-Amanita commented Jun 20, 2023

To note: it has the correct signature in the sub-crate's documentation: https://docs.rs/bevy_ecs/latest/bevy_ecs/schedule/common_conditions/fn.resource_equals.html

And it also happens on main, and for other conditions in the common_conditions module.

any_with_components appears with a weird unnecessary line break in bevy_ecs: https://docs.rs/bevy_ecs/latest/bevy_ecs/schedule/common_conditions/fn.any_with_component.html but not in bevy: https://docs.rs/bevy/latest/bevy/ecs/schedule/common_conditions/fn.any_with_component.html , traits are not always at the same place (in where or not), PartialEq to PartialEq<T>, ...

Probably more a problem with rustdoc, but we can maybe get around it.

Edit: Possibly related issue for rustdoc: rust-lang/rust#44306

@Selene-Amanita Selene-Amanita added the S-Blocked This cannot move forward until something else changes label Jun 21, 2023
@fmease
Copy link

fmease commented Jun 24, 2023

This is definitely the fault of rustdoc (source: I'm an active rustdoc contributor specializing in cross-crate reexports).

@Selene-Amanita
Copy link
Member

Selene-Amanita commented Jun 24, 2023

@fmease thanks a lot for the confirmation! Any idea why there's a line break in any_with_components doc?

@fmease
Copy link

fmease commented Jun 24, 2023

Any idea why there's a line break in any_with_components doc?

Ah, that doesn't look ideal. I've just checked and it's because rustdoc measures & and adds up the length of the function name, the arguments and the return type, realizes it's bigger than 80 bytes and thus inserts some line breaks inside the argument list.
I'm sure this heuristic can be improved (made more sophisticated and also made to count the generics) but I think the most straightforward fix would be to just keep it the way it is and to just not print an extra line break if the argument list is empty:

pub fn any_with_component<T: Component>(
) -> impl FnMut(Query<'_, '_, (), With<T>>) -> bool + Clone

^ Would you be fine with this output or would you rather have rustdoc output () (parentheses on the same line)? This would imply increasing the threshold, otherwise we would end up with non-standard style. For comparison, rustfmt also breaks empty argument lists into two lines similar to the example I gave above (except that the threshold seems to be a bit higher).

@Selene-Amanita
Copy link
Member

^ Would you be fine with this output or would you rather have rustdoc output ()

I am fine with this output, I'll ask other people on Bevy's discord just in case.

@fmease
Copy link

fmease commented Jun 24, 2023

Ah, I've just noticed that this regressed in rust-lang/rust#109011 meaning that before that it used to be rendered just like I proposed.

matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Jun 25, 2023
…slim-arg-list, r=camelid

rustdoc: get rid of extra line when line-wrapping fn decls with empty arg list

Fixes bevyengine/bevy#8898 (comment):

![Screenshot 2023-06-24 at 23-42-53 any_with_component in bevy_ecs schedule common_conditions - Rust](https://github.com/rust-lang/rust/assets/14913065/4646eba6-b186-4d78-96d9-aad716a4ef5d)

It now prints as shown below (which conforms to the style guide):

```rs
pub fn any_with_component<T: Component>(
) -> impl FnMut(Query<'_, '_, (), With<T>>) -> bool + Clone
```

The bug was introduced in rust-lang#109011.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-ECS Entities, components, systems, and events C-Docs An addition or correction to our documentation S-Blocked This cannot move forward until something else changes
Projects
None yet
3 participants