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

Fix Display docs to accurately describe bounds behaviour #297

Merged
merged 9 commits into from
Aug 22, 2023

Conversation

MegaBluejay
Copy link
Contributor

@MegaBluejay MegaBluejay commented Aug 21, 2023

Resolves #281

Synopsis

The docs for the Display derive state that explicitly specified bounds replace the auto-inferred ones, when this is actually not the case.

There's also some outdated information about what's allowed in #[display(bound(...)] arguments, and incorrect example generated where clauses.

Solution

Update the docs

Checklist

  • Documentation is updated (if required)
  • Tests are added/updated (if required)
  • CHANGELOG entry is added (if required)

@MegaBluejay
Copy link
Contributor Author

MegaBluejay commented Aug 21, 2023

@tyranron
#[display(bound(...))] already adds to the inferred bounds, rather than replacing them

This is shown in the docs example and this test, just not documented correctly.

This was the case at least as far back as v0.99.0, so I'm not sure what to do about the changelog, since there's no actual change being made

@MegaBluejay MegaBluejay marked this pull request as ready for review August 22, 2023 11:42
Copy link
Collaborator

@tyranron tyranron left a comment

Choose a reason for hiding this comment

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

@MegaBluejay please, trace back the commit/PR where this was changed, and mention it in the CHANGELOG.

@tyranron
Copy link
Collaborator

Ah, sorry, didn't understand you correctly. Then, please, add the CHANGELOG entry to the "Fixed" section, explaining the wrong docs were fixed.

Do you see a need in more tests to control this behavior?

@MegaBluejay
Copy link
Contributor Author

Do you see a need in more tests to control this behavior?

I added a few for the case with no inferred Display bound, but overall LGTM

Copy link
Collaborator

@tyranron tyranron left a comment

Choose a reason for hiding this comment

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

@MegaBluejay good job! 🍻

@tyranron tyranron added this to the 1.0.0 milestone Aug 22, 2023
@tyranron tyranron added the docs label Aug 22, 2023
@tyranron tyranron merged commit 58f6dd3 into JelteF:master Aug 22, 2023
16 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Redundant bound for Debug on skipped field/make display(bound) do the same as debug(bound)
2 participants